From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Daniel P. Berrange" Subject: Re: [libvirt] [PATCH V2 2/3] libxl: support PARAVIRT and ACPI shutdown flags Date: Fri, 2 May 2014 10:40:34 +0100 Message-ID: <20140502094034.GD22878__36689.8321467035$1399023741$gmane$org@redhat.com> References: <1398982479-20632-1-git-send-email-jfehlig@suse.com> <1398982479-20632-3-git-send-email-jfehlig@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: <1398982479-20632-3-git-send-email-jfehlig@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 Thu, May 01, 2014 at 04:14:38PM -0600, Jim Fehlig wrote: > Add support for VIR_DOMAIN_SHUTDOWN_PARAVIRT and > VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flags in > libxlDomainShutdownFlags(). > > Signed-off-by: Jim Fehlig > --- > src/libxl/libxl_driver.c | 34 ++++++++++++++++++++++++++-------- > 1 file changed, 26 insertions(+), 8 deletions(-) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index dcc3ac2..28e8512 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -873,7 +873,11 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) > int ret = -1; > libxlDomainObjPrivatePtr priv; > > - virCheckFlags(0, -1); > + virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | > + VIR_DOMAIN_SHUTDOWN_PARAVIRT, -1); > + if (flags == 0) > + flags = VIR_DOMAIN_SHUTDOWN_PARAVIRT | > + VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN; > > if (!(vm = libxlDomObjFromDomain(dom))) > goto cleanup; > @@ -888,18 +892,32 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) > } > > priv = vm->privateData; > - if (libxl_domain_shutdown(priv->ctx, vm->def->id) != 0) { > + if (flags & VIR_DOMAIN_SHUTDOWN_PARAVIRT) { > + ret = libxl_domain_shutdown(priv->ctx, vm->def->id); > + if (ret == 0) > + goto cleanup; > + > + if (ret != ERROR_NOPARAVIRT) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Failed to shutdown domain '%d' with libxenlight"), > + vm->def->id); > + ret = -1; > + goto cleanup; > + } I think you need 'ret = -1' here too, otherwise if domain shutdown fails, but flags does not request ACPI, then we can leak ret == ERROR_NOPARAVIRT back to the public API. > + } > + > + if (flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) { > + ret = libxl_send_trigger(priv->ctx, vm->def->id, > + LIBXL_TRIGGER_POWER, 0); > + if (ret == 0) > + goto cleanup; > + > virReportError(VIR_ERR_INTERNAL_ERROR, > _("Failed to shutdown domain '%d' with libxenlight"), > vm->def->id); > - goto cleanup; > + ret = -1; > } > > - /* vm is marked shutoff (or removed from domains list if not persistent) > - * in shutdown event handler. > - */ > - ret = 0; > - > cleanup: > if (vm) > virObjectUnlock(vm); ACK to the patch with that fix added (post 1.2.4 release of course) 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 :|