[PATCH 0/6] qemuBuildHostNetProps: Properly handle unsupported netdevs and remove virJSONValueObjectAppendStringPrintf
3:20 a.m.
First patch fixes a crasher if unsupported network device is used and
subsequently virJSONValueObjectAppendStringPrintf is removed.
Peter Krempa (6):
qemuBuildHostNetProps: Report proper errors for unhandled interface
types
qemuBuildHostNetProps: Don't use virJSONValueObjectAppendStringPrintf
to format address
qemuBuildChannelGuestfwdNetdevProps: Don't use
virJSONValueObjectAppendStringPrintf
qemuBuildHostNetProps: Append ipv6 address using virJSONValueObjectAdd
qemuBuildHostNetProps: Append aliases without
virJSONValueObjectAppendStringPrintf
util: json: Remove unused virJSONValueObjectAppendStringPrintf
src/libvirt_private.syms | 1 -
src/qemu/qemu_command.c | 113 +++++++++++++++++++++++++--------------
src/util/virjson.c | 17 ------
src/util/virjson.h | 6 ---
4 files changed, 74 insertions(+), 63 deletions(-)
--
2.39.2
3:20 a.m.
New subject: [PATCH 1/6] qemuBuildHostNetProps: Report proper errors for unhandled interface types
VIR_DOMAIN_NET_TYPE_NULL and VIR_DOMAIN_NET_TYPE_VDS are not implemented
for the qemu driver but the formatter code in 'qemuBuildHostNetProps'
didn't report an error for them and didn't even return from the function
when they were encountered.
This caused a crash in 'virJSONValueObjectAppendStringPrintf' which
does not tolerate NULL JSON object to append to when the unsupported
devices were used.
Properly report error when unhandled devices are encountered. This also
includes the case for VIR_DOMAIN_NET_TYPE_HOSTDEV, but that code path
should never be reached.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175582
Fixes: 6457619d186
Fixes: 0225483adce
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4839d45a34..589ec3e639 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3995,8 +3995,14 @@ qemuBuildHostNetProps(virDomainObj *vm,
/* Should have been handled earlier via PCI/USB hotplug code. */
case VIR_DOMAIN_NET_TYPE_NULL:
case VIR_DOMAIN_NET_TYPE_VDS:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("network device type '%s' is not supported by this
hypervisor"),
+ virDomainNetTypeToString(netType));
+ return NULL;
+
case VIR_DOMAIN_NET_TYPE_LAST:
- break;
+ virReportEnumRangeError(virDomainNetType, netType);
+ return NULL;
}
if (virJSONValueObjectAppendStringPrintf(netprops, "id",
"host%s", net->info.alias) < 0)
--
2.39.2
4:38 a.m.
New subject: [PATCH 1/6] qemuBuildHostNetProps: Report proper errors for unhandled interface types
On Mon, Mar 06, 2023 at 10:20:30AM +0100, Peter Krempa wrote:
VIR_DOMAIN_NET_TYPE_NULL and VIR_DOMAIN_NET_TYPE_VDS are not
implemented
for the qemu driver but the formatter code in 'qemuBuildHostNetProps'
didn't report an error for them and didn't even return from the function
when they were encountered.
This caused a crash in 'virJSONValueObjectAppendStringPrintf' which
does not tolerate NULL JSON object to append to when the unsupported
devices were used.
Properly report error when unhandled devices are encountered. This also
includes the case for VIR_DOMAIN_NET_TYPE_HOSTDEV, but that code path
should never be reached.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175582
Fixes: 6457619d186
Technically bac6b266fb6a was the first offender ;)
Fixes: 0225483adce
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4839d45a34..589ec3e639 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3995,8 +3995,14 @@ qemuBuildHostNetProps(virDomainObj *vm,
/* Should have been handled earlier via PCI/USB hotplug code. */
case VIR_DOMAIN_NET_TYPE_NULL:
case VIR_DOMAIN_NET_TYPE_VDS:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("network device type '%s' is not supported by this
hypervisor"),
+ virDomainNetTypeToString(netType));
+ return NULL;
+
case VIR_DOMAIN_NET_TYPE_LAST:
- break;
+ virReportEnumRangeError(virDomainNetType, netType);
+ return NULL;
}
if (virJSONValueObjectAppendStringPrintf(netprops, "id",
"host%s", net->info.alias) < 0)
--
2.39.2
3:20 a.m.
New subject: [PATCH 2/6] qemuBuildHostNetProps: Don't use virJSONValueObjectAppendStringPrintf to format address
Prefer virJSONValueObjectAdd which we already use internally combined
with local formatting of the string.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 68 +++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 589ec3e639..784003fbf5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3804,6 +3804,15 @@ qemuBuildNicDevProps(virDomainDef *def,
}
+static char *
+qemuBuildHostNetSocketAddr(virDomainNetDef *net)
+{
+ return g_strdup_printf("%s:%d",
+ NULLSTR_EMPTY(net->data.socket.address),
+ net->data.socket.port);
+}
+
+
virJSONValue *
qemuBuildHostNetProps(virDomainObj *vm,
virDomainNetDef *net)
@@ -3885,40 +3894,53 @@ qemuBuildHostNetProps(virDomainObj *vm,
}
break;
- case VIR_DOMAIN_NET_TYPE_CLIENT:
- if (virJSONValueObjectAdd(&netprops, "s:type", "socket",
NULL) < 0 ||
- virJSONValueObjectAppendStringPrintf(netprops, "connect",
"%s:%d",
- net->data.socket.address,
- net->data.socket.port) < 0)
+ case VIR_DOMAIN_NET_TYPE_CLIENT: {
+ g_autofree char *addr = qemuBuildHostNetSocketAddr(net);
+
+ if (virJSONValueObjectAdd(&netprops,
+ "s:type", "socket",
+ "s:connect", addr,
+ NULL) < 0)
return NULL;
break;
+ }
- case VIR_DOMAIN_NET_TYPE_SERVER:
- if (virJSONValueObjectAdd(&netprops, "s:type", "socket",
NULL) < 0 ||
- virJSONValueObjectAppendStringPrintf(netprops, "listen",
"%s:%d",
-
NULLSTR_EMPTY(net->data.socket.address),
- net->data.socket.port) < 0)
+ case VIR_DOMAIN_NET_TYPE_SERVER: {
+ g_autofree char *addr = qemuBuildHostNetSocketAddr(net);
+
+ if (virJSONValueObjectAdd(&netprops,
+ "s:type", "socket",
+ "s:listen", addr,
+ NULL) < 0)
return NULL;
break;
+ }
- case VIR_DOMAIN_NET_TYPE_MCAST:
- if (virJSONValueObjectAdd(&netprops, "s:type", "socket",
NULL) < 0 ||
- virJSONValueObjectAppendStringPrintf(netprops, "mcast",
"%s:%d",
- net->data.socket.address,
- net->data.socket.port) < 0)
+ case VIR_DOMAIN_NET_TYPE_MCAST: {
+ g_autofree char *addr = qemuBuildHostNetSocketAddr(net);
+
+ if (virJSONValueObjectAdd(&netprops,
+ "s:type", "socket",
+ "s:mcast", addr,
+ NULL) < 0)
return NULL;
break;
+ }
- case VIR_DOMAIN_NET_TYPE_UDP:
- if (virJSONValueObjectAdd(&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)
+ case VIR_DOMAIN_NET_TYPE_UDP: {
+ g_autofree char *addr = qemuBuildHostNetSocketAddr(net);
+ g_autofree char *localaddr = g_strdup_printf("%s:%d",
+ net->data.socket.localaddr,
+ net->data.socket.localport);
+
+ if (virJSONValueObjectAdd(&netprops,
+ "s:type", "socket",
+ "s:udp", addr,
+ "s:localaddr", localaddr,
+ NULL) < 0)
return NULL;
break;
+ }
case VIR_DOMAIN_NET_TYPE_USER:
if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) {
--
2.39.2
3:20 a.m.
New subject: [PATCH 3/6] qemuBuildChannelGuestfwdNetdevProps: Don't use virJSONValueObjectAppendStringPrintf
Use virJSONValueObjectAdd and format the string directly via
g_strdup_printf. In the end virJSONValueObjectAppendStringPrintf will be
removed.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 784003fbf5..420fe4ed0c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10475,22 +10475,24 @@ qemuBuildParallelChrDeviceProps(virDomainChrDef *chr)
virJSONValue *
qemuBuildChannelGuestfwdNetdevProps(virDomainChrDef *chr)
{
+ g_autofree char *guestfwdstr = NULL;
+ g_autoptr(virJSONValue) guestfwdstrobj = NULL;
g_autoptr(virJSONValue) guestfwdarr = virJSONValueNewArray();
- g_autoptr(virJSONValue) guestfwdstrobj = virJSONValueNewObject();
g_autofree char *addr = NULL;
virJSONValue *ret = NULL;
if (!(addr = virSocketAddrFormat(chr->target.addr)))
return NULL;
+ guestfwdstr = g_strdup_printf("tcp:%s:%i-chardev:char%s",
+ addr,
+ virSocketAddrGetPort(chr->target.addr),
+ chr->info.alias);
+
/* 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)
+ if (virJSONValueObjectAdd(&guestfwdstrobj, "s:str", guestfwdstr, NULL)
< 0)
return NULL;
if (virJSONValueArrayAppend(guestfwdarr, &guestfwdstrobj) < 0)
--
2.39.2
3:20 a.m.
New subject: [PATCH 4/6] qemuBuildHostNetProps: Append ipv6 address using virJSONValueObjectAdd
The 'ipv6-prefix' and 'ipv6-prefixlen' fields can be directly added
using virJSONValueObjectAdd rather tha by two separate calls.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 420fe4ed0c..1fc235153e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3974,11 +3974,10 @@ qemuBuildHostNetProps(virDomainObj *vm,
if (virJSONValueObjectAppendString(netprops, "net",
ipv4netaddr) < 0)
return NULL;
} else if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) {
- if (virJSONValueObjectAppendString(netprops, "ipv6-prefix",
addr) < 0)
- return NULL;
- if (ip->prefix &&
- virJSONValueObjectAppendNumberUlong(netprops,
"ipv6-prefixlen",
- ip->prefix) < 0)
+ if (virJSONValueObjectAdd(&netprops,
+ "s:ipv6-prefix", addr,
+ "p:ipv6-prefixlen",
ip->prefix,
+ NULL) < 0)
return NULL;
}
}
--
2.39.2
3:53 a.m.
New subject: [PATCH 4/6] qemuBuildHostNetProps: Append ipv6 address using virJSONValueObjectAdd
On a Monday in 2023, Peter Krempa wrote:
The 'ipv6-prefix' and 'ipv6-prefixlen' fields can be
directly added
using virJSONValueObjectAdd rather tha by two separate calls.
*than
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
3:20 a.m.
New subject: [PATCH 5/6] qemuBuildHostNetProps: Append aliases without virJSONValueObjectAppendStringPrintf
Format aliases into temporary strings and append them using
virJSONValueObjectAdd.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1fc235153e..e5c093e7ea 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3821,6 +3821,7 @@ qemuBuildHostNetProps(virDomainObj *vm,
size_t i;
qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net);
g_autoptr(virJSONValue) netprops = NULL;
+ g_autofree char *alias = g_strdup_printf("host%s", net->info.alias);
if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -3989,15 +3990,20 @@ qemuBuildHostNetProps(virDomainObj *vm,
return NULL;
break;
- case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
- if (virJSONValueObjectAdd(&netprops, "s:type",
"vhost-user", NULL) < 0 ||
- virJSONValueObjectAppendStringPrintf(netprops, "chardev",
"char%s", net->info.alias) < 0)
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: {
+ g_autofree char *charalias = g_strdup_printf("char%s",
net->info.alias);
+
+ if (virJSONValueObjectAdd(&netprops,
+ "s:type", "vhost-user",
+ "s:chardev", charalias,
+ NULL) < 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_VDPA:
/* Caller will pass the fd to qemu with add-fd */
@@ -4026,7 +4032,7 @@ qemuBuildHostNetProps(virDomainObj *vm,
return NULL;
}
- if (virJSONValueObjectAppendStringPrintf(netprops, "id",
"host%s", net->info.alias) < 0)
+ if (virJSONValueObjectAdd(&netprops, "s:id", alias, NULL) < 0)
return NULL;
return g_steal_pointer(&netprops);
--
2.39.2
3:20 a.m.
New subject: [PATCH 6/6] util: json: Remove unused virJSONValueObjectAppendStringPrintf
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt_private.syms | 1 -
src/util/virjson.c | 17 -----------------
src/util/virjson.h | 6 ------
3 files changed, 24 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b249dcc85c..8792bf7f54 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2621,7 +2621,6 @@ virJSONValueObjectAppendNumberLong;
virJSONValueObjectAppendNumberUint;
virJSONValueObjectAppendNumberUlong;
virJSONValueObjectAppendString;
-virJSONValueObjectAppendStringPrintf;
virJSONValueObjectDeflatten;
virJSONValueObjectForeachKeyValue;
virJSONValueObjectGet;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 1c3a50440e..4e822063bc 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -609,23 +609,6 @@ virJSONValueObjectAppendString(virJSONValue *object,
}
-int
-virJSONValueObjectAppendStringPrintf(virJSONValue *object,
- const char *key,
- const char *fmt,
- ...)
-{
- va_list ap;
- g_autofree char *str = NULL;
-
- va_start(ap, fmt);
- str = g_strdup_vprintf(fmt, ap);
- va_end(ap);
-
- return virJSONValueObjectInsertString(object, key, str, false);
-}
-
-
int
virJSONValueObjectPrependString(virJSONValue *object,
const char *key,
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 0ea7caad23..95b8b14ae6 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -204,12 +204,6 @@ virJSONValueObjectAppendString(virJSONValue *object,
const char *key,
const char *value);
int
-virJSONValueObjectAppendStringPrintf(virJSONValue *object,
- const char *key,
- const char *fmt,
- ...)
- G_GNUC_PRINTF(3, 4);
-int
virJSONValueObjectPrependString(virJSONValue *object,
const char *key,
const char *value);
--
2.39.2
3:53 a.m.
On a Monday in 2023, Peter Krempa wrote:
First patch fixes a crasher if unsupported network device is used and
subsequently virJSONValueObjectAppendStringPrintf is removed.
Peter Krempa (6):
qemuBuildHostNetProps: Report proper errors for unhandled interface
types
qemuBuildHostNetProps: Don't use virJSONValueObjectAppendStringPrintf
to format address
qemuBuildChannelGuestfwdNetdevProps: Don't use
virJSONValueObjectAppendStringPrintf
qemuBuildHostNetProps: Append ipv6 address using virJSONValueObjectAdd
qemuBuildHostNetProps: Append aliases without
virJSONValueObjectAppendStringPrintf
util: json: Remove unused virJSONValueObjectAppendStringPrintf
src/libvirt_private.syms | 1 -
src/qemu/qemu_command.c | 113 +++++++++++++++++++++++++--------------
src/util/virjson.c | 17 ------
src/util/virjson.h | 6 ---
4 files changed, 74 insertions(+), 63 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
657
days inactive
657
days old
9 comments
3 participants
participants (3)
-
Ján Tomko -
Martin Kletzander -
Peter Krempa