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

[PATCH v3 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.
This time, only the patches for libxl and SeaBIOS are sent as the qemu-xen
patches haven't changed.

There are two patches for libxl and one patch for SeaBIOS.

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:
  - use more and different macro in the libxl patch.
  - don't run smp_probe() at all in SeaBIOS.

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

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

-- 
Anthony PERARD

[PATCH v3 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>
---
 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

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

[George Dunlap]

On Mon, Jun 24, 2013 at 2:32 PM, Anthony PERARD
<anthony.perard@citrix.com> wrote:
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Looks fine to me:

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
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Anthony PERARD]

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ee1fa9c..95214ed 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4237,33 +4237,70 @@ 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, rc = 0;
+
+    if (libxl_domain_info(CTX, &info, domid) < 0) {
+        LOGE(ERROR, "getting domain info list");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    for (i = 0; i <= info.vcpu_max_id; i++) {
+        if (libxl_bitmap_test(cpumap, i)) {
+            rc = libxl__qmp_cpu_add(gc, domid, i);
+        }
+    }
+out:
+    return rc;
+}
+
+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 v3 2/2] libxl: Use QMP cpu-add to hotplug CPU with qemu-xen.

[George Dunlap]

On Mon, Jun 24, 2013 at 2:33 PM, Anthony PERARD
<anthony.perard@citrix.com> wrote:
> -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)
[snip]
>  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");

[snip]

> +static int libxl__set_vcpuonline_qmp(libxl__gc *gc, uint32_t domid,
> +                                     libxl_bitmap *cpumap)
> +{
> +    libxl_dominfo info;
> +    int i, rc = 0;
> +
> +    if (libxl_domain_info(CTX, &info, domid) < 0) {
> +        LOGE(ERROR, "getting domain info list");
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +    for (i = 0; i <= info.vcpu_max_id; i++) {
> +        if (libxl_bitmap_test(cpumap, i)) {
> +            rc = libxl__qmp_cpu_add(gc, domid, i);
> +        }

It looks like the xenstore version will both online and offline cpus
-- this seems to only add them.

If the qemu-traditional one really does successfully offline vcpus, I
don't think we can take this as-is, as it would mean that
libxl_set_vcpuonline() would have different functionality based on
what version of the device model you're using (and if we ever fix it
later, what version of the library you're using).

 -George

[PATCH V3] xen: Don't perform SMP setup.

[Anthony PERARD]

This is not needed and in case one want to start a guest with fewer vCPU
than the maximum, the function will fail (infinite loop).

It's a "backport" of 5dbf1732940f94771e0b3c45c0960940276bc263.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 src/smp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/smp.c b/src/smp.c
index 3c36f8c..742e371 100644
--- a/src/smp.c
+++ b/src/smp.c
@@ -84,6 +84,9 @@ int apic_id_is_present(u8 apic_id)
 void
 smp_probe(void)
 {
+    if (usingXen())
+        return;
+
     ASSERT32FLAT();
     u32 eax, ebx, ecx, cpuid_features;
     cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
-- 
Anthony PERARD

Re: [PATCH V3] xen: Don't perform SMP setup.

[Ian Campbell]

On Mon, 2013-06-24 at 14:35 +0100, Anthony PERARD wrote:
> This is not needed and in case one want to start a guest with fewer vCPU
> than the maximum, the function will fail (infinite loop).
> 
> It's a "backport" of 5dbf1732940f94771e0b3c45c0960940276bc263.

For me this causes:
src/smp.c: In function ‘smp_probe’:
src/smp.c:87: warning: implicit declaration of function ‘usingXen’
In file included from src/post.c:26:
src/xen.h: At top level:
src/xen.h:14: error: static declaration of ‘usingXen’ follows non-static declaration
src/smp.c:87: note: previous implicit declaration of ‘usingXen’ was here
(and a bunch more).

I think you need a #include?

> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  src/smp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/smp.c b/src/smp.c
> index 3c36f8c..742e371 100644
> --- a/src/smp.c
> +++ b/src/smp.c
> @@ -84,6 +84,9 @@ int apic_id_is_present(u8 apic_id)
>  void
>  smp_probe(void)
>  {
> +    if (usingXen())
> +        return;
> +
>      ASSERT32FLAT();
>      u32 eax, ebx, ecx, cpuid_features;
>      cpuid(1, &eax, &ebx, &ecx, &cpuid_features);



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

Re: [PATCH V3] xen: Don't perform SMP setup.

[Anthony PERARD]

On 26/06/13 17:24, Ian Campbell wrote:
> On Mon, 2013-06-24 at 14:35 +0100, Anthony PERARD wrote:
>> This is not needed and in case one want to start a guest with fewer vCPU
>> than the maximum, the function will fail (infinite loop).
>>
>> It's a "backport" of 5dbf1732940f94771e0b3c45c0960940276bc263.
> 
> For me this causes:
> src/smp.c: In function ‘smp_probe’:
> src/smp.c:87: warning: implicit declaration of function ‘usingXen’
> In file included from src/post.c:26:
> src/xen.h: At top level:
> src/xen.h:14: error: static declaration of ‘usingXen’ follows non-static declaration
> src/smp.c:87: note: previous implicit declaration of ‘usingXen’ was here
> (and a bunch more).
> 
> I think you need a #include?

Yes, I sent another patch that fix it:
<1372260645-26944-1-git-send-email-anthony.perard@citrix.com>

Sorry for that.

-- 
Anthony PERARD

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

Re: [PATCH V3] xen: Don't perform SMP setup.

[Ian Campbell]

On Wed, 2013-06-26 at 17:50 +0100, Anthony PERARD wrote:
> On 26/06/13 17:24, Ian Campbell wrote:
> > On Mon, 2013-06-24 at 14:35 +0100, Anthony PERARD wrote:
> >> This is not needed and in case one want to start a guest with fewer vCPU
> >> than the maximum, the function will fail (infinite loop).
> >>
> >> It's a "backport" of 5dbf1732940f94771e0b3c45c0960940276bc263.
> > 
> > For me this causes:
> > src/smp.c: In function ‘smp_probe’:
> > src/smp.c:87: warning: implicit declaration of function ‘usingXen’
> > In file included from src/post.c:26:
> > src/xen.h: At top level:
> > src/xen.h:14: error: static declaration of ‘usingXen’ follows non-static declaration
> > src/smp.c:87: note: previous implicit declaration of ‘usingXen’ was here
> > (and a bunch more).
> > 
> > I think you need a #include?
> 
> Yes, I sent another patch that fix it:
> <1372260645-26944-1-git-send-email-anthony.perard@citrix.com>

Got it, thanks.

> Sorry for that.





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