[XEN PATCH v2 0/8] Fix libxl with QEMU 6.0 + remove some more deprecated usages.

[XEN PATCH v2 0/8] Fix libxl with QEMU 6.0 + remove some more deprecated usages.

[Anthony PERARD]

Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.deprecated-qemu-qmp-and-cmd-v2

v2:
- fix coding style in patch 3
- all reviewed

The Xen 4.15 release that went out just before QEMU 6.0 won't be compaptible
with the latter. This patch series fixes libxl to replace use of QMP commands
that have been removed from QEMU and to fix usage of deprecated command and
parameters that well be remove from QEMU in the future.

All of the series should be backported to at least Xen 4.15 or it won't be
possible to migrate, hotplug cpu or change cdrom on HVM guest when QEMU 6.0 and
newer is used. QEMU 6.0 is about to be release, within a week.

Backport: 4.15

Anthony PERARD (8):
  libxl: Replace deprecated QMP command by "query-cpus-fast"
  libxl: Replace QEMU's command line short-form boolean option
  libxl: Replace deprecated "cpu-add" QMP command by "device_add"
  libxl: Use -device for cd-rom drives
  libxl: Assert qmp_ev's state in qmp_ev_qemu_compare_version
  libxl: Export libxl__qmp_ev_qemu_compare_version
  libxl: Use `id` with the "eject" QMP command
  libxl: Replace QMP command "change" by "blockdev-change-media"

 tools/libs/light/libxl_disk.c     |  67 +++++++++--
 tools/libs/light/libxl_dm.c       |  30 +++--
 tools/libs/light/libxl_domain.c   | 192 ++++++++++++++++++++++++++++--
 tools/libs/light/libxl_internal.h |   8 ++
 tools/libs/light/libxl_qmp.c      |   6 +-
 5 files changed, 272 insertions(+), 31 deletions(-)

-- 
Anthony PERARD

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

[Anthony PERARD]

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>
Reviewed-by: Jason Andryuk <jandryuk@gmail.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);
+        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)
-- 
Anthony PERARD

[XEN PATCH v2 2/8] libxl: Replace QEMU's command line short-form boolean option

[Anthony PERARD]

