Re: [Xen-devel] [PATCH v4 2/2] libxl: Use QMP cpu-add to hotplug CPU with qemu-xen.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-06-25 at 11:07 +0100, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> ---
> Change: ignore qmp_cpu_add return value.

I think this warrants a comment either in the code or in the actual body
of the commit message, since deliberately ignoring a return value is
something which we might wonder about in 6 months...

Everything else looks fine to me.

> ---
>  tools/libxl/libxl.c | 47 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index ee1fa9c..858a18d 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4237,33 +4237,68 @@ int libxl_domain_get_nodeaffinity(libxl_ctx *ctx, 
> uint32_t domid,
>      return 0;
>  }
>  
> -int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap 
> *cpumap)
> +static int libxl__set_vcpuonline_xenstore(libxl__gc *gc, uint32_t domid,
> +                                         libxl_bitmap *cpumap)
>  {
> -    GC_INIT(ctx);
>      libxl_dominfo info;
>      char *dompath;
>      xs_transaction_t t;
>      int i, rc = ERROR_FAIL;
>  
> -    if (libxl_domain_info(ctx, &info, domid) < 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> +    if (libxl_domain_info(CTX, &info, domid) < 0) {
> +        LOGE(ERROR, "getting domain info list");
>          goto out;
>      }
>      if (!(dompath = libxl__xs_get_dompath(gc, domid)))
>          goto out;
>  
>  retry_transaction:
> -    t = xs_transaction_start(ctx->xsh);
> +    t = xs_transaction_start(CTX->xsh);
>      for (i = 0; i <= info.vcpu_max_id; i++)
>          libxl__xs_write(gc, t,
>                         libxl__sprintf(gc, "%s/cpu/%u/availability", dompath, 
> i),
>                         "%s", libxl_bitmap_test(cpumap, i) ? "online" : 
> "offline");
> -    if (!xs_transaction_end(ctx->xsh, t, 0)) {
> +    if (!xs_transaction_end(CTX->xsh, t, 0)) {
>          if (errno == EAGAIN)
>              goto retry_transaction;
>      } else
>          rc = 0;
>  out:
> +    return rc;
> +}
> +
> +static int libxl__set_vcpuonline_qmp(libxl__gc *gc, uint32_t domid,
> +                                     libxl_bitmap *cpumap)
> +{
> +    libxl_dominfo info;
> +    int i;
> +
> +    if (libxl_domain_info(CTX, &info, domid) < 0) {
> +        LOGE(ERROR, "getting domain info list");
> +        return ERROR_FAIL;
> +    }
> +    for (i = 0; i <= info.vcpu_max_id; i++) {
> +        if (libxl_bitmap_test(cpumap, i)) {
> +            libxl__qmp_cpu_add(gc, domid, i);
> +        }
> +    }
> +    return 0;
> +}
> +
> +int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap 
> *cpumap)
> +{
> +    GC_INIT(ctx);
> +    int rc;
> +    switch (libxl__device_model_version_running(gc, domid)) {
> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> +        rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap);
> +        break;
> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> +        rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap);
> +        break;
> +    default:
> +        rc = ERROR_INVAL;
> +    }
>      GC_FREE;
>      return rc;
>  }



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