From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Daniel P. Berrange" Subject: Re: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags Date: Fri, 2 May 2014 15:14:52 +0100 Message-ID: <20140502141451.GB5206__22669.541827827$1399040198$gmane$org@redhat.com> References: <1398982479-20632-1-git-send-email-jfehlig@suse.com> <1398982479-20632-4-git-send-email-jfehlig@suse.com> <20140502094650.GF22878@redhat.com> <5363A51C.6000802@suse.com> Reply-To: "Daniel P. Berrange" Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <5363A51C.6000802@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jim Fehlig Cc: libvir-list@redhat.com, Ian Campbell , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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 > >> --- > >> 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 :|