Short-form boolean options are deprecated in QEMU 6.0.
Upstream commit that deprecate those: ccd3b3b8112b ("qemu-option: warn
for short-form boolean options").

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libs/light/libxl_dm.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 3599a82ef01b..0a0c1ef7c62e 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -977,14 +977,14 @@ static char *dm_spice_options(libxl__gc *gc,
     if (spice->host)
         opt = GCSPRINTF("%s,addr=%s", opt, spice->host);
     if (libxl_defbool_val(spice->disable_ticketing))
-        opt = GCSPRINTF("%s,disable-ticketing", opt);
+        opt = GCSPRINTF("%s,disable-ticketing=on", opt);
     else
         opt = GCSPRINTF("%s,password=%s", opt, spice->passwd);
     opt = GCSPRINTF("%s,agent-mouse=%s", opt,
                     libxl_defbool_val(spice->agent_mouse) ? "on" : "off");
 
     if (!libxl_defbool_val(spice->clipboard_sharing))
-        opt = GCSPRINTF("%s,disable-copy-paste", opt);
+        opt = GCSPRINTF("%s,disable-copy-paste=on", opt);
 
     if (spice->image_compression)
         opt = GCSPRINTF("%s,image-compression=%s", opt,
@@ -1224,7 +1224,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
         flexarray_append(dm_args, "-chardev");
         if (state->dm_monitor_fd >= 0) {
             flexarray_append(dm_args,
-                GCSPRINTF("socket,id=libxl-cmd,fd=%d,server,nowait",
+                GCSPRINTF("socket,id=libxl-cmd,fd=%d,server=on,wait=off",
                           state->dm_monitor_fd));
 
             /*
@@ -1237,7 +1237,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
         } else {
             flexarray_append(dm_args,
                              GCSPRINTF("socket,id=libxl-cmd,"
-                                       "path=%s,server,nowait",
+                                       "path=%s,server=on,wait=off",
                                        libxl__qemu_qmp_path(gc, guest_domid)));
         }
 
@@ -1247,7 +1247,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
         flexarray_append(dm_args, "-chardev");
         flexarray_append(dm_args,
                          GCSPRINTF("socket,id=libxenstat-cmd,"
-                                        "path=%s/qmp-libxenstat-%d,server,nowait",
+                                        "path=%s/qmp-libxenstat-%d,server=on,wait=off",
                                         libxl__run_dir_path(), guest_domid));
 
         flexarray_append(dm_args, "-mon");
@@ -1264,7 +1264,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
             case LIBXL_CHANNEL_CONNECTION_SOCKET:
                 path = guest_config->channels[i].u.socket.path;
                 chardev = GCSPRINTF("socket,id=libxl-channel%d,path=%s,"
-                                    "server,nowait", devid, path);
+                                    "server=on,wait=off", devid, path);
                 break;
             default:
                 /* We've forgotten to add the clause */
@@ -1577,7 +1577,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
         nics[i].colo_##sock_port) {                                         \
         flexarray_append(dm_args, "-chardev");                              \
         flexarray_append(dm_args,                                           \
-            GCSPRINTF("socket,id=%s,host=%s,port=%s,server,nowait",         \
+            GCSPRINTF("socket,id=%s,host=%s,port=%s,server=on,wait=off",    \
                       nics[i].colo_##sock_id,                               \
                       nics[i].colo_##sock_ip,                               \
                       nics[i].colo_##sock_port));                           \
-- 
Anthony PERARD

[XEN PATCH v2 3/8] libxl: Replace deprecated "cpu-add" QMP command by "device_add"

[Anthony PERARD]

The command "cpu-add" for CPU hotplug is deprecated and has been
removed from QEMU 6.0 (April 2021). We need to add cpus with the
command "device_add" now.

In order to find out which parameters to pass to "device_add" we first
make a call to "query-hotpluggable-cpus" which list the cpus drivers
and properties.

The algorithm to figure out which CPU to add, and by extension if any
CPU needs to be hotplugged, is in the function that adds the cpus.
Because of that, the command "query-hotpluggable-cpus" is always
called, even when not needed.

In case we are using a version of QEMU older than 2.7 (Sept 2016)
which don't have "query-hotpluggable-cpus", we fallback to using
"cpu-add".

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---

Notes:
    v2:
    - fix coding style

 tools/libs/light/libxl_domain.c | 89 ++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 8c003aa7cb04..c00c36c92879 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -1805,6 +1805,7 @@ typedef struct set_vcpuonline_state {
     libxl_dominfo info;
     libxl_bitmap final_map;
     int index; /* for loop on final_map */
+    const char *cpu_driver;
 } set_vcpuonline_state;
 
 static void set_vcpuonline_qmp_cpus_fast_queried(libxl__egc *,
@@ -1814,6 +1815,10 @@ static void set_vcpuonline_qmp_cpus_queried(libxl__egc *,
 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_query_hotpluggable_cpus(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc);
+static void set_vcpuonline_qmp_device_add_cpu(libxl__egc *,
+    libxl__ev_qmp *, const libxl__json_object *response, 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,
@@ -1951,13 +1956,54 @@ static void set_vcpuonline_qmp_query_cpus_parse(libxl__egc *egc,
         libxl_bitmap_reset(final_map, i);
     }
 
+    qmp->callback = set_vcpuonline_qmp_query_hotpluggable_cpus;
+    rc = libxl__ev_qmp_send(egc, qmp, "query-hotpluggable-cpus", NULL);
+
 out:
     libxl_bitmap_dispose(&current_map);
+    if (rc)
+        set_vcpuonline_done(egc, svos, rc); /* must be last */
+}
+
+static void set_vcpuonline_qmp_query_hotpluggable_cpus(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc)
+{
+    set_vcpuonline_state *svos = CONTAINER_OF(qmp, *svos, qmp);
+    const libxl__json_object *cpu;
+    const libxl__json_object *cpu_driver;
+
+    if (rc == ERROR_QMP_COMMAND_NOT_FOUND) {
+        /* We are probably connected to a version of QEMU older than 2.7,
+         * let's fallback to using "cpu-add" command. */
+        svos->index = -1;
+        set_vcpuonline_qmp_add_cpu(egc, qmp, NULL, 0); /* must be last */
+        return;
+    }
+
+    if (rc) goto out;
+
+    /* Parse response to QMP command "query-hotpluggable-cpus"
+     * [ { 'type': 'str', ... ]
+     *
+     * We are looking for the driver name for CPU to be hotplug. We'll
+     * assume that cpus property are core-id=0, thread-id=0 and
+     * socket-id=$cpu_index, as we start qemu with "-smp %d,maxcpus=%d", so
+     * we don't parse the properties listed for each hotpluggable cpus.
+     */
+
+    cpu = libxl__json_array_get(response, 0);
+    cpu_driver = libxl__json_map_get("type", cpu, JSON_STRING);
+    svos->cpu_driver = libxl__json_object_get_string(cpu_driver);
+
+    if (!svos->cpu_driver)
+        rc = ERROR_QEMU_API;
+
+out:
     svos->index = -1;
-    set_vcpuonline_qmp_add_cpu(egc, qmp, NULL, rc); /* must be last */
+    set_vcpuonline_qmp_device_add_cpu(egc, qmp, NULL, rc); /* must be last */
 }
 
-static void set_vcpuonline_qmp_add_cpu(libxl__egc *egc,
+static void set_vcpuonline_qmp_device_add_cpu(libxl__egc *egc,
     libxl__ev_qmp *qmp, const libxl__json_object *response, int rc)
 {
     STATE_AO_GC(qmp->ao);
@@ -1969,6 +2015,45 @@ static void set_vcpuonline_qmp_add_cpu(libxl__egc *egc,
 
     if (rc) goto out;
 
+    while (libxl_bitmap_cpu_valid(map, ++svos->index)) {
+        if (libxl_bitmap_test(map, svos->index)) {
+            qmp->callback = set_vcpuonline_qmp_device_add_cpu;
+            libxl__qmp_param_add_string(gc, &args, "id", GCSPRINTF("cpu-%d", svos->index));
+            libxl__qmp_param_add_string(gc, &args, "driver", svos->cpu_driver);
+            /* We'll assume that we start QEMU with -smp %d,maxcpus=%d, so
+             * that "core-id" and "thread-id" are always 0 so that
+             * "socket-id" correspond the cpu index.
+             * Those properties are otherwise listed by
+             * "query-hotpluggable-cpus". */
+            libxl__qmp_param_add_integer(gc, &args, "socket-id", svos->index);
+            libxl__qmp_param_add_integer(gc, &args, "core-id", 0);
+            libxl__qmp_param_add_integer(gc, &args, "thread-id", 0);
+            rc = libxl__ev_qmp_send(egc, qmp, "device_add", args);
+            if (rc) goto out;
+            return;
+        }
+    }
+
+out:
+    set_vcpuonline_done(egc, svos, rc);
+}
+
+/* Fallback function for QEMU older than 2.7, when
+ * 'query-hotpluggable-cpus' wasn't available and vcpu object couldn't be
+ * added with 'device_add'. */
+static void set_vcpuonline_qmp_add_cpu(libxl__egc *egc, libxl__ev_qmp *qmp,
+                                       const libxl__json_object *response,
+                                       int rc)
+{
+    STATE_AO_GC(qmp->ao);
+    set_vcpuonline_state *svos = CONTAINER_OF(qmp, *svos, qmp);
+    libxl__json_object *args = NULL;
+
+    /* Convenience aliases */
+    libxl_bitmap *map = &svos->final_map;
+
+    if (rc) goto out;
+
     while (libxl_bitmap_cpu_valid(map, ++svos->index)) {
         if (libxl_bitmap_test(map, svos->index)) {
             qmp->callback = set_vcpuonline_qmp_add_cpu;
-- 
Anthony PERARD

[XEN PATCH v2 4/8] libxl: Use -device for cd-rom drives

[Anthony PERARD]

This allows to set an `id` on the device instead of only the drive. We
are going to need the `id` with the "eject" and
"blockdev-change-media" QMP command as using `device` parameter on
those is deprecated. (`device` is the `id` of the `-drive` on the
command line).

We set the same `id` on both -device and -drive as QEMU doesn't
complain and we can then either do "eject id=$id" or "eject
device=$id".

Using "-drive + -device" instead of only "-drive" has been
available since at least QEMU 0.15, and seems to be the preferred way as it
separates the host part (-drive which describe the disk image location
and format) from the guest part (-device which describe the emulated
device). More information in qemu.git/docs/qdev-device-use.txt .

Changing the command line during migration for the cdrom seems fine.
Also the documentation about migration in QEMU explains that the device
state ID is "been formed from a bus name and device address", so
second IDE bus and first device address on bus is still thus and
doesn't matter if written "-drive if=ide,index=2" or "-drive
ide-cd,bus=ide.1,unit=0".
See qemu.git/docs/devel/migration.rst .

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libs/light/libxl_dm.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 0a0c1ef7c62e..5b01cf284163 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -1913,6 +1913,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
             }
 
             if (disks[i].is_cdrom) {
+                const char *drive_id;
                 if (disk > 4) {
                     LOGD(WARN, guest_domid, "Emulated CDROM can be only one of the first 4 disks.\n"
                          "Disk %s will be available via PV drivers but not as an "
@@ -1920,13 +1921,22 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                          disks[i].vdev);
                     continue;
                 }
-                drive = libxl__sprintf(gc,
-                         "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
-                         disk, dev_number);
+
+                drive_id = GCSPRINTF("ide-%i", dev_number);
+                drive = GCSPRINTF("if=none,readonly=on,id=%s", drive_id);
 
                 if (target_path)
                     drive = libxl__sprintf(gc, "%s,file=%s,format=%s",
                                            drive, target_path, format);
+
+                flexarray_vappend(dm_args,
+                    "-drive", drive,
+                    "-device",
+                    GCSPRINTF("ide-cd,id=%s,drive=%s,bus=ide.%u,unit=%u",
+                              drive_id, drive_id,
+                              disk / 2, disk % 2),
+                    NULL);
+                continue;
             } else {
                 /*
                  * Explicit sd disks are passed through as is.
-- 
Anthony PERARD

[XEN PATCH v2 5/8] libxl: Assert qmp_ev's state in qmp_ev_qemu_compare_version

[Anthony PERARD]

We are supposed to read the version information only when qmp_ev is in
state "Connected" (that correspond to state==qmp_state_connected),
assert it so that the function isn't used too early.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libs/light/libxl_qmp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libs/light/libxl_qmp.c b/tools/libs/light/libxl_qmp.c
index 9b638e6f5442..d0967c9f029f 100644
--- a/tools/libs/light/libxl_qmp.c
+++ b/tools/libs/light/libxl_qmp.c
@@ -292,6 +292,8 @@ static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
 static int qmp_ev_qemu_compare_version(libxl__ev_qmp *ev, int major,
                                        int minor, int micro)
 {
+    assert(ev->state == qmp_state_connected);
+
 #define CHECK_VERSION(level) do { \
     if (ev->qemu_version.level > (level)) return +1; \
     if (ev->qemu_version.level < (level)) return -1; \
-- 
Anthony PERARD

[XEN PATCH v2 6/8] libxl: Export libxl__qmp_ev_qemu_compare_version

[Anthony PERARD]

We are going to want to check QEMU's version in other places where we
can use libxl__ev_qmp_send.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libs/light/libxl_internal.h | 8 ++++++++
 tools/libs/light/libxl_qmp.c      | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 44a2f3c8fe3b..0b4671318c82 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -492,6 +492,14 @@ _hidden int libxl__ev_qmp_send(libxl__egc *egc, libxl__ev_qmp *ev,
                                const char *cmd, libxl__json_object *args);
 _hidden void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev);
 
+/* return values:
+ *   < 0  if qemu's version <  asked version
+ *   = 0  if qemu's version == asked version
+ *   > 0  if qemu's version >  asked version
+ */
+_hidden int libxl__qmp_ev_qemu_compare_version(libxl__ev_qmp *ev, int major,
+                                               int minor, int micro);
+
 typedef enum {
     /* initial state */
     qmp_state_disconnected = 1,
diff --git a/tools/libs/light/libxl_qmp.c b/tools/libs/light/libxl_qmp.c
index d0967c9f029f..fb146a54cb9c 100644
--- a/tools/libs/light/libxl_qmp.c
+++ b/tools/libs/light/libxl_qmp.c
@@ -289,7 +289,7 @@ static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
  *   = 0  if qemu's version == asked version
  *   > 0  if qemu's version >  asked version
  */
-static int qmp_ev_qemu_compare_version(libxl__ev_qmp *ev, int major,
+int libxl__qmp_ev_qemu_compare_version(libxl__ev_qmp *ev, int major,
                                        int minor, int micro)
 {
     assert(ev->state == qmp_state_connected);
@@ -1073,7 +1073,7 @@ static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int fdset
     /* The `live` parameter was added to QEMU 2.11. It signals QEMU that
      * the save operation is for a live migration rather than for taking a
      * snapshot. */
-    if (qmp_ev_qemu_compare_version(ev, 2, 11, 0) >= 0)
+    if (libxl__qmp_ev_qemu_compare_version(ev, 2, 11, 0) >= 0)
         libxl__qmp_param_add_bool(gc, &args, "live", dsps->live);
     QMP_PARAMETERS_SPRINTF(&args, "filename", "/dev/fdset/%d", fdset);
     rc = libxl__ev_qmp_send(egc, ev, "xen-save-devices-state", args);
-- 
Anthony PERARD

[XEN PATCH v2 7/8] libxl: Use `id` with the "eject" QMP command

[Anthony PERARD]

`device` parameter is deprecated since QEMU 2.8.

This requires changes to the command line introduced by:
    "libxl: Use -device for cd-rom drives"

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libs/light/libxl_disk.c | 43 +++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
index 411ffeaca6ce..faabdea7a4c3 100644
--- a/tools/libs/light/libxl_disk.c
+++ b/tools/libs/light/libxl_disk.c
@@ -656,6 +656,8 @@ typedef struct {
 
 static void cdrom_insert_lock_acquired(libxl__egc *, libxl__ev_slowlock *,
                                        int rc);
+static void cdrom_insert_qmp_connected(libxl__egc *, libxl__ev_qmp *,
+                                       const libxl__json_object *, int rc);
 static void cdrom_insert_ejected(libxl__egc *egc, libxl__ev_qmp *,
                                  const libxl__json_object *, int rc);
 static void cdrom_insert_addfd_cb(libxl__egc *egc, libxl__ev_qmp *,
@@ -770,13 +772,12 @@ static void cdrom_insert_lock_acquired(libxl__egc *egc,
      */
 
     if (cis->dm_ver == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
-        libxl__json_object *args = NULL;
-        int devid = libxl__device_disk_dev_number(cis->disk->vdev,
-                                                  NULL, NULL);
-
-        QMP_PARAMETERS_SPRINTF(&args, "device", "ide-%i", devid);
-        cis->qmp.callback = cdrom_insert_ejected;
-        rc = libxl__ev_qmp_send(egc, &cis->qmp, "eject", args);
+        /* Before running the "eject" command, we need to know QEMU's
+         * version to find out which command to issue.
+         * cis->qmp isn't in Connected state yet, so run a dummy command
+         * to have QEMU's version available. */
+        cis->qmp.callback = cdrom_insert_qmp_connected;
+        rc = libxl__ev_qmp_send(egc, &cis->qmp, "query-version", NULL);
         if (rc) goto out;
     } else {
         cdrom_insert_ejected(egc, &cis->qmp, NULL, 0); /* must be last */
@@ -787,6 +788,34 @@ static void cdrom_insert_lock_acquired(libxl__egc *egc,
     cdrom_insert_done(egc, cis, rc); /* must be last */
 }
 
+static void cdrom_insert_qmp_connected(libxl__egc *egc, libxl__ev_qmp *qmp,
+                                       const libxl__json_object *response,
+                                       int rc)
+{
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
+    STATE_AO_GC(cis->ao);
+    libxl__json_object *args = NULL;
+    int devid = libxl__device_disk_dev_number(cis->disk->vdev,
+                                              NULL, NULL);
+
+    if (rc) goto out;
+
+    /* Using `device` parameter is deprecated since QEMU 2.8, we should
+     * use `id` now. They both have different meaning but we set the
+     * same `id` on -drive and -device on the command line.
+     */
+    if (libxl__qmp_ev_qemu_compare_version(qmp, 2, 8, 0) >= 0)
+        QMP_PARAMETERS_SPRINTF(&args, "id", "ide-%i", devid);
+    else
+        QMP_PARAMETERS_SPRINTF(&args, "device", "ide-%i", devid);
+    qmp->callback = cdrom_insert_ejected;
+    rc = libxl__ev_qmp_send(egc, qmp, "eject", args);
+    if (rc) goto out;
+    return;
+out:
+    cdrom_insert_done(egc, cis, rc); /* must be last */
+}
+
 static void cdrom_insert_ejected(libxl__egc *egc,
                                  libxl__ev_qmp *qmp,
                                  const libxl__json_object *response,
-- 
Anthony PERARD

[XEN PATCH v2 8/8] libxl: Replace QMP command "change" by "blockdev-change-media"

[Anthony PERARD]

"change" command as been removed in QEMU 6.0. We can use
"blockdev-change-medium" instead.

Using `id` with "blockdev-change-medium" requires a change to the QEMU
command line, introduced by:
    "libxl: Use -device for cd-rom drives"

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libs/light/libxl_disk.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
index faabdea7a4c3..93936d0dd0f8 100644
--- a/tools/libs/light/libxl_disk.c
+++ b/tools/libs/light/libxl_disk.c
@@ -962,12 +962,26 @@ static void cdrom_insert_addfd_cb(libxl__egc *egc,
     fdset = libxl__json_object_get_integer(o);
 
     devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
-    QMP_PARAMETERS_SPRINTF(&args, "device", "ide-%i", devid);
-    QMP_PARAMETERS_SPRINTF(&args, "target", "/dev/fdset/%d", fdset);
-    libxl__qmp_param_add_string(gc, &args, "arg",
-        libxl__qemu_disk_format_string(disk->format));
     qmp->callback = cdrom_insert_inserted;
-    rc = libxl__ev_qmp_send(egc, qmp, "change", args);
+
+    /* "change" is deprecated since QEMU 2.5 and the `device` parameter for
+     * for "blockdev-change-medium" is deprecated in QEMU 2.8.
+     * But `id` is only available in 2.8 we'll start using the new command
+     * with `id` with QEMU 2.8.
+     */
+    if (libxl__qmp_ev_qemu_compare_version(qmp, 2, 8, 0) >= 0) {
+        QMP_PARAMETERS_SPRINTF(&args, "id", "ide-%i", devid);
+        QMP_PARAMETERS_SPRINTF(&args, "filename", "/dev/fdset/%d", fdset);
+        libxl__qmp_param_add_string(gc, &args, "format",
+            libxl__qemu_disk_format_string(disk->format));
+        rc = libxl__ev_qmp_send(egc, qmp, "blockdev-change-medium", args);
+    } else {
+        QMP_PARAMETERS_SPRINTF(&args, "device", "ide-%i", devid);
+        QMP_PARAMETERS_SPRINTF(&args, "target", "/dev/fdset/%d", fdset);
+        libxl__qmp_param_add_string(gc, &args, "arg",
+            libxl__qemu_disk_format_string(disk->format));
+        rc = libxl__ev_qmp_send(egc, qmp, "change", args);
+    }
 out:
     if (rc)
         cdrom_insert_done(egc, cis, rc); /* must be last */
-- 
Anthony PERARD

Re: [XEN PATCH v2 0/8] Fix libxl with QEMU 6.0 + remove some more deprecated usages.

[Julien Grall]

Hi Anthony,

On 11/05/2021 11:28, Anthony PERARD wrote:
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.deprecated-qemu-qmp-and-cmd-v2
> 
> v2:
> - fix coding style in patch 3
> - all reviewed
> 
> The Xen 4.15 release that went out just before QEMU 6.0 won't be compaptible
> with the latter. This patch series fixes libxl to replace use of QMP commands
> that have been removed from QEMU and to fix usage of deprecated command and
> parameters that well be remove from QEMU in the future.
> 
> All of the series should be backported to at least Xen 4.15 or it won't be
> possible to migrate, hotplug cpu or change cdrom on HVM guest when QEMU 6.0 and
> newer is used. QEMU 6.0 is about to be release, within a week.
> 
> Backport: 4.15
> 
> Anthony PERARD (8):
>    libxl: Replace deprecated QMP command by "query-cpus-fast"
>    libxl: Replace QEMU's command line short-form boolean option
>    libxl: Replace deprecated "cpu-add" QMP command by "device_add"
>    libxl: Use -device for cd-rom drives
>    libxl: Assert qmp_ev's state in qmp_ev_qemu_compare_version
>    libxl: Export libxl__qmp_ev_qemu_compare_version
>    libxl: Use `id` with the "eject" QMP command
>    libxl: Replace QMP command "change" by "blockdev-change-media"

I have committed the series.

Cheers,

-- 
Julien Grall

Re: [XEN PATCH v2 0/8] Fix libxl with QEMU 6.0 + remove some more deprecated usages.

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Am Tue, 11 May 2021 10:28:02 +0100
schrieb Anthony PERARD <anthony.perard@citrix.com>:

> All of the series should be backported to at least Xen 4.15 or it won't be
> possible to migrate, hotplug cpu or change cdrom on HVM guest when QEMU 6.0 and
> newer is used. QEMU 6.0 is about to be release, within a week.
> 
> Backport: 4.15

In case a backport to 4.15 is actually done, please backport this and the folluwup fixes also to 4.14.

Thank you.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]