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:57:09 +0100 Message-ID: <20140502145709.GF5206__8196.92983427576$1399042735$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> <20140502141451.GB5206@redhat.com> <5363AF2D.8020104@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: <5363AF2D.8020104@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, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Fri, May 02, 2014 at 08:43:57AM -0600, Jim Fehlig wrote: > Daniel P. Berrange wrote: > > On Fri, May 02, 2014 at 08:01:00AM -0600, Jim Fehlig wrote: > > > >> Daniel P. Berrange wrote: > >> > >>> 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. > > > > Just to clarify, do you mean changing this patch to the attached one? Yes, you got it. > From e76f891bd843dd4f7b895d3929c9f561162a69a9 Mon Sep 17 00:00:00 2001 > From: Jim Fehlig > Date: Thu, 1 May 2014 15:00:47 -0600 > Subject: [PATCH 3/3] libxl: support PARAVIRT reboot flag > > Add support for the VIR_DOMAIN_REBOOT_PARAVIRT flag in > libxlDomainReboot(). > > Signed-off-by: Jim Fehlig > --- > src/libxl/libxl_driver.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 28e8512..edbfa57 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -938,7 +938,9 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) > int ret = -1; > libxlDomainObjPrivatePtr priv; > > - virCheckFlags(0, -1); > + virCheckFlags(VIR_DOMAIN_REBOOT_PARAVIRT, -1); > + if (flags == 0) > + flags = VIR_DOMAIN_REBOOT_PARAVIRT; > > if (!(vm = libxlDomObjFromDomain(dom))) > goto cleanup; > @@ -953,13 +955,16 @@ 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; > + > virReportError(VIR_ERR_INTERNAL_ERROR, > _("Failed to reboot domain '%d' with libxenlight"), > vm->def->id); > - goto cleanup; > + ret = -1; > } > - ret = 0; > > cleanup: > if (vm) ACK 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 :|