[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [libvirt] [PATCH] libxl: support ACPI shutdown flag
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 :| _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |