Re: [PULL v2 00/13] misc patches removing deprecated features

[Daniel P . Berrangé]

On Mon, Mar 22, 2021 at 10:52:02AM +0000, Peter Maydell wrote:
> On Thu, 18 Mar 2021 at 09:30, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > ----------------------------------------------------------------
> > Remove many old deprecated features
> >
> > The following features have been deprecated for well over the 2
> > release cycle we promise
> >
> >   ``-drive file=json:{...{'driver':'file'}}`` (since 3.0)
> >   ``-vnc acl`` (since 4.0.0)
> >   ``-mon ...,control=readline,pretty=on|off`` (since 4.1)
> >   ``migrate_set_downtime`` and ``migrate_set_speed`` (since 2.8.0)
> >   ``query-named-block-nodes`` result ``encryption_key_missing`` (since 
> > 2.10.0)
> >   ``query-block`` result ``inserted.encryption_key_missing`` (since 2.10.0)
> >   ``migrate-set-cache-size`` and ``query-migrate-cache-size`` (since 2.11.0)
> >   ``query-named-block-nodes`` and ``query-block`` result 
> > dirty-bitmaps[i].status (since 4.0)
> >   ``query-cpus`` (since 2.12.0)
> >   ``query-cpus-fast`` ``arch`` output member (since 3.0.0)
> >   ``query-events`` (since 4.0)
> >   chardev client socket with ``wait`` option (since 4.0)
> >   ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` 
> > (since 4.0.0)
> >   ``ide-drive`` (since 4.2)
> >   ``scsi-disk`` (since 4.2)
> 
> Just FYI, it turns out that removing 'query-cpus' has broken Xen,
> whose libxl control library is still using it:
> 
> https://lists.xenproject.org/archives/html/xen-devel/2021-03/msg01178.html

I don't know what Xen's expected min QEMU version is, but if it
can assume QEMU >= 2.12, then there's no need to check for existance
of query-cpus-fast, and can just apply

diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 5d4ec90711..5fdcc182c0 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -1749,14 +1749,14 @@ static int qmp_parse_query_cpus(libxl__gc *gc,
     const libxl__json_object *cpu;
 
     libxl_bitmap_set_none(map);
-    /* Parse response to QMP command "query-cpus":
-     * [ { 'CPU': 'int',...} ]
+    /* 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", cpu, JSON_INTEGER);
+        o = libxl__json_map_get("cpu-index", cpu, JSON_INTEGER);
         if (!o) {
             LOGD(ERROR, domid, "Failed to retrieve CPU index.");
             return ERROR_QEMU_API;
@@ -1841,7 +1841,7 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid,
                                              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);
+            rc = libxl__ev_qmp_send(egc, qmp, "query-cpus-fast", NULL);
             if (rc) goto out;
             return AO_INPROGRESS;
         default:
@@ -2199,7 +2199,7 @@ static void retrieve_domain_configuration_lock_acquired(
         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);
+        rc = libxl__ev_qmp_send(egc, &rdcs->qmp, "query-cpus-fast", NULL);
         if (rc) goto out;
         has_callback = true;
     }


If Xen needs to support QEMU < 2.12, then it will need to
probe for existance of query-cpus-fast using "query-commands"
as libvirt does...

> This suggests we should probably try to coordinate a bit better
> with Xen as one of our downstream QMP users.

They could be added to the MAINTAINERS file for the deprecated features
doc, so that paches automatically CC xen-devel (or another suitable
list or person). 



> Am I right in thinking that QEMU now has an option that Xen
> could turn on in their CI for "make attempts to use deprecated
> features fail" so they could catch them earlier in the
> deprecate-and-delete cycle?

Yes, you just merged it from Markus:

  https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg06803.html

it won't capture all deprecations, but it will catch QMP related ones
and thus I believe would have caught the query-cpus deprecaton.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|