Re: [Xen-devel] [PATCH] libxl: support ACPI shutdown flag

[Jim Fehlig <jfehlig@xxxxxxxx>]

Any comments on this small patch?  If possible, I'd like to push it for
the 1.2.4 release.

Thanks!
Jim

Jim Fehlig wrote:
> Add support for VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flag in
> libxlDomainShutdownFlags().  Inspired by similar functionality
> in the Xen xl client.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>
> I considered invoking libxl_send_trigger() immediately when
> VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flag is specified, but in the
> end decided to only honor the flag if a "normal" shutdown request
> failed.  This behavior is similar to xl and conforms to the
> virDomainShutdownFlags() docs
>
> "If @flags is set to zero, then the hypervisor will choose the method
> of shutdown it considers best. To have greater control pass one or
> more of the virDomainShutdownFlagValues. The order in which the
> hypervisor tries each shutdown method is undefined, and a hypervisor
> is not required to support all methods."
>
> I'm certainly receptive to only invoking libxl_send_trigger() when
> VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN is specified if folks think that
> is a better approach.
>
>  src/libxl/libxl_driver.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index b3f8df6..2b30c08 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -868,7 +868,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int 
> flags)
>      int ret = -1;
>      libxlDomainObjPrivatePtr priv;
>  
> -    virCheckFlags(0, -1);
> +    virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN, -1);
>  
>      if (!(vm = libxlDomObjFromDomain(dom)))
>          goto cleanup;
> @@ -883,17 +883,35 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int 
> flags)
>      }
>  
>      priv = vm->privateData;
> -    if (libxl_domain_shutdown(priv->ctx, vm->def->id) != 0) {
> +    ret = libxl_domain_shutdown(priv->ctx, vm->def->id);
> +    if (ret == ERROR_NOPARAVIRT) {
> +        if (flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) {
> +            VIR_DEBUG("PV control interface not available, "
> +                      "sending ACPI power button event.");
> +            ret = libxl_send_trigger(priv->ctx, vm->def->id,
> +                                     LIBXL_TRIGGER_POWER, 0);
> +        } else {
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("PV control interface not available, "
> +                               "preventing external graceful shutdown. "
> +                               "Consider using an ACPI power event."));
> +            ret = -1;
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (ret != 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Failed to shutdown domain '%d' with libxenlight"),
>                         vm->def->id);
> +        ret = -1;
>          goto cleanup;
>      }
>  
> -    /* vm is marked shutoff (or removed from domains list if not persistent)
> +    /* ret == 0 == successful shutdown request
> +     * vm is marked shutoff (or removed from domains list if not persistent)
>       * in shutdown event handler.
>       */
> -    ret = 0;
>  
>   cleanup:
>      if (vm)
>   

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