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
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
Anthony PERARD
2013-Jun-24 13:33 UTC
[PATCH v3 2/2] libxl: Use QMP cpu-add to hotplug CPU with qemu-xen.
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
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
George Dunlap
2013-Jun-24 15:57 UTC
Re: [PATCH v3 2/2] libxl: Use QMP cpu-add to hotplug CPU with qemu-xen.
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
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
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
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
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