Re: [libvirt] [PATCH 2/3] Introduce two new methods for triggering controlled shutdown/reboot
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(a)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 :|