[libvirt] [libvirt PATCH v2 10/44] Deprecate QEMU_CAPS_MONITOR_JSON

[Andrea Bolognani]

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 at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization