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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-02-09 at 09:16 +0000, Ian Campbell wrote:
> On Fri, 2012-02-03 at 06:50 +0000, rshriram@xxxxxxxxx wrote:
> > # HG changeset patch
> > # User Shriram Rajagopalan <rshriram@xxxxxxxxx>
> > # 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@xxxxxxxxx>
> > 
> > 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)

BTW the rest of the patch looked fine. If those two 1s had been 0s I
would have: Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Ian.



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