diff mbox

[v3,5/5] libxl: only issue cpu-add call to QEMU for not present CPU

Message ID 1465983102-19308-6-git-send-email-wei.liu2@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Liu June 15, 2016, 9:31 a.m. UTC
Calculate the final bitmap for CPUs to add to avoid having annoying
error messages complaining those CPUs are already present.

We can also properly handle error from QMP now.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>

v3:
1. Add Anthony's Reviewed-by tag.
---
 tools/libxl/libxl.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

Comments

Ian Jackson July 8, 2016, 5:48 p.m. UTC | #1
Wei Liu writes ("[PATCH v3 5/5] libxl: only issue cpu-add call to QEMU for not present CPU"):
> Calculate the final bitmap for CPUs to add to avoid having annoying
> error messages complaining those CPUs are already present.

Can this be expanded a bit ?  I think perhaps it would benefit from
"When X and Y and Z, qemu prints an annoying message about Q".

I think I have understood it by reverse engineering the code but I
would still like a better commit message.

Thanks,
Ian.
Wei Liu July 11, 2016, 11:32 a.m. UTC | #2
On Fri, Jul 08, 2016 at 06:48:27PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v3 5/5] libxl: only issue cpu-add call to QEMU for not present CPU"):
> > Calculate the final bitmap for CPUs to add to avoid having annoying
> > error messages complaining those CPUs are already present.
> 
> Can this be expanded a bit ?  I think perhaps it would benefit from
> "When X and Y and Z, qemu prints an annoying message about Q".
> 
> I think I have understood it by reverse engineering the code but I
> would still like a better commit message.

Sure. I can paste in the error message into commit log.

Wei.

> 
> Thanks,
> Ian.
diff mbox

Patch

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b748bf1..0e2c15a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5753,19 +5753,38 @@  static int libxl__set_vcpuonline_qmp(libxl__gc *gc, uint32_t domid,
                                      libxl_bitmap *cpumap,
                                      const libxl_dominfo *info)
 {
-    int i;
+    int i, rc;
+    libxl_bitmap current_map, final_map;
+
+    libxl_bitmap_init(&current_map);
+    libxl_bitmap_init(&final_map);
+
+    libxl_bitmap_alloc(CTX, &current_map, info->vcpu_max_id + 1);
+    libxl_bitmap_set_none(&current_map);
+    rc = libxl__qmp_query_cpus(gc, domid, &current_map);
+    if (rc) {
+        LOG(ERROR, "failed to query cpus for domain %d", domid);
+        goto out;
+    }
+
+    libxl_bitmap_copy_alloc(CTX, &final_map, cpumap);
 
-    for (i = 0; i <= info->vcpu_max_id; i++) {
-        if (libxl_bitmap_test(cpumap, i)) {
-            /* Return value is ignore because it does not tell anything useful
-             * on the completion of the command.
-             * (For instance, "CPU already plugged-in" give the same return
-             * value as "command not supported".)
-             */
-            libxl__qmp_cpu_add(gc, domid, i);
+    libxl_for_each_set_bit(i, current_map)
+        libxl_bitmap_reset(&final_map, i);
+
+    libxl_for_each_set_bit(i, final_map) {
+        rc = libxl__qmp_cpu_add(gc, domid, i);
+        if (rc) {
+            LOG(ERROR, "failed to add cpu %d to domain %d", i, domid);
+            goto out;
         }
     }
-    return 0;
+
+    rc = 0;
+out:
+    libxl_bitmap_dispose(&current_map);
+    libxl_bitmap_dispose(&final_map);
+    return rc;
 }
 
 int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)