diff mbox series

[XEN,1/8] libxl: Replace deprecated QMP command by "query-cpus-fast"

Message ID 20210423161558.224367-2-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series Fix libxl with QEMU 6.0 + remove some more deprecated usages. | expand

Commit Message

Anthony PERARD April 23, 2021, 4:15 p.m. UTC 
We use the deprecated QMP command "query-cpus" which is removed in the
QEMU 6.0 release. There's a replacement which is "query-cpus-fast",
and have been available since QEMU 2.12 (April 2018).

This patch try the new command first and when the command isn't
available, it fall back to the deprecated one so libxl still works
with older QEMU versions.

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

Notes:
    This is v2 of '[XEN PATCH for-4.15] libxl: Replace deprecated QMP
    command by "query-cpus-fast"' as the patch never made it into the
    release.
    
    changes:
    - introduce a fallback for when the new command isn't available.

 tools/libs/light/libxl_domain.c | 103 ++++++++++++++++++++++++++++++--
 1 file changed, 98 insertions(+), 5 deletions(-)

Comments

Jason Andryuk April 28, 2021, 4:53 p.m. UTC | #1 
On Fri, Apr 23, 2021 at 12:16 PM Anthony PERARD
<anthony.perard@citrix.com> wrote:
>
> We use the deprecated QMP command "query-cpus" which is removed in the
> QEMU 6.0 release. There's a replacement which is "query-cpus-fast",
> and have been available since QEMU 2.12 (April 2018).
>
> This patch try the new command first and when the command isn't
> available, it fall back to the deprecated one so libxl still works
> with older QEMU versions.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>
> Notes:
>     This is v2 of '[XEN PATCH for-4.15] libxl: Replace deprecated QMP
>     command by "query-cpus-fast"' as the patch never made it into the
>     release.
>
>     changes:
>     - introduce a fallback for when the new command isn't available.
>
>  tools/libs/light/libxl_domain.c | 103 ++++++++++++++++++++++++++++++--
>  1 file changed, 98 insertions(+), 5 deletions(-)
>
> diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
> index 5d4ec9071160..8c003aa7cb04 100644
> --- a/tools/libs/light/libxl_domain.c
> +++ b/tools/libs/light/libxl_domain.c
> @@ -1740,6 +1740,35 @@ static int libxl__set_vcpuonline_xenstore(libxl__gc *gc, uint32_t domid,
>      return rc;
>  }
>
> +static int qmp_parse_query_cpus_fast(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-fast":
> +     * [ { 'cpu-index': '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-index", cpu, JSON_INTEGER);

Looks like qmp_parse_query_cpus_fast and qmp_parse_query_cpus just
differ by the key string.  So you could pass it in as an argument -
maybe with qmp_parse_query_cpus_fast and qmp_parse_query_cpus as
wrappers around a common implementation?

But if you prefer this separate function, it's fine.

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Anthony PERARD May 10, 2021, 2:11 p.m. UTC | #2 
On Wed, Apr 28, 2021 at 12:53:12PM -0400, Jason Andryuk wrote:
> On Fri, Apr 23, 2021 at 12:16 PM Anthony PERARD <anthony.perard@citrix.com> wrote:
> > +static int qmp_parse_query_cpus_fast(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-fast":
> > +     * [ { 'cpu-index': '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-index", cpu, JSON_INTEGER);
> 
> Looks like qmp_parse_query_cpus_fast and qmp_parse_query_cpus just
> differ by the key string.  So you could pass it in as an argument -
> maybe with qmp_parse_query_cpus_fast and qmp_parse_query_cpus as
> wrappers around a common implementation?
> 
> But if you prefer this separate function, it's fine.

I think it's better to have two different functions because we are
parsing two different commands, even if they are very similar.

> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

Thanks,
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 5d4ec9071160..8c003aa7cb04 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -1740,6 +1740,35 @@  static int libxl__set_vcpuonline_xenstore(libxl__gc *gc, uint32_t domid,
     return rc;
 }
 
+static int qmp_parse_query_cpus_fast(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-fast":
+     * [ { 'cpu-index': '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-index", 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;
+}
+
 static int qmp_parse_query_cpus(libxl__gc *gc,
                                 libxl_domid domid,
                                 const libxl__json_object *response,
@@ -1778,8 +1807,13 @@  typedef struct set_vcpuonline_state {
     int index; /* for loop on final_map */
 } set_vcpuonline_state;
 
+static void set_vcpuonline_qmp_cpus_fast_queried(libxl__egc *,
+    libxl__ev_qmp *, const libxl__json_object *, 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_query_cpus_parse(libxl__egc *,
+    libxl__ev_qmp *qmp, const libxl__json_object *,
+    bool query_cpus_fast, 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,
@@ -1840,8 +1874,8 @@  int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid,
                                              set_vcpuonline_timeout,
                                              LIBXL_QMP_CMD_TIMEOUT * 1000);
             if (rc) goto out;
-            qmp->callback = set_vcpuonline_qmp_cpus_queried;
-            rc = libxl__ev_qmp_send(egc, qmp, "query-cpus", NULL);
+            qmp->callback = set_vcpuonline_qmp_cpus_fast_queried;
+            rc = libxl__ev_qmp_send(egc, qmp, "query-cpus-fast", NULL);
             if (rc) goto out;
             return AO_INPROGRESS;
         default:
@@ -1860,11 +1894,39 @@  int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid,
     return AO_INPROGRESS;
 }
 
+static void set_vcpuonline_qmp_cpus_fast_queried(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    set_vcpuonline_state *svos = CONTAINER_OF(qmp, *svos, qmp);
+
+    if (rc == ERROR_QMP_COMMAND_NOT_FOUND) {
+        /* Try again, we probably talking to a QEMU older than 2.12 */
+        qmp->callback = set_vcpuonline_qmp_cpus_queried;
+        rc = libxl__ev_qmp_send(egc, qmp, "query-cpus", NULL);
+        if (rc) goto out;
+        return;
+    }
+
+out:
+    set_vcpuonline_qmp_query_cpus_parse(egc, qmp, response, true, rc);
+}
+
 static void set_vcpuonline_qmp_cpus_queried(libxl__egc *egc,
     libxl__ev_qmp *qmp, const libxl__json_object *response, int rc)
 {
     EGC_GC;
     set_vcpuonline_state *svos = CONTAINER_OF(qmp, *svos, qmp);
+
+    set_vcpuonline_qmp_query_cpus_parse(egc, qmp, response, false, rc);
+}
+
+static void set_vcpuonline_qmp_query_cpus_parse(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response,
+    bool query_cpus_fast, int rc)
+{
+    EGC_GC;
+    set_vcpuonline_state *svos = CONTAINER_OF(qmp, *svos, qmp);
     int i;
     libxl_bitmap current_map;
 
@@ -1876,7 +1938,11 @@  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);
-    rc = qmp_parse_query_cpus(gc, qmp->domid, response, &current_map);
+    if (query_cpus_fast) {
+        rc = qmp_parse_query_cpus_fast(gc, qmp->domid, response, &current_map);
+    } else {
+        rc = qmp_parse_query_cpus(gc, qmp->domid, response, &current_map);
+    }
     if (rc) goto out;
 
     libxl_bitmap_copy_alloc(CTX, final_map, svos->cpumap);
@@ -2121,6 +2187,9 @@  typedef struct {
 
 static void retrieve_domain_configuration_lock_acquired(
     libxl__egc *egc, libxl__ev_slowlock *, int rc);
+static void retrieve_domain_configuration_cpu_fast_queried(
+    libxl__egc *egc, libxl__ev_qmp *qmp,
+    const libxl__json_object *response, int rc);
 static void retrieve_domain_configuration_cpu_queried(
     libxl__egc *egc, libxl__ev_qmp *qmp,
     const libxl__json_object *response, int rc);
@@ -2198,8 +2267,8 @@  static void retrieve_domain_configuration_lock_acquired(
         if (rc) goto out;
         libxl_bitmap_alloc(CTX, &rdcs->qemuu_cpus,
                            d_config->b_info.max_vcpus);
-        rdcs->qmp.callback = retrieve_domain_configuration_cpu_queried;
-        rc = libxl__ev_qmp_send(egc, &rdcs->qmp, "query-cpus", NULL);
+        rdcs->qmp.callback = retrieve_domain_configuration_cpu_fast_queried;
+        rc = libxl__ev_qmp_send(egc, &rdcs->qmp, "query-cpus-fast", NULL);
         if (rc) goto out;
         has_callback = true;
     }
@@ -2210,6 +2279,30 @@  static void retrieve_domain_configuration_lock_acquired(
         retrieve_domain_configuration_end(egc, rdcs, rc);
 }
 
+static void retrieve_domain_configuration_cpu_fast_queried(
+    libxl__egc *egc, libxl__ev_qmp *qmp,
+    const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    retrieve_domain_configuration_state *rdcs =
+        CONTAINER_OF(qmp, *rdcs, qmp);
+
+    if (rc == ERROR_QMP_COMMAND_NOT_FOUND) {
+        /* Try again, we probably talking to a QEMU older than 2.12 */
+        rdcs->qmp.callback = retrieve_domain_configuration_cpu_queried;
+        rc = libxl__ev_qmp_send(egc, &rdcs->qmp, "query-cpus", NULL);
+        if (rc) goto out;
+        return;
+    }
+
+    if (rc) goto out;
+
+    rc = qmp_parse_query_cpus_fast(gc, qmp->domid, response, &rdcs->qemuu_cpus);
+
+out:
+    retrieve_domain_configuration_end(egc, rdcs, rc);
+}
+
 static void retrieve_domain_configuration_cpu_queried(
     libxl__egc *egc, libxl__ev_qmp *qmp,
     const libxl__json_object *response, int rc)