Re: [PATCH v2 COLOPre 06/13] tools/libxl: Introduce a new internal API libxl__domain_unpause()

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

On Thu, 2015-06-11 at 10:21 +0800, Yang Hongyang wrote:
> 
> On 06/10/2015 11:37 PM, Ian Campbell wrote:
> > On Mon, 2015-06-08 at 11:43 +0800, Yang Hongyang wrote:
> >> From: Wen Congyang <wency@cn.fujitsu.com>
> >>
> >> The guest is paused after libxl_domain_create_restore().
> >> Secondary vm is running in colo mode. So we need to unpause
> >> the guest. The current API libxl_domain_unpause() is
> >> not an internal API. Introduce a new API to support it.
> >> No functional change.
> >
> > In general there is nothing wrong with using a public function
> > internally. Is there some special consideration here?
> 
> It's just that we thought it's better to use internal functions for
> internal purpose.
> Most the public functions take ctx as the first param, the internal functions
> take gc/egc as the first param(although we can get ctx from gcs and call
> public functions when needed).
> If it doesn't matter, we can drop this patch.

It doesn't matter so you can, yes.

> 
> >
> >>
> >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> >> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> >> ---
> >>   tools/libxl/libxl.c          | 20 ++++++++++++++------
> >>   tools/libxl/libxl_internal.h |  1 +
> >>   2 files changed, 15 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> >> index ba2da92..d5691dc 100644
> >> --- a/tools/libxl/libxl.c
> >> +++ b/tools/libxl/libxl.c
> >> @@ -933,9 +933,8 @@ out:
> >>       return AO_INPROGRESS;
> >>   }
> >>
> >> -int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
> >> +int libxl__domain_unpause(libxl__gc *gc, uint32_t domid)
> >>   {
> >> -    GC_INIT(ctx);
> >>       char *path;
> >>       char *state;
> >>       int ret, rc = 0;
> >> @@ -947,7 +946,7 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
> >>       }
> >>
> >>       if (type == LIBXL_DOMAIN_TYPE_HVM) {
> >> -        uint32_t dm_domid = libxl_get_stubdom_id(ctx, domid);
> >> +        uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
> >>
> >>           path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
> >>           state = libxl__xs_read(gc, XBT_NULL, path);
> >> @@ -957,12 +956,21 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
> >>                                            NULL, NULL, NULL);
> >>           }
> >>       }
> >> -    ret = xc_domain_unpause(ctx->xch, domid);
> >> -    if (ret<0) {
> >> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unpausing domain %d", domid);
> >> +
> >> +    ret = xc_domain_unpause(CTX->xch, domid);
> >> +    if (ret < 0) {
> >> +        LIBXL__LOG_ERRNO(CTX, LIBXL__LOG_ERROR, "unpausing domain %d", domid);
> >>           rc = ERROR_FAIL;
> >>       }
> >>    out:
> >> +    return rc;
> >> +}
> >> +
> >> +int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
> >> +{
> >> +    GC_INIT(ctx);
> >> +    int rc = libxl__domain_unpause(gc, domid);
> >> +
> >>       GC_FREE;
> >>       return rc;
> >>   }
> >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> >> index 20364c6..366470f 100644
> >> --- a/tools/libxl/libxl_internal.h
> >> +++ b/tools/libxl/libxl_internal.h
> >> @@ -1044,6 +1044,7 @@ _hidden int libxl__domain_restore(libxl__gc *gc, uint32_t domid);
> >>   _hidden int libxl__domain_resume(libxl__gc *gc, uint32_t domid,
> >>                                    int suspend_cancel);
> >>   _hidden int libxl__domain_s3_resume(libxl__gc *gc, int domid);
> >> +_hidden int libxl__domain_unpause(libxl__gc *gc, uint32_t domid);
> >>
> >>   /* returns 0 or 1, or a libxl error code */
> >>   _hidden int libxl__domain_pvcontrol_available(libxl__gc *gc, uint32_t domid);
> >
> >
> > .
> >
>