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

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

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@xxxxxxxx>
> ---
>  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, 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.
We only did this with QEMU since we didn't have any alternative to
make reboot work with QEMU at the time, since the QEMU guest agent
did not exist then.

A hard reset of the machine board is something you can implement
in the virDomainReset() API if you like though.

> +        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)


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 :|

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel