Re: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags

["Daniel P. Berrange" <berrange@redhat.com>]

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 <jfehlig@suse.com>
> 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 <jfehlig@suse.com>
> ---
>  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 :|