[PATCH v4 0/2] CPU hotplug for qemu-xen

[PATCH v4 0/2] CPU hotplug for qemu-xen

[Anthony PERARD]

Hi all,

This series implements the 'cpu-add' QMP command to hotplug CPU with qemu-xen.

When using adding vcpus with this series, libxl will report many "error". This
is because libxl call the QMP command 'cpu-add' for every plugged vcpus, even
those that have been plugged before. There is currently no way to list cpus
that are plugged by QEMU. Also, the way errors are handled by libxl_qmp those
not help yet to suppress this error printing.

Unplug of vcpu is not supported by the series.

Change since the last set:
  - ignore the return value of libxl__qmp_cpu_add as it those not give any
    usefull information.

And before (v2-v3):
  - use more and different macro in the libxl patch.

Anthony PERARD (2):
  libxl: Add "cpu-add" QMP command.
  libxl: Use QMP cpu-add to hotplug CPU with qemu-xen.

 tools/libxl/libxl.c          | 47 ++++++++++++++++++++++++++++++++++++++------
 tools/libxl/libxl_internal.h |  2 ++
 tools/libxl/libxl_qmp.c      | 21 ++++++++++++++++++++
 3 files changed, 64 insertions(+), 6 deletions(-)

-- 
Anthony PERARD

[PATCH v4 1/2] libxl: Add "cpu-add" QMP command.

[Anthony PERARD]

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
---
 tools/libxl/libxl_internal.h |  2 ++
 tools/libxl/libxl_qmp.c      | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3ba3a21..3e45b94 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1412,6 +1412,8 @@ _hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
 /* Set dirty bitmap logging status */
 _hidden int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enable);
 _hidden int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid, const libxl_device_disk *disk);
+/* Add a virtual CPU */
+_hidden int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int index);
 /* close and free the QMP handler */
 _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
 /* remove the socket file, if the file has already been removed,
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 644d2c0..3d6dec6 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -668,6 +668,18 @@ static void qmp_parameters_add_bool(libxl__gc *gc,
     qmp_parameters_common_add(gc, param, name, obj);
 }
 
+static void qmp_parameters_add_integer(libxl__gc *gc,
+                                       libxl__json_object **param,
+                                       const char *name, const int i)
+{
+    libxl__json_object *obj;
+
+    obj = libxl__json_object_alloc(gc, JSON_INTEGER);
+    obj->u.i = i;
+
+    qmp_parameters_common_add(gc, param, name, obj);
+}
+
 #define QMP_PARAMETERS_SPRINTF(args, name, format, ...) \
     qmp_parameters_add_string(gc, args, name, \
                               libxl__sprintf(gc, format, __VA_ARGS__))
@@ -929,6 +941,15 @@ int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid,
     }
 }
 
+int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int index)
+{
+    libxl__json_object *args = NULL;
+
+    qmp_parameters_add_integer(gc, &args, "id", index);
+
+    return qmp_run_command(gc, domid, "cpu-add", args, NULL, NULL);
+}
+
 int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
                                const libxl_domain_config *guest_config)
 {
-- 
Anthony PERARD

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

[Anthony PERARD]

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

---
Change: ignore qmp_cpu_add return value.
---
 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;
 }
-- 
Anthony PERARD

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

[George Dunlap]

On Tue, Jun 25, 2013 at 11:07 AM, Anthony PERARD
<anthony.perard@citrix.com> wrote:
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>

Ian / Ian, I think we need to get this checked Real Soon Now if it's
going to make it through the push-gate tomorrow in time to make it
into the RC for the test day on Friday.

 -George

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

[Ian Campbell]

On Tue, 2013-06-25 at 11:07 +0100, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> ---
> 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;
>  }