Re: [Xen-devel] [PATCH] Prevent xl save from segfaulting when control/shutdown key is removed

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell
> Sent: 23 November 2011 10:13
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] Prevent xl save from segfaulting
> when control/shutdown key is removed
> 
> On Wed, 2011-11-23 at 09:45 +0000, Paul Durrant wrote:
> > # HG changeset patch
> > # User Paul Durrant <paul.durrant@xxxxxxxxxx> # Date 1322041530 0
> #
> > Node ID 3341e3e990568f459ae984cd9d2cac2d546eaa4e
> > # Parent  0a0c02a616768bfab16c072788cb76be1893c37f
> > Prevent xl save from segfaulting when control/shutdown key is
> removed
> >
> > To acknowledge the tools' setting of control/shutdown it is normal
> for
> > PV drivers to rm the key. This leads to libxl__xs_read() returning
> > NULL and thus a subsequent strcmp on the return value will cause a
> segfault.
> 
> The Linux PV drivers actually write "" rather than removing the key
> which is how this didn't get spotted already. We should be robust to
> PV drivers which remove it as well.
> 
> At start of day .../control is created ro (to the guest) whereas
> .../control/shutdown is created rw. Can you confirm that a second
> operation still succeeds if the guest rm's the node on the first
> action?
> 

At the moment I can't. I'm getting a complete VM wedge-up on restore somewhere 
in the region of re-connecting to store. I'll check the subsequent suspend 
behaviour once I can get the Vm to actually come back.
BTW what is the reason for creating control ro to the guest? In XenServer we 
allow the guest to write the control key to advertise feature-shutdown, 
feature-suspend etc. so that the tools know what values of control/shutdown the 
guest will respond to.

  Paul

> My concern is that while the first time the round the node will be
> rw the second time round the write will actually re-create the node
> (without setting the permissions) which might result in the node
> being ro for the guest (xenstore perms confuse me, but I think new
> nodes inherit the parent permissions).
> 
> That's assuming there's any chance of a second operation. I'm
> thinking of a wedged reboot followed by an attempt to shutdown
> instead or something like that. Perhaps in practice that wouldn't
> work anyway.
> 
> Apart form the above this change improves the robustness of the code
> so:
> 
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> 
> 
> >
> > diff -r 0a0c02a61676 -r 3341e3e99056 tools/libxl/libxl_dom.c
> > --- a/tools/libxl/libxl_dom.c       Mon Nov 21 21:28:34 2011 +0000
> > +++ b/tools/libxl/libxl_dom.c       Wed Nov 23 09:45:30 2011 +0000
> > @@ -444,6 +444,7 @@ static int libxl__domain_suspend_common_
> >              usleep(100000);
> >
> >              state = libxl__xs_read(si->gc, XBT_NULL, path);
> > +            if (!state) state = "";
> >
> >              watchdog--;
> >          }
> > @@ -463,6 +464,7 @@ static int libxl__domain_suspend_common_
> >              t = xs_transaction_start(ctx->xsh);
> >
> >              state = libxl__xs_read(si->gc, t, path);
> > +            if (!state) state = "";
> >
> >              if (!strcmp(state, "suspend"))
> >                  libxl__xs_write(si->gc, t, path, "");
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel