On 5/15/20 10:27 AM, Peter Krempa wrote: While you are touching this, s/is be placed into/will be placed into the/ With that, Reviewed-by: Eric Blake <eblake(a)redhat.com&gt; -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

On 5/15/20 10:27 AM, Peter Krempa wrote: preferred It would be nice if there were a single spelling we could use for all boolean CLI parameters across all supported qemu versions. But this is certainly the most conservative way to deal with the fact that some of the CLI parameters are ad hoc rather than sharing common code, and thus inconsistent on which spellings work where. Reviewed-by: Eric Blake <eblake(a)redhat.com&gt; -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

On 5/15/20 10:27 AM, Peter Krempa wrote: Do we actually have to special-case "type"? Reviewed-by: Eric Blake <eblake(a)redhat.com&gt; -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

The 'netdev_add' command was recently formally described in qemu via the QMP schema. This means that it also requires the arguments to be properly formatted. Our current approach is to generate the command line and then use qemuMonitorJSONKeywordStringToJSON to get the JSON properties for the monitor. This will not work if we need to pass some fields as numbers or booleans. In this step we re-do internals of qemuBuildHostNetStr to format a JSON object which is converted back via virQEMUBuildNetdevCommandlineFromJSON to the equivalent command line. This will later allow fixing of the monitor code to use the JSON object directly rather than rely on the conversion. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 163 +++++++++++++++++++++++++--------------- src/qemu/qemu_command.h | 12 +-- src/qemu/qemu_hotplug.c | 13 +++- 3 files changed, 119 insertions(+), 69 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2850953bd0..00d1b4121d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3564,7 +3564,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, } -char * +virJSONValuePtr qemuBuildHostNetStr(virDomainNetDefPtr net, char **tapfd, size_t tapfdSize, @@ -3573,10 +3573,11 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, const char *slirpfd) { bool is_tap = false; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virDomainNetType netType = virDomainNetGetActualType(net); size_t i; + g_autoptr(virJSONValue) netprops = NULL; + if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("scripts are not supported on interfaces of type %s"), @@ -3594,54 +3595,75 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_DIRECT: case VIR_DOMAIN_NET_TYPE_ETHERNET: - virBufferAddLit(&buf, "tap,"); + if (virJSONValueObjectCreate(&netprops, "s:type", "tap", NULL) < 0) + return NULL; + /* for one tapfd 'fd=' shall be used, * for more than one 'fds=' is the right choice */ if (tapfdSize == 1) { - virBufferAsprintf(&buf, "fd=%s,", tapfd[0]); + if (virJSONValueObjectAdd(netprops, "s:fd", tapfd[0], NULL) < 0) + return NULL; } else { - virBufferAddLit(&buf, "fds="); - for (i = 0; i < tapfdSize; i++) { - if (i) - virBufferAddChar(&buf, ':'); - virBufferAdd(&buf, tapfd[i], -1); - } - virBufferAddChar(&buf, ','); + g_auto(virBuffer) fdsbuf = VIR_BUFFER_INITIALIZER; + + for (i = 0; i < tapfdSize; i++) + virBufferAsprintf(&fdsbuf, "%s:", tapfd[i]); + + virBufferTrim(&fdsbuf, ":"); + + if (virJSONValueObjectAdd(netprops, + "s:fds", virBufferCurrentContent(&fdsbuf), + NULL) < 0) + return NULL; } + is_tap = true; break; case VIR_DOMAIN_NET_TYPE_CLIENT: - virBufferAsprintf(&buf, "socket,connect=%s:%d,", - net->data.socket.address, - net->data.socket.port); + if (virJSONValueObjectCreate(&netprops, "s:type", "socket", NULL) < 0 || + virJSONValueObjectAppendStringPrintf(netprops, "connect", "%s:%d", + net->data.socket.address, + net->data.socket.port) < 0) + return NULL; break; case VIR_DOMAIN_NET_TYPE_SERVER: - virBufferAsprintf(&buf, "socket,listen=%s:%d,", - NULLSTR_EMPTY(net->data.socket.address), - net->data.socket.port); + if (virJSONValueObjectCreate(&netprops, "s:type", "socket", NULL) < 0 || + virJSONValueObjectAppendStringPrintf(netprops, "listen", "%s:%d", + NULLSTR_EMPTY(net->data.socket.address), + net->data.socket.port) < 0) + return NULL; break; case VIR_DOMAIN_NET_TYPE_MCAST: - virBufferAsprintf(&buf, "socket,mcast=%s:%d,", - net->data.socket.address, - net->data.socket.port); + if (virJSONValueObjectCreate(&netprops, "s:type", "socket", NULL) < 0 || + virJSONValueObjectAppendStringPrintf(netprops, "mcast", "%s:%d", + net->data.socket.address, + net->data.socket.port) < 0) + return NULL; break; case VIR_DOMAIN_NET_TYPE_UDP: - virBufferAsprintf(&buf, "socket,udp=%s:%d,localaddr=%s:%d,", - net->data.socket.address, - net->data.socket.port, - net->data.socket.localaddr, - net->data.socket.localport); + if (virJSONValueObjectCreate(&netprops, "s:type", "socket", NULL) < 0 || + virJSONValueObjectAppendStringPrintf(netprops, "udp", "%s:%d", + net->data.socket.address, + net->data.socket.port) < 0 || + virJSONValueObjectAppendStringPrintf(netprops, "localaddr", "%s:%d", + net->data.socket.localaddr, + net->data.socket.localport) < 0) + return NULL; break; case VIR_DOMAIN_NET_TYPE_USER: if (slirpfd) { - virBufferAsprintf(&buf, "socket,fd=%s,", slirpfd); + if (virJSONValueObjectCreate(&netprops, "s:type", "socket", NULL) < 0 || + virJSONValueObjectAppendString(netprops, "fd", slirpfd) < 0) + return NULL; } else { - virBufferAddLit(&buf, "user,"); + if (virJSONValueObjectCreate(&netprops, "s:type", "user", NULL) < 0) + return NULL; + for (i = 0; i < net->guestIP.nips; i++) { const virNetDevIPAddr *ip = net->guestIP.ips[i]; g_autofree char *addr = NULL; @@ -3650,29 +3672,40 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, return NULL; if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { - virBufferAsprintf(&buf, "net=%s", addr); + g_autofree char *ipv4netaddr = NULL; + if (ip->prefix) - virBufferAsprintf(&buf, "/%u", ip->prefix); - virBufferAddChar(&buf, ','); + ipv4netaddr = g_strdup_printf("%s/%u", addr, ip->prefix); + else + ipv4netaddr = g_strdup(addr); + + if (virJSONValueObjectAppendString(netprops, "net", ipv4netaddr) < 0) + return NULL; } else if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) { - virBufferAsprintf(&buf, "ipv6-prefix=%s,", addr); - if (ip->prefix) - virBufferAsprintf(&buf, "ipv6-prefixlen=%u,", ip->prefix); + if (virJSONValueObjectAppendString(netprops, "ipv6-prefix", addr) < 0) + return NULL; + if (ip->prefix && + virJSONValueObjectAppendNumberUlong(netprops, "ipv6-prefixlen", + ip->prefix) < 0) + return NULL; } } } break; case VIR_DOMAIN_NET_TYPE_INTERNAL: - virBufferAddLit(&buf, "user,"); + if (virJSONValueObjectCreate(&netprops, "s:type", "user", NULL) < 0) + return NULL; break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - virBufferAsprintf(&buf, "vhost-user,chardev=char%s,", - net->info.alias); - if (net->driver.virtio.queues > 1) - virBufferAsprintf(&buf, "queues=%u,", - net->driver.virtio.queues); + if (virJSONValueObjectCreate(&netprops, "s:type", "vhost-user", NULL) < 0 || + virJSONValueObjectAppendStringPrintf(netprops, "chardev", "char%s", net->info.alias) < 0) + return NULL; + + if (net->driver.virtio.queues > 1 && + virJSONValueObjectAppendNumberUlong(netprops, "queues", net->driver.virtio.queues) < 0) + return NULL; break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: @@ -3681,31 +3714,38 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break; } - virBufferAsprintf(&buf, "id=host%s,", net->info.alias); + if (virJSONValueObjectAppendStringPrintf(netprops, "id", "host%s", net->info.alias) < 0) + return NULL; if (is_tap) { if (vhostfdSize) { - virBufferAddLit(&buf, "vhost=on,"); + if (virJSONValueObjectAppendBoolean(netprops, "vhost", true) < 0) + return NULL; + if (vhostfdSize == 1) { - virBufferAsprintf(&buf, "vhostfd=%s,", vhostfd[0]); + if (virJSONValueObjectAdd(netprops, "s:vhostfd", vhostfd[0], NULL) < 0) + return NULL; } else { - virBufferAddLit(&buf, "vhostfds="); - for (i = 0; i < vhostfdSize; i++) { - if (i) - virBufferAddChar(&buf, ':'); - virBufferAdd(&buf, vhostfd[i], -1); - } - virBufferAddChar(&buf, ','); + g_auto(virBuffer) fdsbuf = VIR_BUFFER_INITIALIZER; + + for (i = 0; i < vhostfdSize; i++) + virBufferAsprintf(&fdsbuf, "%s:", vhostfd[i]); + + virBufferTrim(&fdsbuf, ":"); + + if (virJSONValueObjectAdd(netprops, + "s:vhostfds", virBufferCurrentContent(&fdsbuf), + NULL) < 0) + return NULL; } } - if (net->tune.sndbuf_specified) - virBufferAsprintf(&buf, "sndbuf=%lu,", net->tune.sndbuf); - } - - virBufferTrim(&buf, ","); + if (net->tune.sndbuf_specified && + virJSONValueObjectAppendNumberUlong(netprops, "sndbuf", net->tune.sndbuf) < 0) + return NULL; + } - return virBufferContentAndReset(&buf); + return g_steal_pointer(&netprops); } @@ -7670,6 +7710,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, bool requireNicdev = false; qemuSlirpPtr slirp; size_t i; + g_autoptr(virJSONValue) hostnetprops = NULL; if (!bootindex) @@ -7873,11 +7914,15 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, if (chardev) virCommandAddArgList(cmd, "-chardev", chardev, NULL); - if (!(host = qemuBuildHostNetStr(net, - tapfdName, tapfdSize, - vhostfdName, vhostfdSize, - slirpfdName))) + if (!(hostnetprops = qemuBuildHostNetStr(net, + tapfdName, tapfdSize, + vhostfdName, vhostfdSize, + slirpfdName))) goto cleanup; + + if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops))) + goto cleanup; + virCommandAddArgList(cmd, "-netdev", host, NULL); /* Possible combinations: diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 7665b68548..6cdd9debe0 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -89,12 +89,12 @@ qemuBuildChrDeviceStr(char **deviceStr, char * qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr); -char *qemuBuildHostNetStr(virDomainNetDefPtr net, - char **tapfd, - size_t tapfdSize, - char **vhostfd, - size_t vhostfdSize, - const char *slirpfd); +virJSONValuePtr qemuBuildHostNetStr(virDomainNetDefPtr net, + char **tapfd, + size_t tapfdSize, + char **vhostfd, + size_t vhostfdSize, + const char *slirpfd); /* Current, best practice */ char *qemuBuildNicDevStr(virDomainDefPtr def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2976ba7647..c6789dcef3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -54,6 +54,7 @@ #include "virstoragefile.h" #include "virstring.h" #include "virtime.h" +#include "virqemu.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1157,6 +1158,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, size_t vhostfdSize = 0; size_t queueSize = 0; g_autofree char *nicstr = NULL; + g_autoptr(virJSONValue) netprops = NULL; g_autofree char *netstr = NULL; int ret = -1; bool releaseaddr = false; @@ -1382,10 +1384,13 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, for (i = 0; i < vhostfdSize; i++) vhostfdName[i] = g_strdup_printf("vhostfd-%s%zu", net->info.alias, i); - if (!(netstr = qemuBuildHostNetStr(net, - tapfdName, tapfdSize, - vhostfdName, vhostfdSize, - slirpfdName))) + if (!(netprops = qemuBuildHostNetStr(net, + tapfdName, tapfdSize, + vhostfdName, vhostfdSize, + slirpfdName))) + goto cleanup; + + if (!(netstr = virQEMUBuildNetdevCommandlineFromJSON(netprops))) goto cleanup; qemuDomainObjEnterMonitor(driver, vm); -- 2.26.2

QEMU models guestfwd as: 'guestfwd': [ { "str": "tcp:10.0.2.1:4600-chardev:charchannel0" }, { "str": "...."}, ] I guess the original idea was to make it extensible while not worrying about adding another object for it. Either way it requires us to add yet another JSON->cmdline convertor for arrays. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virqemu.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 0f8cab29df..f3e9966598 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -109,6 +109,41 @@ virQEMUBuildCommandLineJSONArrayNumbered(const char *key, } +/** + * This array convertor is for quirky cases where the QMP schema mandates an + * array of objects with only one attribute 'str' which needs to be formatted as + * repeated key-value pairs without the 'str' being printed: + * + * 'guestfwd': [ + * { "str": "tcp:10.0.2.1:4600-chardev:charchannel0" }, + * { "str": "...."}, + * ] + */ +static int +virQEMUBuildCommandLineJSONArrayObjectsStr(const char *key, + virJSONValuePtr array, + virBufferPtr buf, + const char *skipKey G_GNUC_UNUSED, + bool onOff G_GNUC_UNUSED) +{ + g_auto(virBuffer) tmp = VIR_BUFFER_INITIALIZER; + size_t i; + + for (i = 0; i < virJSONValueArraySize(array); i++) { + virJSONValuePtr member = virJSONValueArrayGet(array, i); + const char *str = virJSONValueObjectGetString(member, "str"); + + if (!str) + return -1; + + virBufferAsprintf(&tmp, "%s=%s,", key, str); + } + + virBufferAddBuffer(buf, &tmp); + return 0; +} + + /* internal iterator to handle nested object formatting */ static int virQEMUBuildCommandLineJSONIterate(const char *key, @@ -267,7 +302,8 @@ virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props) virBufferAsprintf(&buf, "%s,", type); - if (virQEMUBuildCommandLineJSON(props, &buf, "type", true, NULL) < 0) + if (virQEMUBuildCommandLineJSON(props, &buf, "type", true, + virQEMUBuildCommandLineJSONArrayObjectsStr) < 0) return NULL; return virBufferContentAndReset(&buf); -- 2.26.2

Syntax of guestfwd channel also needs to be modified to conform to the QAPI schema. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++++++++------ src/qemu/qemu_command.h | 2 +- src/qemu/qemu_hotplug.c | 6 +++++- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 00d1b4121d..2ed8e66f66 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8567,6 +8567,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, for (i = 0; i < def->nchannels; i++) { virDomainChrDefPtr channel = def->channels[i]; g_autofree char *chardevstr = NULL; + g_autoptr(virJSONValue) netdevprops = NULL; g_autofree char *netdevstr = NULL; if (!(chardevstr = qemuBuildChrChardevStr(logManager, secManager, @@ -8581,8 +8582,12 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, switch ((virDomainChrChannelTargetType) channel->targetType) { case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: - if (!(netdevstr = qemuBuildChannelGuestfwdNetdevProps(channel))) + if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(channel))) return -1; + + if (!(netdevstr = virQEMUBuildNetdevCommandlineFromJSON(netdevprops))) + return -1; + virCommandAddArgList(cmd, "-netdev", netdevstr, NULL); break; @@ -9850,19 +9855,39 @@ qemuBuildParallelChrDeviceStr(char **deviceStr, } -char * +virJSONValuePtr qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr) { + g_autoptr(virJSONValue) guestfwdarr = virJSONValueNewArray(); + g_autoptr(virJSONValue) guestfwdstrobj = virJSONValueNewObject(); g_autofree char *addr = NULL; - int port; + virJSONValuePtr ret = NULL; if (!(addr = virSocketAddrFormat(chr->target.addr))) return NULL; - port = virSocketAddrGetPort(chr->target.addr); + /* this may seem weird, but qemu indeed decided that 'guestfwd' parameter + * is an array of objects which have just one member named 'str' which + * contains the description */ + if (virJSONValueObjectAppendStringPrintf(guestfwdstrobj, "str", + "tcp:%s:%i-chardev:char%s", + addr, + virSocketAddrGetPort(chr->target.addr), + chr->info.alias) < 0) + return NULL; + + if (virJSONValueArrayAppend(guestfwdarr, guestfwdstrobj) < 0) + return NULL; + guestfwdstrobj = NULL; - return g_strdup_printf("user,guestfwd=tcp:%s:%i-chardev:char%s,id=%s", - addr, port, chr->info.alias, chr->info.alias); + if (virJSONValueObjectCreate(&ret, + "s:type", "user", + "a:guestfwd", &guestfwdarr, + "s:id", chr->info.alias, + NULL) < 0) + return NULL; + + return ret; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 6cdd9debe0..1daa07982c 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -86,7 +86,7 @@ qemuBuildChrDeviceStr(char **deviceStr, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps); -char * +virJSONValuePtr qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr); virJSONValuePtr qemuBuildHostNetStr(virDomainNetDefPtr net, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c6789dcef3..cd9bc9b2f8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2113,6 +2113,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, virErrorPtr orig_err; virDomainDefPtr vmdef = vm->def; g_autofree char *devstr = NULL; + g_autoptr(virJSONValue) netdevprops = NULL; g_autofree char *netdevstr = NULL; virDomainChrSourceDefPtr dev = chr->source; g_autofree char *charAlias = NULL; @@ -2153,7 +2154,10 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, teardowncgroup = true; if (guestfwd) { - if (!(netdevstr = qemuBuildChannelGuestfwdNetdevProps(chr))) + if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(chr))) + goto cleanup; + + if (!(netdevstr = virQEMUBuildNetdevCommandlineFromJSON(netdevprops))) goto cleanup; } else { if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0) -- 2.26.2

Now that all code paths generate JSON props we can remove the conversion to command line arguments and back in the monitor code. Note that the test which is removed in this commit will be replaced by a stronger testsuite later. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 14 +++----------- src/qemu/qemu_monitor.c | 8 ++++---- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 15 +++------------ src/qemu/qemu_monitor_json.h | 2 +- tests/qemumonitorjsontest.c | 2 -- 6 files changed, 12 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cd9bc9b2f8..2b0cdad4e7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1159,7 +1159,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, size_t queueSize = 0; g_autofree char *nicstr = NULL; g_autoptr(virJSONValue) netprops = NULL; - g_autofree char *netstr = NULL; int ret = -1; bool releaseaddr = false; bool iface_connected = false; @@ -1390,9 +1389,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, slirpfdName))) goto cleanup; - if (!(netstr = virQEMUBuildNetdevCommandlineFromJSON(netprops))) - goto cleanup; - qemuDomainObjEnterMonitor(driver, vm); if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { @@ -1404,7 +1400,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, charDevPlugged = true; } - if (qemuMonitorAddNetdev(priv->mon, netstr, + if (qemuMonitorAddNetdev(priv->mon, &netprops, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize, slirpfd, slirpfdName) < 0) { @@ -2114,7 +2110,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, virDomainDefPtr vmdef = vm->def; g_autofree char *devstr = NULL; g_autoptr(virJSONValue) netdevprops = NULL; - g_autofree char *netdevstr = NULL; virDomainChrSourceDefPtr dev = chr->source; g_autofree char *charAlias = NULL; bool chardevAttached = false; @@ -2156,9 +2151,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, if (guestfwd) { if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(chr))) goto cleanup; - - if (!(netdevstr = virQEMUBuildNetdevCommandlineFromJSON(netdevprops))) - goto cleanup; } else { if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0) goto cleanup; @@ -2181,8 +2173,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, goto exit_monitor; chardevAttached = true; - if (netdevstr) { - if (qemuMonitorAddNetdev(priv->mon, netdevstr, + if (netdevprops) { + if (qemuMonitorAddNetdev(priv->mon, &netdevprops, NULL, NULL, 0, NULL, NULL, 0, -1, NULL) < 0) goto exit_monitor; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 9c853ccb93..2911307f0e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2667,7 +2667,7 @@ qemuMonitorCloseFileHandle(qemuMonitorPtr mon, int qemuMonitorAddNetdev(qemuMonitorPtr mon, - const char *netdevstr, + virJSONValuePtr *props, int *tapfd, char **tapfdName, int tapfdSize, int *vhostfd, char **vhostfdName, int vhostfdSize, int slirpfd, char *slirpfdName) @@ -2675,10 +2675,10 @@ qemuMonitorAddNetdev(qemuMonitorPtr mon, int ret = -1; size_t i = 0, j = 0; - VIR_DEBUG("netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d" + VIR_DEBUG("props=%p tapfd=%p tapfdName=%p tapfdSize=%d" "vhostfd=%p vhostfdName=%p vhostfdSize=%d" "slirpfd=%d slirpfdName=%s", - netdevstr, tapfd, tapfdName, tapfdSize, + props, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize, slirpfd, slirpfdName); QEMU_CHECK_MONITOR(mon); @@ -2696,7 +2696,7 @@ qemuMonitorAddNetdev(qemuMonitorPtr mon, qemuMonitorSendFileHandle(mon, slirpfdName, slirpfd) < 0) goto cleanup; - ret = qemuMonitorJSONAddNetdev(mon, netdevstr); + ret = qemuMonitorJSONAddNetdev(mon, props); cleanup: if (ret < 0) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2e35d94bda..378e2532c7 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -881,7 +881,7 @@ int qemuMonitorCloseFileHandle(qemuMonitorPtr mon, const char *fdname); int qemuMonitorAddNetdev(qemuMonitorPtr mon, - const char *netdevstr, + virJSONValuePtr *props, int *tapfd, char **tapfdName, int tapfdSize, int *vhostfd, char **vhostfdName, int vhostfdSize, int slirpfd, char *slirpfdName); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1224d1c9cc..a98479a55d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3986,23 +3986,14 @@ int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, - const char *netdevstr) + virJSONValuePtr *props) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; - g_autoptr(virJSONValue) args = NULL; - - cmd = qemuMonitorJSONMakeCommand("netdev_add", NULL); - if (!cmd) - return -1; - - args = qemuMonitorJSONKeywordStringToJSON(netdevstr, "type"); - if (!args) - return -1; + virJSONValuePtr pr = g_steal_pointer(props); - if (virJSONValueObjectAppend(cmd, "arguments", args) < 0) + if (!(cmd = qemuMonitorJSONMakeCommandInternal("netdev_add", pr))) return -1; - args = NULL; /* obj owns reference to args now */ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) return -1; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index d3d46ec2f7..83c5e25ca5 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -210,7 +210,7 @@ int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, const char *fdname); int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, - const char *netdevstr); + virJSONValuePtr *props); int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon, const char *alias); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index f58b18a110..a50b157dbc 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1315,7 +1315,6 @@ GEN_TEST_FUNC(qemuMonitorJSONDump, "dummy_protocol", "elf", true) GEN_TEST_FUNC(qemuMonitorJSONGraphicsRelocate, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, "localhost", 12345, 12346, "certsubjectval") -GEN_TEST_FUNC(qemuMonitorJSONAddNetdev, "id=net0,type=user") GEN_TEST_FUNC(qemuMonitorJSONRemoveNetdev, "net0") GEN_TEST_FUNC(qemuMonitorJSONDelDevice, "ide0") GEN_TEST_FUNC(qemuMonitorJSONAddDevice, "some_dummy_devicestr") @@ -3226,7 +3225,6 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONMigrate); DO_TEST_GEN(qemuMonitorJSONDump); DO_TEST_GEN(qemuMonitorJSONGraphicsRelocate); - DO_TEST_GEN(qemuMonitorJSONAddNetdev); DO_TEST_GEN(qemuMonitorJSONRemoveNetdev); DO_TEST_GEN(qemuMonitorJSONDelDevice); DO_TEST_GEN(qemuMonitorJSONAddDevice); -- 2.26.2

qemuxml2argv test suite is way more comprehensive than the hotplug suite. Since we share the code paths for monitor and command line hotplug we can easily test the properties of devices against the QAPI schema. To achieve this we'll need to skip the JSON->commandline conversion for the test run so that we can analyze the pure properties. This patch adds flags for the comand line generator and hook them into the JSON->commandline convertor for -netdev. An upcoming patch will make use of this new infrastructure. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 28 +++++++++++++++++----------- src/qemu/qemu_command.h | 7 ++++++- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 11 +++++++++-- src/qemu/qemu_process.h | 1 + src/util/virqemu.c | 9 ++++++++- src/util/virqemu.h | 3 ++- tests/qemuxml2argvtest.c | 6 ++++-- 8 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2ed8e66f66..f9a2732587 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7691,7 +7691,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, virNetDevVPortProfileOp vmop, bool standalone, size_t *nnicindexes, - int **nicindexes) + int **nicindexes, + unsigned int flags) { virDomainDefPtr def = vm->def; int ret = -1; @@ -7920,7 +7921,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, slirpfdName))) goto cleanup; - if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops))) + if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops, + (flags & QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON)))) goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL); @@ -7996,7 +7998,8 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, bool standalone, size_t *nnicindexes, int **nicindexes, - unsigned int *bootHostdevNet) + unsigned int *bootHostdevNet, + unsigned int flags) { size_t i; int last_good_net = -1; @@ -8020,7 +8023,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, if (qemuBuildInterfaceCommandLine(driver, vm, logManager, secManager, cmd, net, qemuCaps, bootNet, vmop, standalone, nnicindexes, - nicindexes) < 0) + nicindexes, flags) < 0) goto error; last_good_net = i; @@ -8556,7 +8559,8 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, virQEMUDriverConfigPtr cfg, const virDomainDef *def, virQEMUCapsPtr qemuCaps, - bool chardevStdioLogd) + bool chardevStdioLogd, + unsigned int flags) { size_t i; unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT | @@ -8585,7 +8589,8 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(channel))) return -1; - if (!(netdevstr = virQEMUBuildNetdevCommandlineFromJSON(netdevprops))) + if (!(netdevstr = virQEMUBuildNetdevCommandlineFromJSON(netdevprops, + (flags & QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON)))) return -1; virCommandAddArgList(cmd, "-netdev", netdevstr, NULL); @@ -9521,7 +9526,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, bool standalone, bool enableFips, size_t *nnicindexes, - int **nicindexes) + int **nicindexes, + unsigned int flags) { size_t i; char uuid[VIR_UUID_STRING_BUFLEN]; @@ -9534,9 +9540,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, bool chardevStdioLogd = priv->chardevStdioLogd; VIR_DEBUG("driver=%p def=%p mon=%p " - "qemuCaps=%p migrateURI=%s snapshot=%p vmop=%d", + "qemuCaps=%p migrateURI=%s snapshot=%p vmop=%d flags=0x%x", driver, def, priv->monConfig, - qemuCaps, migrateURI, snapshot, vmop); + qemuCaps, migrateURI, snapshot, vmop, flags); if (qemuBuildCommandLineValidate(driver, def) < 0) return NULL; @@ -9681,7 +9687,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildNetCommandLine(driver, vm, logManager, secManager, cmd, qemuCaps, vmop, standalone, - nnicindexes, nicindexes, &bootHostdevNet) < 0) + nnicindexes, nicindexes, &bootHostdevNet, flags) < 0) return NULL; if (qemuBuildSmartcardCommandLine(logManager, secManager, cmd, cfg, def, qemuCaps, @@ -9697,7 +9703,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, return NULL; if (qemuBuildChannelsCommandLine(logManager, secManager, cmd, cfg, def, qemuCaps, - chardevStdioLogd) < 0) + chardevStdioLogd, flags) < 0) return NULL; if (qemuBuildConsoleCommandLine(logManager, secManager, cmd, cfg, def, qemuCaps, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 1daa07982c..55bc67072a 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -43,6 +43,10 @@ VIR_ENUM_DECL(qemuVideo); VIR_ENUM_DECL(qemuSoundCodec); +typedef enum { + QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON = 1 << 0, +} qemuBuildCommandLineFlags; + virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, virLogManagerPtr logManager, virSecurityManagerPtr secManager, @@ -53,7 +57,8 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, bool standalone, bool enableFips, size_t *nnicindexes, - int **nicindexes); + int **nicindexes, + unsigned int flags); /* Generate the object properties for pr-manager */ virJSONValuePtr qemuBuildPRManagerInfoProps(virStorageSourcePtr src); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index db65862163..dd9ae30bb5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7423,7 +7423,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, } if (!(cmd = qemuProcessCreatePretendCmd(driver, vm, NULL, - qemuCheckFips(), true, + qemuCheckFips(), true, false, VIR_QEMU_PROCESS_START_COLD))) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f7f6793113..ab32d49d6e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6746,7 +6746,7 @@ qemuProcessLaunch(virConnectPtr conn, snapshot, vmop, false, qemuCheckFips(), - &nnicindexes, &nicindexes))) + &nnicindexes, &nicindexes, 0))) goto cleanup; if (incoming && incoming->fd != -1) @@ -7189,8 +7189,11 @@ qemuProcessCreatePretendCmd(virQEMUDriverPtr driver, const char *migrateURI, bool enableFips, bool standalone, + bool jsonPropsValidation, unsigned int flags) { + unsigned int buildflags = 0; + virCheckFlags(VIR_QEMU_PROCESS_START_COLD | VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY, NULL); @@ -7200,6 +7203,9 @@ qemuProcessCreatePretendCmd(virQEMUDriverPtr driver, if (standalone) flags |= VIR_QEMU_PROCESS_START_STANDALONE; + if (jsonPropsValidation) + buildflags = QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON; + if (qemuProcessInit(driver, vm, NULL, QEMU_ASYNC_JOB_NONE, !!migrateURI, flags) < 0) return NULL; @@ -7218,7 +7224,8 @@ qemuProcessCreatePretendCmd(virQEMUDriverPtr driver, standalone, enableFips, NULL, - NULL); + NULL, + buildflags); } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 3077d3ef9e..15e67b9762 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -101,6 +101,7 @@ virCommandPtr qemuProcessCreatePretendCmd(virQEMUDriverPtr driver, const char *migrateURI, bool enableFips, bool standalone, + bool jsonPropsValidation, unsigned int flags); int qemuProcessInit(virQEMUDriverPtr driver, diff --git a/src/util/virqemu.c b/src/util/virqemu.c index f3e9966598..cf76346bfa 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -290,16 +290,23 @@ virQEMUBuildCommandLineJSON(virJSONValuePtr value, /** * virQEMUBuildNetdevCommandlineFromJSON: * @props: JSON properties describing a netdev + * @rawjson: don't transform to commandline args, but just stringify json * * Converts @props into arguments for -netdev including all the quirks and * differences between the monitor and command line syntax. + * + * @rawjson is meant for testing of the schema in the xml2argvtest */ char * -virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props) +virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props, + bool rawjson) { const char *type = virJSONValueObjectGetString(props, "type"); g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + if (rawjson) + return virJSONValueToString(props, false); + virBufferAsprintf(&buf, "%s,", type); if (virQEMUBuildCommandLineJSON(props, &buf, "type", true, diff --git a/src/util/virqemu.h b/src/util/virqemu.h index 22f47851df..b1296cb657 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -50,7 +50,8 @@ int virQEMUBuildCommandLineJSON(virJSONValuePtr value, virQEMUBuildCommandLineJSONArrayFormatFunc array); char * -virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props); +virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props, + bool rawjson); int virQEMUBuildObjectCommandlineFromJSON(virBufferPtr buf, virJSONValuePtr objprops); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9fa1afd9d0..5123580ee2 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -396,7 +396,8 @@ testCompareXMLToArgvCreateArgs(virQEMUDriverPtr drv, virDomainObjPtr vm, const char *migrateURI, struct testQemuInfo *info, - unsigned int flags) + unsigned int flags, + bool jsonPropsValidation) { size_t i; @@ -471,6 +472,7 @@ testCompareXMLToArgvCreateArgs(virQEMUDriverPtr drv, return qemuProcessCreatePretendCmd(drv, vm, migrateURI, (flags & FLAG_FIPS), false, + jsonPropsValidation, VIR_QEMU_PROCESS_START_COLD); } @@ -566,7 +568,7 @@ testCompareXMLToArgv(const void *data) virResetLastError(); if (!(cmd = testCompareXMLToArgvCreateArgs(&driver, vm, migrateURI, info, - flags))) { + flags, false))) { if (flags & FLAG_EXPECT_FAILURE) goto ok; goto cleanup; -- 2.26.2

It always loads the latest schema. Prepare for loading others as well. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tests/qemublocktest.c | 2 +- tests/qemuhotplugtest.c | 2 +- tests/qemumonitorjsontest.c | 4 ++-- tests/testutilsqemuschema.c | 2 +- tests/testutilsqemuschema.h | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 8057d1e2c0..0cdedb9ad4 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1056,7 +1056,7 @@ mymain(void) diskxmljsondata.qemuCaps = caps_x86_64; imagecreatedata.qemuCaps = caps_x86_64; - if (!(qmp_schema_x86_64 = testQEMUSchemaLoad("x86_64"))) { + if (!(qmp_schema_x86_64 = testQEMUSchemaLoadLatest("x86_64"))) { ret = -1; goto cleanup; } diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 1843e79d77..ba3fc4d814 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -636,7 +636,7 @@ mymain(void) if (!(driver.domainEventState = virObjectEventStateNew())) return EXIT_FAILURE; - if (!(qmpschema = testQEMUSchemaLoad("x86_64"))) { + if (!(qmpschema = testQEMUSchemaLoadLatest("x86_64"))) { VIR_TEST_VERBOSE("failed to load qapi schema\n"); return EXIT_FAILURE; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index a50b157dbc..6d5a1d5fe2 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -3124,7 +3124,7 @@ mymain(void) virEventRegisterDefaultImpl(); - if (!(qapiData.schema = testQEMUSchemaLoad("x86_64"))) { + if (!(qapiData.schema = testQEMUSchemaLoadLatest("x86_64"))) { VIR_TEST_VERBOSE("failed to load qapi schema"); ret = -1; goto cleanup; @@ -3394,7 +3394,7 @@ mymain(void) #undef DO_TEST_QUERY_JOBS virHashFree(qapiData.schema); - if (!(qapiData.schema = testQEMUSchemaLoad("s390x"))) { + if (!(qapiData.schema = testQEMUSchemaLoadLatest("s390x"))) { VIR_TEST_VERBOSE("failed to load qapi schema for s390x"); ret = -1; goto cleanup; diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index 898be68b0a..060a5d7054 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -661,7 +661,7 @@ testQEMUSchemaGetLatest(const char* arch) virHashTablePtr -testQEMUSchemaLoad(const char* arch) +testQEMUSchemaLoadLatest(const char *arch) { virJSONValuePtr schema; diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h index 8b6803afda..acedb77677 100644 --- a/tests/testutilsqemuschema.h +++ b/tests/testutilsqemuschema.h @@ -41,4 +41,4 @@ virJSONValuePtr testQEMUSchemaGetLatest(const char* arch); virHashTablePtr -testQEMUSchemaLoad(const char* arch); +testQEMUSchemaLoadLatest(const char *arch); -- 2.26.2

Add testQEMUSchemaLoad and refactor internals so that we can load the QMP schema from an arbitrary caps replies file. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tests/testutilsqemuschema.c | 64 ++++++++++++++++++++++++------------- tests/testutilsqemuschema.h | 3 ++ 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index 060a5d7054..d9b55fd787 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -607,37 +607,23 @@ testQEMUSchemaValidateCommand(const char *command, } -/** - * testQEMUSchemaGetLatest: - * - * Returns the schema data as the qemu monitor would reply from the latest - * replies file used for qemucapabilitiestest for the x86_64 architecture. - */ -virJSONValuePtr -testQEMUSchemaGetLatest(const char* arch) +static virJSONValuePtr +testQEMUSchemaLoadReplies(const char *filename) { - g_autofree char *capsLatestFile = NULL; - g_autofree char *capsLatest = NULL; + g_autofree char *caps = NULL; char *schemaReply; char *end; g_autoptr(virJSONValue) reply = NULL; virJSONValuePtr schema = NULL; - if (!(capsLatestFile = testQemuGetLatestCapsForArch(arch, "replies"))) { - VIR_TEST_VERBOSE("failed to find latest caps replies"); + if (virTestLoadFile(filename, &caps) < 0) return NULL; - } - VIR_TEST_DEBUG("replies file: '%s'", capsLatestFile); - - if (virTestLoadFile(capsLatestFile, &capsLatest) < 0) - return NULL; - - if (!(schemaReply = strstr(capsLatest, "\"execute\": \"query-qmp-schema\"")) || + if (!(schemaReply = strstr(caps, "\"execute\": \"query-qmp-schema\"")) || !(schemaReply = strstr(schemaReply, "\n\n")) || !(end = strstr(schemaReply + 2, "\n\n"))) { VIR_TEST_VERBOSE("failed to find reply to 'query-qmp-schema' in '%s'", - capsLatestFile); + filename); return NULL; } @@ -646,13 +632,13 @@ testQEMUSchemaGetLatest(const char* arch) if (!(reply = virJSONValueFromString(schemaReply))) { VIR_TEST_VERBOSE("failed to parse 'query-qmp-schema' reply from '%s'", - capsLatestFile); + filename); return NULL; } if (!(schema = virJSONValueObjectStealArray(reply, "return"))) { VIR_TEST_VERBOSE("missing qapi schema data in reply in '%s'", - capsLatestFile); + filename); return NULL; } @@ -660,6 +646,28 @@ testQEMUSchemaGetLatest(const char* arch) } +/** + * testQEMUSchemaGetLatest: + * + * Returns the schema data as the qemu monitor would reply from the latest + * replies file used for qemucapabilitiestest for the x86_64 architecture. + */ +virJSONValuePtr +testQEMUSchemaGetLatest(const char *arch) +{ + g_autofree char *capsLatestFile = NULL; + + if (!(capsLatestFile = testQemuGetLatestCapsForArch(arch, "replies"))) { + VIR_TEST_VERBOSE("failed to find latest caps replies"); + return NULL; + } + + VIR_TEST_DEBUG("replies file: '%s'", capsLatestFile); + + return testQEMUSchemaLoadReplies(capsLatestFile); +} + + virHashTablePtr testQEMUSchemaLoadLatest(const char *arch) { @@ -670,3 +678,15 @@ testQEMUSchemaLoadLatest(const char *arch) return virQEMUQAPISchemaConvert(schema); } + + +virHashTablePtr +testQEMUSchemaLoad(const char *filename) +{ + virJSONValuePtr schema; + + if (!(schema = testQEMUSchemaLoadReplies(filename))) + return NULL; + + return virQEMUQAPISchemaConvert(schema); +} diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h index acedb77677..c90a6b626d 100644 --- a/tests/testutilsqemuschema.h +++ b/tests/testutilsqemuschema.h @@ -42,3 +42,6 @@ testQEMUSchemaGetLatest(const char* arch); virHashTablePtr testQEMUSchemaLoadLatest(const char *arch); + +virHashTablePtr +testQEMUSchemaLoad(const char *filename); -- 2.26.2

Our hotplug test cases are weak in comparison to the xml2arvtest. Use all the the data to also valiadte the arguments for -netdev and -blockdev against the appropriate commands of the QMP schema. Note that currently it's done just for the _CAPS versions of tests but commenting out a line in the test file allows to validate even cases which don't use real capabilities. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tests/Makefile.am | 2 +- tests/qemuxml2argvtest.c | 76 ++++++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.c | 5 +++ tests/testutilsqemu.h | 1 + 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index fc516376b4..f5766a7790 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -570,7 +570,7 @@ qemuxml2argvtest_SOURCES = \ testutils.c testutils.h \ virfilewrapper.c virfilewrapper.h \ $(NULL) -qemuxml2argvtest_LDADD = libqemutestdriver.la \ +qemuxml2argvtest_LDADD = libqemutestdriver.la libqemumonitortestutils.la \ $(LDADDS) $(LIBXML_LIBS) libqemuxml2argvmock_la_SOURCES = \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5123580ee2..4f613e8f1a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -18,6 +18,7 @@ # include "qemu/qemu_migration.h" # include "qemu/qemu_process.h" # include "qemu/qemu_slirp.h" +# include "qemu/qemu_qapi.h" # include "datatypes.h" # include "conf/storage_conf.h" # include "cpu/cpu_map.h" @@ -26,6 +27,8 @@ # include "virmock.h" # include "virfilewrapper.h" # include "configmake.h" +# include "testutilsqemuschema.h" +# include "qemu/qemu_monitor_json.h" # define LIBVIRT_QEMU_CAPSPRIV_H_ALLOW # include "qemu/qemu_capspriv.h" @@ -477,6 +480,76 @@ testCompareXMLToArgvCreateArgs(virQEMUDriverPtr drv, } +static int +testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv, + virDomainObjPtr vm, + const char *migrateURI, + struct testQemuInfo *info, + unsigned int flags) +{ + VIR_AUTOSTRINGLIST args = NULL; + size_t nargs = 0; + size_t i; + g_autoptr(virHashTable) schema = NULL; + g_autoptr(virCommand) cmd = NULL; + + if (info->schemafile) + schema = testQEMUSchemaLoad(info->schemafile); + + /* comment out with line comment to enable schema checking for non _CAPS tests + if (!schema) + schema = testQEMUSchemaLoadLatest(virArchToString(info->arch)); + // */ + + if (!schema) + return 0; + + if (!(cmd = testCompareXMLToArgvCreateArgs(drv, vm, migrateURI, info, flags, + true))) + return -1; + + if (virCommandGetArgList(cmd, &args, &nargs) < 0) + return -1; + + for (i = 0; i < nargs; i++) { + g_auto(virBuffer) debug = VIR_BUFFER_INITIALIZER; + g_autoptr(virJSONValue) jsonargs = NULL; + + if (STREQ(args[i], "-blockdev")) { + if (!(jsonargs = virJSONValueFromString(args[i + 1]))) + return -1; + + if (testQEMUSchemaValidateCommand("blockdev-add", jsonargs, + schema, false, false, &debug) < 0) { + VIR_TEST_VERBOSE("failed to validate -blockdev '%s' against QAPI schema: %s", + args[i + 1], virBufferCurrentContent(&debug)); + return -1; + } + + i++; + } else if (STREQ(args[i], "-netdev")) { + if (!(jsonargs = virJSONValueFromString(args[i + 1]))) + return -1; + + /* skip the validation for pre-QAPIfication cases */ + if (virQEMUQAPISchemaPathExists("netdev_add/arg-type/type/!string", schema)) + continue; + + if (testQEMUSchemaValidateCommand("netdev_add", jsonargs, + schema, false, false, &debug) < 0) { + VIR_TEST_VERBOSE("failed to validate -netdev '%s' against QAPI schema: %s", + args[i + 1], virBufferCurrentContent(&debug)); + return -1; + } + + i++; + } + } + + return 0; +} + + static int testCompareXMLToArgv(const void *data) { @@ -578,6 +651,9 @@ testCompareXMLToArgv(const void *data) goto cleanup; } + if (testCompareXMLToArgvValidateSchema(&driver, vm, migrateURI, info, flags) < 0) + goto cleanup; + if (!(actualargv = virCommandToString(cmd, false))) goto cleanup; diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 9f9eb4033c..4b83b8ff25 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -768,6 +768,10 @@ testQemuInfoSetArgs(struct testQemuInfo *info, if (stripmachinealiases) virQEMUCapsStripMachineAliases(qemuCaps); info->flags |= FLAG_REAL_CAPS; + + /* provide path to the replies file for schema testing */ + capsfile[strlen(capsfile) - 3] = '\0'; + info->schemafile = g_strdup_printf("%sreplies", capsfile); } if (!qemuCaps) { @@ -794,5 +798,6 @@ testQemuInfoClear(struct testQemuInfo *info) { VIR_FREE(info->infile); VIR_FREE(info->outfile); + VIR_FREE(info->schemafile); virObjectUnref(info->qemuCaps); } diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index edee6e450c..e7c5032012 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -64,6 +64,7 @@ struct testQemuInfo { unsigned int flags; unsigned int parseFlags; virArch arch; + char *schemafile; }; virCapsPtr testQemuCapsInit(void); -- 2.26.2