[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/3] Add support for shutdown / reboot APIs in LXC driver



On Thu, Nov 29, 2012 at 01:39:34PM -0500, Eric Blake wrote:
> > Add support for doing controlled shutdown / reboot in the LXC
> > driver. The default behaviour is to try talking to /dev/initctl
> > inside the container's virtual root (/proc/$INITPID/root). This
> > works with sysvinit or systemd. If that file does not exist
> > then send SIGTERM (for shutdown) or SIGHUP (for reboot). These
> > signals are not any kind of particular standard for shutdown
> > or reboot, just something apps can choose to handle. The new
> > virDomainSendProcessSignal allows for sending custom signals.
> > 
> > We might allow the choice of SIGTERM/HUP to be configured for
> > LXC containers via the XML in the future.
> 
> Maybe even via new attributes or sub-elements of the existing
> <on_reboot> XML.
> 
> > +
> > +    if (flags == 0 ||
> > +        (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
> > +        if ((rc =
> > virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
> > +                                        vroot)) < 0) {
> > +            goto cleanup;
> > +        }
> > +        if (rc == 0 && flags != 0 &&
> 
> Based on libvirt.c (well, if you follow my advice in 2/3 about
> enforcing the mutual exclusion between the four explicit
> shutdown methods), if 'flags != 0', then we are guaranteed
> that 'flags == VIR_DOMAIN_SHUTDOWN_INITCTL)' at this point...
> 
> > +            ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
> 
> ...which means this leg of the conditional is always true.
> 
> Or is your intent to allow the user to specify multiple flags
> rather than enforcing mutual exclusion between the flags?  And
> if so, you need to fix libvirt.c to drop the mutual exclusion,
> as well as to document that when multiple flags are specified,
> it is up to the hypervisor which method is attempted first.

Yep, I think allowing multiple flags is fine at the API level.

I'd do that as a followup commit since it doesn't affect this,
only the existing QEMU code.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]