[libvirt] [PATCH 2/3] Introduce two new methods for triggering controlled shutdown/reboot

[Daniel P. Berrange]

On Thu, Nov 29, 2012 at 01:27:26PM -0500, Eric Blake wrote:
> > The virDomainShutdownFlags and virDomainReboot APIs allow the caller
> > to request the operation is implemented via either acpi button press
> > or a guest agent. For containers, a couple of other methods make
> > sense, a message to /dev/initctl, and direct kill(SIGTERM|HUP) of
> > the container init process.
> 
> Indeed - this is a nice way to tie in your earlier patches.
> 
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> >  include/libvirt/libvirt.h.in |  4 ++++
> >  tools/virsh-domain.c         | 18 ++++++++++++++----
> >  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> Alas, you are missing documentation of the new flags in
> src/libvirt.c (not that the existing flags were documented
> there yet), 

Well the docs refer to the corresponding enum, whcih will
be included in the docs with its descriptions. So isn't
that better than duplicating the enum descriptions again.

>             as well as missing an update to this portion
> of libvirt.c:virDomainShutdownFlags() (and its reboot
> counterpart):
> 
>     /* At most one of these two flags should be set.  */
>     if ((flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) &&
>         (flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
>         virReportInvalidArg(flags, "%s",
>                             _("flags for acpi power button and guest agent are mutually exclusive"));
>         goto error;
>     }
> 
> That should be updated to check that now at most one of the
> four flags are present.

IMHO that is a bogus check that should be deleted. Though the QEMU
driver might not implement it, it seems perfectly valid to ask the
driver to try multiple methods - after all it is able todo that by
default. Sure you can't specify ordering, but that's fine.

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 :|