On Fri, May 02, 2014 at 08:01:00AM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
On Thu, May 01, 2014 at 04:14:39PM -0600, Jim Fehlig wrote:
Add support for VIR_DOMAIN_REBOOT_PARAVIRT and VIR_DOMAIN_REBOOT_ACPI_POWER_BTN flags in libxlDomainReboot().
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 28e8512..6c63251 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -938,7 +938,11 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) int ret = -1; libxlDomainObjPrivatePtr priv;
- virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN | + VIR_DOMAIN_REBOOT_PARAVIRT, -1); + if (flags == 0) + flags = VIR_DOMAIN_REBOOT_PARAVIRT | + VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
if (!(vm = libxlDomObjFromDomain(dom))) goto cleanup; @@ -953,13 +957,31 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) }
priv = vm->privateData; - if (libxl_domain_reboot(priv->ctx, vm->def->id) != 0) { + if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) { + ret = libxl_domain_reboot(priv->ctx, vm->def->id); + if (ret == 0) + goto cleanup; + + if (ret != ERROR_NOPARAVIRT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reboot domain '%d' with libxenlight"), + vm->def->id); + ret = -1; + goto cleanup; + } + } + + if (flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) { + ret = libxl_send_trigger(priv->ctx, vm->def->id, + LIBXL_TRIGGER_RESET, 0);
What does this trigger in ACPI ? IIUC, it'll do a hard reset of the board,
Yes, I think you are right. Similar to pushing the reset button.
which is not the same as a controlled reboot which this API wants. There isn't any ACPI button that I know of that guests will interpret todo a controlled reboot, so in the QEMU driver we actually just send a normal ACPI shutdown event. We have QEMU configured with the '-no-shutdown' flag so when it finishes doing an controlled APCI shutdown, we can then reset the board and start the CPUs again, which gives the illusion of a controlled reboot.
Given that Xen has a decent paravirt reboot facility I'd probably just not bother with trying to fake the controlled reboot via ACPI.
Ok, that sounds reasonable to me. I'll drop this patch when pushing the others, post 1.2.4. Should 1/3 retain the VIR_DOMAIN_REBOOT_PARAVIRT addition tovirDomainRebootFlagValues?
I don't think you need to drop the patch/code. It is still useful, IMHO, to have the explicit flag for VIR_DOMAIN_REBOOT_PARAVIRT. I'd just suggest you remove the block of code for VIR_DOMAIN_REBOOT_ACPI_POWER_BTN impl in the reboot method. 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 :|