Re: [libvirt] [libvirt PATCH v2 10/44] Deprecate QEMU_CAPS_MONITOR_JSON
On Mon, 2018-04-09 at 17:20 +0200, Ján Tomko wrote:
We require QEMU >= 0.15.0, assume every QEMU supports it.
s/0.15.0/1.5.0/
Unless the usable monitor was introduced in 0.15.0, in which case
your version is more precise.
Sadly that does not let us trivially drop qemuMonitor's
priv->monJSON bool, because of qemuDomainQemuAttach.
I'm perfectly fine with dropping that being a follow-up patch,
just like the one taking care of qemuCaps->usedQMP. But just so
we're on the same page, you're planning on doing that, right?
[...]
diff --git a/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
b/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
index f192dd0063..21a3f591df 100644
--- a/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
+++ b/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
@@ -17,7 +17,7 @@ QEMU_AUDIO_DRV=none \
-nodefaults \
-chardev socket,id=charmonitor,\
path=/tmp/lib/domain--1-aarch64test/monitor.sock,server,nowait \
--mon chardev=charmonitor,id=monitor,mode=readline \
+-mon chardev=charmonitor,id=monitor,mode=control \
-no-acpi \
-boot c \
-kernel /aarch64.kernel \
[ ... etc ... ]
There's one more test case that was introduced in the meantime,
aarch64-traditional-pci, which is also affected by this. Make sure
you regenerate the .args for it too.
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8ff23f2ba9..c87ff2a87a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -462,8 +462,7 @@ testCompareXMLToArgv(const void *data)
virSetConnectSecret(conn);
virSetConnectStorage(conn);
- if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_MONITOR_JSON))
- flags |= FLAG_JSON;
+ flags |= FLAG_JSON;
It looks like FLAG_JSON can be dropped entirely now.
I'm actually unclear on what its purpose is supposed to be: it
doesn't seem to be used at all, we just set it if the JSON monitor
is supported and then never check whether it's set after that.
In fact, even on master, I can tweak the test to either alway set
it or never set it, and in either case 'make check' will still
complete successfully.
With the above addressed,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization