Re: [PATCH 4 of 6 V3] libxl: support suspend_cancel in domain_resume

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2012-02-03 at 06:50 +0000, rshriram@cs.ubc.ca wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1328251593 28800
> # Node ID f853c88f0230a2e9d2e1006a9cd220c4cd27e74d
> # Parent  329b3c94c618addb1e802cebc7fe23b12b432398
> libxl: support suspend_cancel in domain_resume
> 
> Add an extra parameter to libxl_domain_resume indicating
> if the caller wishes to use the SUSPEND_CANCEL style
> resume instead of the normal resume.
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> 
> diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -229,24 +229,29 @@ int libxl_domain_rename(libxl_ctx *ctx, 
>      return rc;
>  }
>  
> -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid)
> +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel)
>  {
>      GC_INIT(ctx);
>      int rc = 0;
>  
> -    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Called domain_resume on "
> -                "non-cooperative hvm domain %u", domid);
> -        rc = ERROR_NI;
> -        goto out;
> -    }
> -    if (xc_domain_resume(ctx->xch, domid, 0)) {
> +    if (xc_domain_resume(ctx->xch, domid, suspend_cancel)) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                          "xc_domain_resume failed for domain %u",
>                          domid);
>          rc = ERROR_FAIL;
>          goto out;
>      }
> +
> +    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> +        rc = libxl__domain_resume_device_model(gc, domid);
> +        if (rc) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "failed to resume device model for domain %u:%d",
> +                       domid, rc);
> +            goto out;
> +        }
> +    }
> +
>      if (!xs_resume_domain(ctx->xsh, domid)) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                          "xs_resume_domain failed for domain %u",
> diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
> @@ -268,7 +268,12 @@ int libxl_domain_create_restore(libxl_ct
>  void libxl_domain_config_dispose(libxl_domain_config *d_config);
>  int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
>                            uint32_t domid, int fd);
> -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
> +
> +/* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
> + *   If this parameter is true, use co-operative resume. The guest
> + *   must support this.
> + */
> +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
>  int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
>  int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
>  int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
> diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -2751,7 +2751,7 @@ static void migrate_domain(const char *d
>          if (common_domname) {
>              libxl_domain_rename(ctx, domid, away_domname, common_domname);
>          }
> -        rc = libxl_domain_resume(ctx, domid);
> +        rc = libxl_domain_resume(ctx, domid, 1);

Previously this code would have been equivalent to passing 0 not 1. It
may be ok to change this but it should be separate. However I'm a bit
dubious about changing it without adding some code to detect if the
guest supports it. (I know libxl_domain_resume is currently broken for
the non-cooperative suspend case but we shouldn't paper over that)

Ian.