[33/35] libxl: Extract qmp_parse_query_cpus

Commit Message

Anthony PERARD Aug. 2, 2019, 3:36 p.m. UTC

The QMP command "query-cpus" is called from different places, extract
the algorithm that parse the answer into a separate function.

Signed-off-by: Anthony PERARD <[e-mail]>
---
 tools/libxl/libxl_domain.c | 69 ++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 29 deletions(-)

Comments

Ian Jackson Sept. 17, 2019, 5:42 p.m. UTC | #1

Anthony PERARD writes ("[PATCH 33/35] libxl: Extract qmp_parse_query_cpus"):
> The QMP command "query-cpus" is called from different places, extract
> the algorithm that parse the answer into a separate function.

I hope you meant to write "No functional change."

If so, then with that added to the commit message:

Acked-by: Ian Jackson <[e-mail]>

Thanks,
Ian.

Anthony PERARD Sept. 19, 2019, 2:46 p.m. UTC | #2

On Tue, Sep 17, 2019 at 06:42:10PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH 33/35] libxl: Extract qmp_parse_query_cpus"):
> > The QMP command "query-cpus" is called from different places, extract
> > the algorithm that parse the answer into a separate function.
> 
> I hope you meant to write "No functional change."

It was supposed to be that way, but part of the changes in this patch
should be in the previous patch. It also fix the previous patch which
won't work on it's own.

I think I'm going to squash both patch together and leave a note in the
commit description as to why the function qmp_parse_query_cpus exist,
that is:
    libxl: Use ev_qmp in libxl_set_vcpuonline
    libxl: Extract qmp_parse_query_cpus

Ian Jackson Sept. 19, 2019, 4:17 p.m. UTC | #3

Anthony PERARD writes ("Re: [PATCH 33/35] libxl: Extract qmp_parse_query_cpus"):
> On Tue, Sep 17, 2019 at 06:42:10PM +0100, Ian Jackson wrote:
> > Anthony PERARD writes ("[PATCH 33/35] libxl: Extract qmp_parse_query_cpus"):
> > > The QMP command "query-cpus" is called from different places, extract
> > > the algorithm that parse the answer into a separate function.
> > 
> > I hope you meant to write "No functional change."
> 
> It was supposed to be that way, but part of the changes in this patch
> should be in the previous patch. It also fix the previous patch which
> won't work on it's own.
> 
> I think I'm going to squash both patch together and leave a note in the
> commit description as to why the function qmp_parse_query_cpus exist,
> that is:
>     libxl: Use ev_qmp in libxl_set_vcpuonline
>     libxl: Extract qmp_parse_query_cpus

OK

Patch

diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 4726e27d20..b97e874a9c 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1457,6 +1457,35 @@  static int libxl__set_vcpuonline_xenstore(libxl__gc *gc, uint32_t domid,
     return rc;
 }
 
+static int qmp_parse_query_cpus(libxl__gc *gc,
+                                libxl_domid domid,
+                                const libxl__json_object *response,
+                                libxl_bitmap *const map)
+{
+    int i;
+    const libxl__json_object *cpu;
+
+    libxl_bitmap_set_none(map);
+    /* Parse response to QMP command "query-cpus":
+     * [ { 'CPU': 'int',...} ]
+     */
+    for (i = 0; (cpu = libxl__json_array_get(response, i)); i++) {
+        unsigned int cpu_index;
+        const libxl__json_object *o;
+
+        o = libxl__json_map_get("CPU", cpu, JSON_INTEGER);
+        if (!o) {
+            LOGD(ERROR, domid, "Failed to retrieve CPU index.");
+            return ERROR_QEMU_API;
+        }
+
+        cpu_index = libxl__json_object_get_integer(o);
+        libxl_bitmap_set(map, cpu_index);
+    }
+
+    return 0;
+}
+
 typedef struct set_vcpuonline_state {
     libxl__ev_qmp qmp;
     libxl__ev_time timeout;
@@ -1466,18 +1495,14 @@  typedef struct set_vcpuonline_state {
     int index; /* for loop on final_map */
 } set_vcpuonline_state;
 
-static void set_vcpuonline_qmp_cpus_queried(libxl__egc *, libxl__ev_qmp *,
-                                            const libxl__json_object *,
-                                            int rc);
-static void set_vcpuonline_qmp_add_cpu(libxl__egc *, libxl__ev_qmp *,
-                                       const libxl__json_object *response,
-                                       int rc);
-static void set_vcpuonline_timeout(libxl__egc *egc, libxl__ev_time *ev,
-                                   const struct timeval *requested_abs,
-                                   int rc);
+static void set_vcpuonline_qmp_cpus_queried(libxl__egc *,
+    libxl__ev_qmp *, const libxl__json_object *, int rc);
+static void set_vcpuonline_qmp_add_cpu(libxl__egc *,
+    libxl__ev_qmp *, const libxl__json_object *response, int rc);
+static void set_vcpuonline_timeout(libxl__egc *egc,
+    libxl__ev_time *ev, const struct timeval *requested_abs, int rc);
 static void set_vcpuonline_done(libxl__egc *egc,
-                                set_vcpuonline_state *svos,
-                                int rc);
+    set_vcpuonline_state *svos, int rc);
 
 int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid,
                          libxl_bitmap *cpumap,
@@ -1493,6 +1518,7 @@  int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid,
     svos->qmp.domid = domid;
     svos->qmp.payload_fd = -1;
     libxl__ev_time_init(&svos->timeout);
+    svos->cpumap = cpumap;
     libxl_dominfo_init(&svos->info);
     libxl_bitmap_init(&svos->final_map);
 
@@ -1556,7 +1582,6 @@  static void set_vcpuonline_qmp_cpus_queried(libxl__egc *egc,
 {
     EGC_GC;
     set_vcpuonline_state *svos = CONTAINER_OF(qmp, *svos, qmp);
-    const libxl__json_object *cpu;
     int i;
     libxl_bitmap current_map;
 
@@ -1568,23 +1593,8 @@  static void set_vcpuonline_qmp_cpus_queried(libxl__egc *egc,
     if (rc) goto out;
 
     libxl_bitmap_alloc(CTX, &current_map, svos->info.vcpu_max_id + 1);
-    libxl_bitmap_set_none(&current_map);
-
-    for (i = 0; (cpu = libxl__json_array_get(response, i)); i++) {
-        unsigned int cpu_index;
-        const libxl__json_object *o;
-
-        o = libxl__json_map_get("CPU", cpu, JSON_INTEGER);
-        if (!o) {
-            LOGD(ERROR, qmp->domid, "Failed to retrieve CPU index.");
-            rc = ERROR_QEMU_API;
-            goto out;
-        }
-
-        cpu_index = libxl__json_object_get_integer(o);
-        libxl_bitmap_set(&current_map, cpu_index);
-    }
-
+    rc = qmp_parse_query_cpus(gc, qmp->domid, response, &current_map);
+    if (rc) goto out;
 
     libxl_bitmap_copy_alloc(CTX, final_map, svos->cpumap);
 
@@ -1653,6 +1663,7 @@  static void set_vcpuonline_done(libxl__egc *egc,
 
     libxl_bitmap_dispose(&svos->final_map);
     libxl_dominfo_dispose(&svos->info);
+    libxl__ev_time_deregister(gc, &svos->timeout);
     libxl__ev_qmp_dispose(gc, &svos->qmp);
     libxl__ao_complete(egc, ao, rc);
 }