On Tue, Apr 22, 2014 at 11:12:16AM -0600, Jim Fehlig wrote:
> Add support for VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flag in
> libxlDomainShutdownFlags(). Inspired by similar functionality
> in the Xen xl client.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>
> I considered invoking libxl_send_trigger() immediately when
> VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flag is specified, but in the
> end decided to only honor the flag if a "normal" shutdown request
> failed. This behavior is similar to xl and conforms to the
> virDomainShutdownFlags() docs
>
> "If @flags is set to zero, then the hypervisor will choose the method
> of shutdown it considers best. To have greater control pass one or
> more of the virDomainShutdownFlagValues. The order in which the
> hypervisor tries each shutdown method is undefined, and a hypervisor
> is not required to support all methods."
>
> I'm certainly receptive to only invoking libxl_send_trigger() when
> VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN is specified if folks think that
> is a better approach.
The way I consider shutdown to operate is slightly different to
how you've done it in your patch
- flags == 0 -> arbitrary upto the hypervisor to decide.
Here you invoke libxl_domain_shutdown() and raise error
if this fails. This is a valid implementation, though
I'd suggest you could choose to try libxl_send_trigger
too in this scenario
The QEMU driver considers flags == 0, to be equivalent to
the union of all flags it supports. ie it effectivel changes
flags==0 to be flags = (ACPI_POWER_BTN | GUEST_AGENT)
- flags & VIR_DOMAIN_SHUTDOWN_NNNN -> exclusively try the
choice specified by the user.
When VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN is passed, you
are still trying libxl_domain_shutdown() before you try
to run libxl_send_trigger(). IMHO this is a bug - it
should try libxl_send_trigger exclusively if that was
the only bit that was specified in the flags.
IIUC libxl_domain_shutdown() will use the Xen paravirt channel
to trigger a controlled shutdown in the guest. Currently we have
the following flags defined
VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, /* hypervisor choice */
VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), /* Send ACPI event */
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), /* Use guest agent */
VIR_DOMAIN_SHUTDOWN_INITCTL = (1 << 2), /* Use initctl */
VIR_DOMAIN_SHUTDOWN_SIGNAL = (1 << 3), /* Send a signal */
None of those really map to the Xen paravirt shutdown
so I think we should define a new flag in the API.
VIR_DOMAIN_SHUTDOWN_PARAVIRT = (1 << 4), /* Use paravirt guest control */
Then I think the pseudo code makes sense
- If flags == 0
flags = (PARAVIRT | ACPI_POWER_BTN)
- If flags & PARAVIRT
libxl_domain_shutdown()
if (ret == 0)
goto done
if (ret != ERROR_NOPARAVIT) {
virRaiseError(...)
goto cleanup;
}
- If flags & ACPI_POWER_BTN
libxl_send_trigger()
if (ret == 0)
goto done
virRaiseError(...)
goto cleanup;
This gives users/apps full control over which methods are
tried. In the flags==0 case it'll try as many as possible.
If neccessary the user can force it to only do ACPI power,
or only do paravirt.
Regards,
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