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@xxxxxxxxxx>
> > ---
> > 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list