Re: [Xen-devel] [PATCH] libxl: Make domain_shutdown fail if graceful not possible

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2011-01-19 at 17:02 +0000, Ian Jackson wrote:
> Currently "xl shutdown" (like "xm shutdown") is not capable of doing
> the proper ACPI negotiation with an HVM no-pv-drivers guest which
> would be necessary for a graceful shutdown.
> 
> Instead (following the ill-advised lead of "xm shutdown") it simply
> shoots the guest in the head.
> 
> This patch changes the behaviour so that "xl shutdown" fails if the
> domain cannot be shut down gracefully for this reason and suggests in
> the error message using destroy instead.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

I strongly agree with the principal that graceful commands shouldn't
fallback automatically to a non-graceful variant!

Acked/Reviewed-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Perhaps the check should be before the xs_write though -- i.e. check for
pvdriver support before attempting to use it?

Ian.

> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 7ffb985..2cc2d21 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -510,29 +510,19 @@ int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t 
> domid, int req)
>  
>      xs_write(ctx->xsh, XBT_NULL, shutdown_path, req_table[req], 
> strlen(req_table[req]));
>      if (libxl__domain_is_hvm(ctx,domid)) {
> -        unsigned long acpi_s_state = 0;
>          unsigned long pvdriver = 0;
>          int ret;
> -        ret = xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_ACPI_S_STATE, 
> &acpi_s_state);
> -        if (ret<0) {
> -            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting ACPI S-state");
> -            libxl__free_all(&gc);
> -            return ERROR_FAIL;
> -        }
>          ret = xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_CALLBACK_IRQ, 
> &pvdriver);
>          if (ret<0) {
>              LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting HVM callback 
> IRQ");
>              libxl__free_all(&gc);
>              return ERROR_FAIL;
>          }
> -        if (!pvdriver || acpi_s_state != 0) {
> -            ret = xc_domain_shutdown(ctx->xch, domid, req);
> -            if (ret<0) {
> -                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unpausing domain");
> -                libxl__free_all(&gc);
> -                return ERROR_FAIL;
> -            }
> -       }
> +        if (!pvdriver) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "HVM domain without PV 
> drivers:"
> +                       " graceful shutdown not possible, use destroy");
> +            return ERROR_FAIL;
> +        }
>      }
>      libxl__free_all(&gc);
>      return 0;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel