Re: [PATCH v2 13/13] tools: don't stop xenstore domain when stopping dom0

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2015-12-18 at 15:53 +0100, Juergen Gross wrote:
> On 18/12/15 15:42, Andrew Cooper wrote:
> > On 18/12/15 13:14, Juergen Gross wrote:
> > > When restarting or shutting down dom0 the xendomains script tries to
> > > stop all other domains. Don't do this for the xenstore domain, as it
> > > might survive a dom0 reboot in the future.
> > > 
> > > The same applies to xl shutdown --all.
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > ---
> > >  tools/hotplug/Linux/xendomains.in | 17 +++++++++++++++++
> > >  tools/libxl/xl_cmdimpl.c          | 19 +++++++++++++++----
> > >  2 files changed, 32 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tools/hotplug/Linux/xendomains.in
> > > b/tools/hotplug/Linux/xendomains.in
> > > index dfe0b33..70b7f16 100644
> > > --- a/tools/hotplug/Linux/xendomains.in
> > > +++ b/tools/hotplug/Linux/xendomains.in
> > > @@ -196,6 +196,17 @@ rdnames()
> > >      done
> > >  }
> > >  
> > > +# set xenstore domain id (or 0 if no xenstore domain)
> > > +get_xsdomid()
> > 
> > A get/set mismatch.
> 
> Hmm, depends.
> 
> It is getting the domid of the xenstore domain and is setting
> XS_DOMID accordingly. The main semantics are to get the correct
> domid.
> 
> > 
> > > +{
> > > +    ${bindir}/xenstore-exists /tool/xenstored/domid
> > > +    if test $? -ne 0; then
> > > +        XS_DOMID=0
> > > +    else
> > > +        XS_DOMID=`${bindir}/xenstore-read /tool/xenstored/domid`

Please update docs/misc/xenstore-paths.markdown with this.

Did you mean /tools?

Earlier in the series there was a patch which looped over xc_dom_info
looking for the xs domain -- if this is in xenstore can't it use that?

> > > +    fi
> > 
> > This is racy.  Can't you use a failure of xenstore-read as a signal
> > that
> > the key doesn't exist?
> 
> In theory it is racy. OTOH the race would require the xenstore domain to
> be started between the call of xenstore-exists and xenstore-read, but
> xenstore-exists will block in case no xenstore is available. And no, I
> don't have to check that. the whole script will bail out early in this
> case as in the beginning xl is tested to work which will be the case
> with xenstore available only.
> 
> And using xenstore-read alone is ugly as it will barf in case the key
> isn't existing.

XS_DOMID=`${bindir}/xenstore-read /tool/xenstored/domid 2>/dev/null`

seems like it should work:
root@st40:~# xenstore-read /foo 2>/dev/null; echo $?
1
root@st40:~# xenstore-read /local/domain/0/name 2>/dev/null; echo $?
Domain-0
0

> 
> 
> Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel