From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH v2 13/13] tools: don't stop xenstore domain when stopping dom0 Date: Thu, 7 Jan 2016 07:52:22 +0100 Message-ID: <568E0B26.6050405@suse.com> References: <1450444471-6454-1-git-send-email-jgross@suse.com> <1450444471-6454-14-git-send-email-jgross@suse.com> <56741B43.3040800@citrix.com> <56741DEC.7050809@suse.com> <1452098017.21055.114.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1452098017.21055.114.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Andrew Cooper , xen-devel@lists.xen.org, ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com, wei.liu2@citrix.com, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On 06/01/16 17:33, Ian Campbell wrote: > 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 >>>> --- >>>> 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. Okay. > > Did you mean /tools? No. The xenstore path is /tool/... > > 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? Hen and egg problem. You need to know how to connect to xenstore (domain or daemon) before being able to read xenstore. > >>>> + 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 Okay, I'll change it. Juergen