| Message ID | 1452258526-4797-15-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2016-01-08 at 14:08 +0100, Juergen Gross wrote: > +# get xenstore domain id (or 0 if no xenstore domain) > +get_xsdomid() > +{ > + XS_DOMID=`${bindir}/xenstore-read /tool/xenstored/domid 2>/dev/null` > + if test $? -ne 0; then > + XS_DOMID=0 > + fi In principal this could be: + XS_DOMID=`${bindir}/xenstore-read /tool/xenstored/domid 2>/dev/null || echo 0` But I'm far from convinced that is an improvement. > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index f9933cb..29f9d19 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -4856,15 +4856,17 @@ static int main_shutdown_or_reboot(int do_reboot, > int argc, char **argv) > if (wait_for_it) > deathws = calloc(nb_domain, sizeof(*deathws)); > > + wait_for_it = 0; Please can we avoid reusing the boolean flag used for argument handling as a counter. I think you will need to introduce a new nrdeathws. I was thinking if you could invert the loop to count downwards such that you can decrement nb_domain for uninteresting domains, but then the uses of deathws[i] don't work, the solving of which would no doubt require another variable anyway. > for (i = 0; i<nb_domain; i++) { > - if (dominfo[i].domid == 0) > + if (dominfo[i].domid == 0 || dominfo[i].never_stop) > continue; > fn(dominfo[i].domid, deathws ? &deathws[i] : NULL, i, > fallback_trigger); > + wait_for_it++; > } > > - if (wait_for_it) { > - wait_for_domain_deaths(deathws, nb_domain - 1 /* not dom 0 > */); > + if (deathws) { > + wait_for_domain_deaths(deathws, wait_for_it); > free(deathws); > } >
On 15/01/16 17:12, Ian Campbell wrote: > On Fri, 2016-01-08 at 14:08 +0100, Juergen Gross wrote: >> +# get xenstore domain id (or 0 if no xenstore domain) >> +get_xsdomid() >> +{ >> + XS_DOMID=`${bindir}/xenstore-read /tool/xenstored/domid 2>/dev/null` >> + if test $? -ne 0; then >> + XS_DOMID=0 >> + fi > > In principal this could be: > + XS_DOMID=`${bindir}/xenstore-read /tool/xenstored/domid 2>/dev/null || echo 0` > > But I'm far from convinced that is an improvement. I think I'll leave it as before. > >> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >> index f9933cb..29f9d19 100644 >> --- a/tools/libxl/xl_cmdimpl.c >> +++ b/tools/libxl/xl_cmdimpl.c >> @@ -4856,15 +4856,17 @@ static int main_shutdown_or_reboot(int do_reboot, >> int argc, char **argv) >> if (wait_for_it) >> deathws = calloc(nb_domain, sizeof(*deathws)); >> >> + wait_for_it = 0; > > Please can we avoid reusing the boolean flag used for argument handling as > a counter. > > I think you will need to introduce a new nrdeathws. Okay, will do. Thanks, Juergen
diff --git a/tools/hotplug/Linux/xendomains.in b/tools/hotplug/Linux/xendomains.in index dfe0b33..9f88649 100644 --- a/tools/hotplug/Linux/xendomains.in +++ b/tools/hotplug/Linux/xendomains.in @@ -196,6 +196,15 @@ rdnames() done } +# get xenstore domain id (or 0 if no xenstore domain) +get_xsdomid() +{ + XS_DOMID=`${bindir}/xenstore-read /tool/xenstored/domid 2>/dev/null` + if test $? -ne 0; then + XS_DOMID=0 + fi +} + LIST_GREP='(domain\|(domid\|(name\|^ {$\|"name":\|"domid":' parseln() { @@ -216,12 +225,14 @@ parseln() is_running() { + get_xsdomid rdname $1 RC=1 name=;id= while read LN; do parseln "$LN" || continue if test $id = 0; then continue; fi + if test $id = $XS_DOMID; then continue; fi case $name in ($NM) RC=0 @@ -302,10 +313,12 @@ start() all_zombies() { + get_xsdomid name=;id= while read LN; do parseln "$LN" || continue if test $id = 0; then continue; fi + if test $id = $XS_DOMID; then continue; fi if test "$state" != "-b---d" -a "$state" != "-----d"; then return 1; fi @@ -351,11 +364,13 @@ stop() if test "$XENDOMAINS_AUTO_ONLY" = "true"; then rdnames fi + get_xsdomid echo -n "Shutting down Xen domains:" name=;id= while read LN; do parseln "$LN" || continue if test $id = 0; then continue; fi + if test $id = $XS_DOMID; then continue; fi echo -n " $name" if test "$XENDOMAINS_AUTO_ONLY" = "true"; then eval " diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index f9933cb..29f9d19 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -4856,15 +4856,17 @@ static int main_shutdown_or_reboot(int do_reboot, int argc, char **argv) if (wait_for_it) deathws = calloc(nb_domain, sizeof(*deathws)); + wait_for_it = 0; for (i = 0; i<nb_domain; i++) { - if (dominfo[i].domid == 0) + if (dominfo[i].domid == 0 || dominfo[i].never_stop) continue; fn(dominfo[i].domid, deathws ? &deathws[i] : NULL, i, fallback_trigger); + wait_for_it++; } - if (wait_for_it) { - wait_for_domain_deaths(deathws, nb_domain - 1 /* not dom 0 */); + if (deathws) { + wait_for_domain_deaths(deathws, wait_for_it); free(deathws); }
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. Here the xenstore domain is flagged as "never stop". Signed-off-by: Juergen Gross <jgross@suse.com> --- V3: xl shutdown modified to query the "never_stop" flag instead of the xenstore domain id as requested by Ian Campbell drop usage of xenstore-exists as requested by Ian Campbell --- tools/hotplug/Linux/xendomains.in | 15 +++++++++++++++ tools/libxl/xl_cmdimpl.c | 8 +++++--- 2 files changed, 20 insertions(+), 3 deletions(-)