10:33 a.m.
This is a collection of cleanup patches from my ever-growing branch
refactoring handling of chardevs in the qemu driver.
Peter Krempa (16):
qemuBuildCommandLine: Properly check return value of
qemuBuildShmemCommandLine
qemu: hotplug: Add wrapper for qemuMonitorAttachCharDev
qemuMonitorJSONAddDeviceProps: Simplify construction of the command
qemuMonitorJSONAttachCharDevCommand: Format only the properties
qemuMonitorJSONAttachCharDevGetProps: Simplify handling of unsupported
types
qemuMonitorJSONAttachCharDevGetProps: Modernize construction of JSON
objects
qemu: command: Rename qemuBuildHostNetStr -> qemuBuildHostNetProps
qemuBuildChrChardevStr: Use proper type for the switch statement
conf: Convert 'chr' in virDomainShmemDef to proper pointer
qemuDomainAddChardevTLSObjects: Refactor cleanup
qemuDomainEnsurePCIAddress: Don't pass virQEMUDriver explicitly
qemuDomainAttachChrDeviceAssignAddr: Simplify return value handling
qemuDomainAttachChrDevice: Drop 'dev' variable
qemuBuildStorageSourceAttachPrepare(Drive|Chardev): Unexport
conf: Properly instantiate virDomainChrSourceDef in virDomainTPMDef
qemuxml2argvtest: Fix type for faked chardev backing a TPM
src/conf/domain_audit.c | 6 +-
src/conf/domain_conf.c | 31 +++--
src/conf/domain_conf.h | 6 +-
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_command.c | 40 +++---
src/qemu/qemu_command.h | 22 ++-
src/qemu/qemu_domain.c | 6 +-
src/qemu/qemu_domain_address.c | 13 +-
src/qemu/qemu_domain_address.h | 8 +-
src/qemu/qemu_hotplug.c | 101 +++++++-------
src/qemu/qemu_monitor_json.c | 236 ++++++++++++++++----------------
src/qemu/qemu_namespace.c | 2 +-
src/qemu/qemu_tpm.c | 10 +-
src/security/security_dac.c | 6 +-
src/security/security_selinux.c | 6 +-
tests/qemuxml2argvtest.c | 6 +-
16 files changed, 250 insertions(+), 251 deletions(-)
--
2.31.1
10:33 a.m.
New subject: [PATCH 01/16] qemuBuildCommandLine: Properly check return value of qemuBuildShmemCommandLine
Use the customary '< 0' check for return value.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 713304dd22..24dd00af9b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10882,7 +10882,7 @@ qemuBuildCommandLine(virQEMUDriver *driver,
for (i = 0; i < def->nshmems; i++) {
if (qemuBuildShmemCommandLine(logManager, secManager, cmd, cfg,
def, def->shmems[i], qemuCaps,
- chardevStdioLogd))
+ chardevStdioLogd) < 0)
return NULL;
}
--
2.31.1
4 a.m.
New subject: [PATCH 01/16] qemuBuildCommandLine: Properly check return value of qemuBuildShmemCommandLine
On a Thursday in 2021, Peter Krempa wrote:
Use the customary '< 0' check for return value.
We could start a new custom where any value under three is a failure.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 02/16] qemu: hotplug: Add wrapper for qemuMonitorAttachCharDev
Add a simple wrapper for 'qemuMonitorAttachCharDev' named
'qemuHotplugChardevAttach' which will simplify the moving of the
character device property generator out of the monitor code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 328b06245f..6f667dfb76 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -193,6 +193,15 @@ qemuDomainDetachExtensionDevice(qemuMonitor *mon,
}
+static int
+qemuHotplugChardevAttach(qemuMonitor *mon,
+ const char *alias,
+ virDomainChrSourceDef *def)
+{
+ return qemuMonitorAttachCharDev(mon, alias, def);
+}
+
+
static int
qemuHotplugWaitForTrayEject(virDomainObj *vm,
virDomainDiskDef *disk)
@@ -1459,7 +1468,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
}
if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
- if (qemuMonitorAttachCharDev(priv->mon, charDevAlias, net->data.vhostuser)
< 0) {
+ if (qemuHotplugChardevAttach(priv->mon, charDevAlias, net->data.vhostuser)
< 0) {
ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditNet(vm, NULL, net, "attach", false);
goto cleanup;
@@ -1980,9 +1989,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriver *driver,
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorAttachCharDev(priv->mon,
- charAlias,
- redirdev->source) < 0)
+ if (qemuHotplugChardevAttach(priv->mon, charAlias, redirdev->source) < 0)
goto exit_monitor;
chardevAdded = true;
@@ -2242,7 +2249,7 @@ int qemuDomainAttachChrDevice(virQEMUDriver *driver,
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorAttachCharDev(priv->mon, charAlias, chr->source) < 0)
+ if (qemuHotplugChardevAttach(priv->mon, charAlias, chr->source) < 0)
goto exit_monitor;
chardevAttached = true;
@@ -2350,8 +2357,7 @@ qemuDomainAttachRNGDevice(virQEMUDriver *driver,
qemuDomainObjEnterMonitor(driver, vm);
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
- qemuMonitorAttachCharDev(priv->mon, charAlias,
- rng->source.chardev) < 0)
+ qemuHotplugChardevAttach(priv->mon, charAlias, rng->source.chardev) <
0)
goto exit_monitor;
chardevAdded = true;
@@ -3058,8 +3064,7 @@ qemuDomainAttachShmemDevice(virQEMUDriver *driver,
qemuDomainObjEnterMonitor(driver, vm);
if (shmem->server.enabled) {
- if (qemuMonitorAttachCharDev(priv->mon, charAlias,
- &shmem->server.chr) < 0)
+ if (qemuHotplugChardevAttach(priv->mon, charAlias, &shmem->server.chr)
< 0)
goto exit_monitor;
} else {
if (qemuMonitorAddObject(priv->mon, &props, &memAlias) < 0)
@@ -3472,7 +3477,7 @@ qemuDomainAttachFSDevice(virQEMUDriver *driver,
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorAttachCharDev(priv->mon, charAlias, chardev) < 0)
+ if (qemuHotplugChardevAttach(priv->mon, charAlias, chardev) < 0)
goto exit_monitor;
chardevAdded = true;
--
2.31.1
4 a.m.
New subject: [PATCH 02/16] qemu: hotplug: Add wrapper for qemuMonitorAttachCharDev
On a Thursday in 2021, Peter Krempa wrote:
Add a simple wrapper for 'qemuMonitorAttachCharDev' named
'qemuHotplugChardevAttach' which will simplify the moving of the
character device property generator out of the monitor code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 03/16] qemuMonitorJSONAddDeviceProps: Simplify construction of the command
Use 'qemuMonitorJSONMakeCommandInternal' instead of
'qemuMonitorJSONMakeCommand' + 'virJSONValueObjectAppend'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a3bdfaaf0d..2cfae2276d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4154,11 +4154,8 @@ qemuMonitorJSONAddDeviceProps(qemuMonitor *mon,
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
- if (!(cmd = qemuMonitorJSONMakeCommand("device_add", NULL)))
- return -1;
-
- if (virJSONValueObjectAppend(cmd, "arguments", props) < 0)
- return -1;
+ if (!(cmd = qemuMonitorJSONMakeCommandInternal("device_add", props)))
+ return -1;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
--
2.31.1
4 a.m.
New subject: [PATCH 03/16] qemuMonitorJSONAddDeviceProps: Simplify construction of the command
On a Thursday in 2021, Peter Krempa wrote:
Use 'qemuMonitorJSONMakeCommandInternal' instead of
'qemuMonitorJSONMakeCommand' + 'virJSONValueObjectAppend'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 04/16] qemuMonitorJSONAttachCharDevCommand: Format only the properties
Move the addition of the command wrapper to qemuMonitorJSONAttachCharDev
and rename the function to qemuMonitorJSONAttachCharDevGetProps.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2cfae2276d..b036cc4112 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6609,9 +6609,10 @@ qemuMonitorJSONBuildChrChardevReconnect(virJSONValue *object,
}
static virJSONValue *
-qemuMonitorJSONAttachCharDevCommand(const char *chrID,
- const virDomainChrSourceDef *chr)
+qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
+ const virDomainChrSourceDef *chr)
{
+ g_autoptr(virJSONValue) props = NULL;
g_autoptr(virJSONValue) backend = virJSONValueNewObject();
g_autoptr(virJSONValue) data = virJSONValueNewObject();
g_autoptr(virJSONValue) addr = NULL;
@@ -6760,10 +6761,13 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
virJSONValueObjectAppend(backend, "data", &data) < 0)
return NULL;
- return qemuMonitorJSONMakeCommand("chardev-add",
- "s:id", chrID,
- "a:backend", &backend,
- NULL);
+ if (virJSONValueObjectAdd(&props,
+ "s:id", chrID,
+ "a:backend", &backend,
+ NULL) < 0)
+ return NULL;
+
+ return g_steal_pointer(&props);
}
@@ -6774,8 +6778,12 @@ qemuMonitorJSONAttachCharDev(qemuMonitor *mon,
{
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+ g_autoptr(virJSONValue) props = NULL;
+
+ if (!(props = qemuMonitorJSONAttachCharDevGetProps(chrID, chr)))
+ return -1;
- if (!(cmd = qemuMonitorJSONAttachCharDevCommand(chrID, chr)))
+ if (!(cmd = qemuMonitorJSONMakeCommandInternal("chardev-add",
&props)))
return -1;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
--
2.31.1
4:01 a.m.
New subject: [PATCH 04/16] qemuMonitorJSONAttachCharDevCommand: Format only the properties
On a Thursday in 2021, Peter Krempa wrote:
Move the addition of the command wrapper to
qemuMonitorJSONAttachCharDev
and rename the function to qemuMonitorJSONAttachCharDevGetProps.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 05/16] qemuMonitorJSONAttachCharDevGetProps: Simplify handling of unsupported types
Use 'virReportEnumRangeError' for the invalid cases and keep the
original error for known but unsupported chardevs.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index b036cc4112..1ced942161 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6737,16 +6737,14 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
case VIR_DOMAIN_CHR_TYPE_PIPE:
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_NMDM:
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Hotplug unsupported for char device type
'%s'"),
+ virDomainChrTypeToString(chr->type));
+ return NULL;
+
case VIR_DOMAIN_CHR_TYPE_LAST:
- if (virDomainChrTypeToString(chr->type)) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Hotplug unsupported for char device type
'%s'"),
- virDomainChrTypeToString(chr->type));
- } else {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Hotplug unsupported for char device type
'%d'"),
- chr->type);
- }
+ default:
+ virReportEnumRangeError(virDomainChrType, chr->type);
return NULL;
}
--
2.31.1
4:01 a.m.
New subject: [PATCH 05/16] qemuMonitorJSONAttachCharDevGetProps: Simplify handling of unsupported types
On a Thursday in 2021, Peter Krempa wrote:
Use 'virReportEnumRangeError' for the invalid cases and keep
the
original error for known but unsupported chardevs.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 06/16] qemuMonitorJSONAttachCharDevGetProps: Modernize construction of JSON objects
Use 'virJSONValueObjectAdd' instead of the step-by-step manual JSON
object building.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 195 ++++++++++++++++++-----------------
1 file changed, 99 insertions(+), 96 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 1ced942161..a9e87de4d3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6598,139 +6598,139 @@ int qemuMonitorJSONGetTPMTypes(qemuMonitor *mon,
return qemuMonitorJSONGetStringArray(mon, "query-tpm-types", tpmtypes);
}
-static int
-qemuMonitorJSONBuildChrChardevReconnect(virJSONValue *object,
- const virDomainChrSourceReconnectDef *def)
-{
- if (def->enabled != VIR_TRISTATE_BOOL_YES)
- return 0;
-
- return virJSONValueObjectAppendNumberUint(object, "reconnect",
def->timeout);
-}
static virJSONValue *
qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
const virDomainChrSourceDef *chr)
{
g_autoptr(virJSONValue) props = NULL;
- g_autoptr(virJSONValue) backend = virJSONValueNewObject();
- g_autoptr(virJSONValue) data = virJSONValueNewObject();
- g_autoptr(virJSONValue) addr = NULL;
- const char *backend_type = NULL;
- const char *host;
- const char *port;
- g_autofree char *tlsalias = NULL;
- bool telnet;
+ g_autoptr(virJSONValue) backend = NULL;
+ g_autoptr(virJSONValue) backendData = virJSONValueNewObject();
+ const char *backendType = NULL;
switch ((virDomainChrType)chr->type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
- backend_type = "null";
- break;
-
case VIR_DOMAIN_CHR_TYPE_VC:
- backend_type = "vc";
- break;
-
case VIR_DOMAIN_CHR_TYPE_PTY:
- backend_type = "pty";
+ backendType = virDomainChrTypeToString(chr->type);
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
- backend_type = "file";
- if (virJSONValueObjectAppendString(data, "out", chr->data.file.path)
< 0)
- return NULL;
- if (virJSONValueObjectAdd(&data,
+ backendType = "file";
+
+ if (virJSONValueObjectAdd(&backendData,
+ "s:out", chr->data.file.path,
"T:append", chr->data.file.append,
NULL) < 0)
return NULL;
+
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
- backend_type = STRPREFIX(chrID, "parallel") ? "parallel" :
"serial";
- if (virJSONValueObjectAppendString(data, "device",
- chr->data.file.path) < 0)
- return NULL;
- break;
+ if (STRPREFIX(chrID, "parallel"))
+ backendType = "parallel";
+ else
+ backendType = "serial";
- case VIR_DOMAIN_CHR_TYPE_TCP:
- backend_type = "socket";
- addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host,
- chr->data.tcp.service);
- if (!addr ||
- virJSONValueObjectAppend(data, "addr", &addr) < 0)
+ if (virJSONValueObjectAdd(&backendData,
+ "s:device", chr->data.file.path,
+ NULL) < 0)
return NULL;
- telnet = chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
+ break;
- if (chr->data.tcp.listen &&
- virJSONValueObjectAppendBoolean(data, "wait", false) < 0)
- return NULL;
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ case VIR_DOMAIN_CHR_TYPE_TCP: {
+ g_autofree char *tlsalias = NULL;
+ g_autoptr(virJSONValue) addr = NULL;
+ virTristateBool waitval = VIR_TRISTATE_BOOL_ABSENT;
+ virTristateBool telnet = VIR_TRISTATE_BOOL_ABSENT;
+ bool server = false;
+ int reconnect = -1;
+
+ backendType = "socket";
+
+ if (chr->type == VIR_DOMAIN_CHR_TYPE_TCP) {
+ telnet = virTristateBoolFromBool(chr->data.tcp.protocol ==
VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET);
+
+ if (chr->data.tcp.listen) {
+ server = true;
+ waitval = VIR_TRISTATE_BOOL_NO;
+ }
- if (virJSONValueObjectAppendBoolean(data, "telnet", telnet) < 0 ||
- virJSONValueObjectAppendBoolean(data, "server",
chr->data.tcp.listen) < 0)
- return NULL;
- if (chr->data.tcp.tlscreds) {
- if (!(tlsalias = qemuAliasTLSObjFromSrcAlias(chrID)))
+ if (chr->data.tcp.tlscreds &&
+ !(tlsalias = qemuAliasTLSObjFromSrcAlias(chrID)))
return NULL;
- if (virJSONValueObjectAppendString(data, "tls-creds", tlsalias)
< 0)
+ if (!(addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host,
+
chr->data.tcp.service)))
return NULL;
- }
-
- if (qemuMonitorJSONBuildChrChardevReconnect(data,
&chr->data.tcp.reconnect) < 0)
- return NULL;
- break;
- case VIR_DOMAIN_CHR_TYPE_UDP:
- backend_type = "udp";
- host = chr->data.udp.connectHost;
- if (!host)
- host = "";
- addr = qemuMonitorJSONBuildInetSocketAddress(host,
- chr->data.udp.connectService);
- if (!addr ||
- virJSONValueObjectAppend(data, "remote", &addr) < 0)
- return NULL;
+ if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_YES)
+ reconnect = chr->data.tcp.reconnect.timeout;
+ else if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_NO)
+ reconnect = 0;
+ } else {
+ if (chr->data.nix.listen) {
+ server = true;
+ waitval = VIR_TRISTATE_BOOL_NO;
+ }
- host = chr->data.udp.bindHost;
- port = chr->data.udp.bindService;
- if (host || port) {
- if (!host)
- host = "";
- if (!port)
- port = "";
- addr = qemuMonitorJSONBuildInetSocketAddress(host, port);
- if (!addr ||
- virJSONValueObjectAppend(data, "local", &addr) < 0)
+ if (!(addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path)))
return NULL;
+
+ if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES)
+ reconnect = chr->data.tcp.reconnect.timeout;
+ else if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_NO)
+ reconnect = 0;
}
+
+ if (virJSONValueObjectAdd(&backendData,
+ "a:addr", &addr,
+ "T:wait", waitval,
+ "T:telnet", telnet,
+ "b:server", server,
+ "S:tls-creds", tlsalias,
+ "k:reconnect", reconnect,
+ NULL) < 0)
+ return NULL;
+ }
break;
- case VIR_DOMAIN_CHR_TYPE_UNIX:
- backend_type = "socket";
- addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path);
- if (!addr ||
- virJSONValueObjectAppend(data, "addr", &addr) < 0)
- return NULL;
+ case VIR_DOMAIN_CHR_TYPE_UDP: {
+ g_autoptr(virJSONValue) local = NULL;
+ g_autoptr(virJSONValue) remote = NULL;
- if (chr->data.nix.listen &&
- virJSONValueObjectAppendBoolean(data, "wait", false) < 0)
- return NULL;
+ backendType = "udp";
- if (virJSONValueObjectAppendBoolean(data, "server",
chr->data.nix.listen) < 0)
+ if (!(remote =
qemuMonitorJSONBuildInetSocketAddress(NULLSTR_EMPTY(chr->data.udp.connectHost),
+
chr->data.udp.connectService)))
return NULL;
- if (qemuMonitorJSONBuildChrChardevReconnect(data,
&chr->data.nix.reconnect) < 0)
+ if (chr->data.udp.bindHost || chr->data.udp.bindService) {
+ if (!(local =
qemuMonitorJSONBuildInetSocketAddress(NULLSTR_EMPTY(chr->data.udp.bindHost),
+
NULLSTR_EMPTY(chr->data.udp.bindService))))
+ return NULL;
+ }
+
+ if (virJSONValueObjectAdd(&backendData,
+ "a:remote", &remote,
+ "A:local", &local,
+ NULL) < 0)
return NULL;
+ }
break;
+
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
- backend_type = "spicevmc";
+ backendType = "spicevmc";
- if (virJSONValueObjectAppendString(data, "type",
-
virDomainChrSpicevmcTypeToString(chr->data.spicevmc)) < 0)
+ if (virJSONValueObjectAdd(&backendData,
+ "s:type",
virDomainChrSpicevmcTypeToString(chr->data.spicevmc),
+ NULL) < 0)
return NULL;
+
break;
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
@@ -6748,15 +6748,18 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
return NULL;
}
- if (chr->logfile &&
- virJSONValueObjectAdd(&data,
- "s:logfile", chr->logfile,
- "T:logappend", chr->logappend,
- NULL) < 0)
- return NULL;
+ if (chr->logfile) {
+ if (virJSONValueObjectAdd(&backendData,
+ "S:logfile", chr->logfile,
+ "T:logappend", chr->logappend,
+ NULL) < 0)
+ return NULL;
+ }
- if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0
||
- virJSONValueObjectAppend(backend, "data", &data) < 0)
+ if (virJSONValueObjectAdd(&backend,
+ "s:type", backendType,
+ "A:data", &backendData,
+ NULL) < 0)
return NULL;
if (virJSONValueObjectAdd(&props,
--
2.31.1
4:31 a.m.
New subject: [PATCH 06/16] qemuMonitorJSONAttachCharDevGetProps: Modernize construction of JSON objects
On a Thursday in 2021, Peter Krempa wrote:
Use 'virJSONValueObjectAdd' instead of the step-by-step manual
JSON
object building.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 195 ++++++++++++++++++-----------------
1 file changed, 99 insertions(+), 96 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 1ced942161..a9e87de4d3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
const virDomainChrSourceDef *chr)
{
g_autoptr(virJSONValue) props = NULL;
- g_autoptr(virJSONValue) backend = virJSONValueNewObject();
- g_autoptr(virJSONValue) data = virJSONValueNewObject();
- g_autoptr(virJSONValue) addr = NULL;
- const char *backend_type = NULL;
- const char *host;
- const char *port;
- g_autofree char *tlsalias = NULL;
- bool telnet;
+ g_autoptr(virJSONValue) backend = NULL;
+ g_autoptr(virJSONValue) backendData = virJSONValueNewObject();
+ const char *backendType = NULL;
The backendType and backendData renames could've been done upfront to
simplify this patch, at the cost of changing the same lines multiple
times.
switch ((virDomainChrType)chr->type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
- backend_type = "null";
- break;
-
case VIR_DOMAIN_CHR_TYPE_VC:
- backend_type = "vc";
- break;
-
case VIR_DOMAIN_CHR_TYPE_PTY:
- backend_type = "pty";
+ backendType = virDomainChrTypeToString(chr->type);
[...]
- if (qemuMonitorJSONBuildChrChardevReconnect(data,
&chr->data.tcp.reconnect) < 0)
- return NULL;
- break;
- case VIR_DOMAIN_CHR_TYPE_UDP:
- backend_type = "udp";
- host = chr->data.udp.connectHost;
- if (!host)
- host = "";
- addr = qemuMonitorJSONBuildInetSocketAddress(host,
- chr->data.udp.connectService);
- if (!addr ||
- virJSONValueObjectAppend(data, "remote", &addr) < 0)
- return NULL;
+ if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_YES)
+ reconnect = chr->data.tcp.reconnect.timeout;
+ else if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_NO)
+ reconnect = 0;
Before, qemuMonitorJSONBuildChrChardevReconnect only formatted the
timeout value if enabled was set to BOOL_YES.
[A] Please split out the == BOOL_NO additions that unify it with what we do
in the cold start commandline generator.
+ } else {
+ if (chr->data.nix.listen) {
+ server = true;
+ waitval = VIR_TRISTATE_BOOL_NO;
+ }
[...]
@@ -6748,15 +6748,18 @@ qemuMonitorJSONAttachCharDevGetProps(const
char *chrID,
return NULL;
}
- if (chr->logfile &&
- virJSONValueObjectAdd(&data,
- "s:logfile", chr->logfile,
- "T:logappend", chr->logappend,
- NULL) < 0)
- return NULL;
+ if (chr->logfile) {
+ if (virJSONValueObjectAdd(&backendData,
+ "S:logfile", chr->logfile,
+ "T:logappend", chr->logappend,
+ NULL) < 0)
+ return NULL;
+ }
Apart from the style change [B] There's no need to change 's' to 'S'
here.
- if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0
||
- virJSONValueObjectAppend(backend, "data", &data) < 0)
+ if (virJSONValueObjectAdd(&backend,
+ "s:type", backendType,
+ "A:data", &backendData,
+ NULL) < 0)
return NULL;
With [A][B] addressed,
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 07/16] qemu: command: Rename qemuBuildHostNetStr -> qemuBuildHostNetProps
The function is already returning JSON properties, rename it
accordingly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 22 +++++++++++-----------
src/qemu/qemu_command.h | 15 ++++++++-------
src/qemu/qemu_hotplug.c | 8 ++++----
3 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 24dd00af9b..9293a413e6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3936,13 +3936,13 @@ qemuBuildNicDevProps(virDomainDef *def,
virJSONValue *
-qemuBuildHostNetStr(virDomainNetDef *net,
- char **tapfd,
- size_t tapfdSize,
- char **vhostfd,
- size_t vhostfdSize,
- const char *slirpfd,
- const char *vdpadev)
+qemuBuildHostNetProps(virDomainNetDef *net,
+ char **tapfd,
+ size_t tapfdSize,
+ char **vhostfd,
+ size_t vhostfdSize,
+ const char *slirpfd,
+ const char *vdpadev)
{
bool is_tap = false;
virDomainNetType netType = virDomainNetGetActualType(net);
@@ -8975,10 +8975,10 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
if (chardev)
virCommandAddArgList(cmd, "-chardev", chardev, NULL);
- if (!(hostnetprops = qemuBuildHostNetStr(net,
- tapfdName, tapfdSize,
- vhostfdName, vhostfdSize,
- slirpfdName, vdpafdName)))
+ if (!(hostnetprops = qemuBuildHostNetProps(net,
+ tapfdName, tapfdSize,
+ vhostfdName, vhostfdSize,
+ slirpfdName, vdpafdName)))
goto cleanup;
if (qemuBuildNetdevCommandlineFromJSON(cmd, hostnetprops, qemuCaps) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 81fd24406d..c544c9ec67 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -87,13 +87,14 @@ qemuBuildChrDeviceProps(const virDomainDef *vmdef,
virJSONValue *
qemuBuildChannelGuestfwdNetdevProps(virDomainChrDef *chr);
-virJSONValue *qemuBuildHostNetStr(virDomainNetDef *net,
- char **tapfd,
- size_t tapfdSize,
- char **vhostfd,
- size_t vhostfdSize,
- const char *slirpfd,
- const char *vdpadev);
+virJSONValue *
+qemuBuildHostNetProps(virDomainNetDef *net,
+ char **tapfd,
+ size_t tapfdSize,
+ char **vhostfd,
+ size_t vhostfdSize,
+ const char *slirpfd,
+ const char *vdpadev);
/* Current, best practice */
virJSONValue *
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6f667dfb76..3df0a1f17a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1459,10 +1459,10 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
vdpafdName = g_strdup_printf("/dev/fdset/%d", fdinfo.fdset);
}
- if (!(netprops = qemuBuildHostNetStr(net,
- tapfdName, tapfdSize,
- vhostfdName, vhostfdSize,
- slirpfdName, vdpafdName))) {
+ if (!(netprops = qemuBuildHostNetProps(net,
+ tapfdName, tapfdSize,
+ vhostfdName, vhostfdSize,
+ slirpfdName, vdpafdName))) {
ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;
}
--
2.31.1
4:34 a.m.
New subject: [PATCH 07/16] qemu: command: Rename qemuBuildHostNetStr -> qemuBuildHostNetProps
On a Thursday in 2021, Peter Krempa wrote:
The function is already returning JSON properties, rename it
accordingly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 22 +++++++++++-----------
src/qemu/qemu_command.h | 15 ++++++++-------
src/qemu/qemu_hotplug.c | 8 ++++----
3 files changed, 23 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 08/16] qemuBuildChrChardevStr: Use proper type for the switch statement
Add the missing cases.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9293a413e6..f97cdc70ab 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5137,7 +5137,7 @@ qemuBuildChrChardevStr(virLogManager *logManager,
if (!(charAlias = qemuAliasChardevFromDevAlias(alias)))
return NULL;
- switch (dev->type) {
+ switch ((virDomainChrType) dev->type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
virBufferAsprintf(&buf, "null,id=%s", charAlias);
break;
@@ -5290,6 +5290,8 @@ qemuBuildChrChardevStr(virLogManager *logManager,
dev->data.spiceport.channel);
break;
+ case VIR_DOMAIN_CHR_TYPE_NMDM:
+ case VIR_DOMAIN_CHR_TYPE_LAST:
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported chardev '%s'"),
--
2.31.1
4:35 a.m.
New subject: [PATCH 08/16] qemuBuildChrChardevStr: Use proper type for the switch statement
On a Thursday in 2021, Peter Krempa wrote:
Add the missing cases.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 09/16] conf: Convert 'chr' in virDomainShmemDef to proper pointer
The main reason is to ensure that the private data are properly
allocated for every instance.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_audit.c | 2 +-
src/conf/domain_conf.c | 17 ++++++++++-------
src/conf/domain_conf.h | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 6 +++---
src/qemu/qemu_hotplug.c | 2 +-
6 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index b08233fa49..69c5792b07 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -898,7 +898,7 @@ virDomainAuditShmem(virDomainObj *vm,
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname = virAuditEncode("vm", vm->def->name);
- const char *srcpath = virDomainChrSourceDefGetPath(&def->server.chr);
+ const char *srcpath = virDomainChrSourceDefGetPath(def->server.chr);
const char *virt = virDomainAuditGetVirtType(vm->def);
char *shmpath = NULL;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index da0c64b460..52f513f488 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3069,7 +3069,7 @@ void virDomainShmemDefFree(virDomainShmemDef *def)
return;
virDomainDeviceInfoClear(&def->info);
- virDomainChrSourceDefClear(&def->server.chr);
+ virObjectUnref(def->server.chr);
g_free(def->name);
g_free(def);
}
@@ -13665,11 +13665,14 @@ virDomainShmemDefParseXML(virDomainXMLOption *xmlopt,
if ((server = virXPathNode("./server[1]", ctxt))) {
g_autofree char *tmp = NULL;
+ if (!(def->server.chr = virDomainChrSourceDefNew(xmlopt)))
+ return NULL;
+
def->server.enabled = true;
- def->server.chr.type = VIR_DOMAIN_CHR_TYPE_UNIX;
- def->server.chr.data.nix.listen = false;
+ def->server.chr->type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ def->server.chr->data.nix.listen = false;
if ((tmp = virXMLPropString(server, "path")))
- def->server.chr.data.nix.path = virFileSanitizePath(tmp);
+ def->server.chr->data.nix.path = virFileSanitizePath(tmp);
}
if ((msi = virXPathNode("./msi[1]", ctxt))) {
@@ -16814,8 +16817,8 @@ virDomainShmemDefEquals(virDomainShmemDef *src,
return false;
if (src->server.enabled) {
- if (STRNEQ_NULLABLE(src->server.chr.data.nix.path,
- dst->server.chr.data.nix.path))
+ if (STRNEQ_NULLABLE(src->server.chr->data.nix.path,
+ dst->server.chr->data.nix.path))
return false;
}
@@ -25868,7 +25871,7 @@ virDomainShmemDefFormat(virBuffer *buf,
if (def->server.enabled) {
virBufferAddLit(buf, "<server");
- virBufferEscapeString(buf, " path='%s'",
def->server.chr.data.nix.path);
+ virBufferEscapeString(buf, " path='%s'",
def->server.chr->data.nix.path);
virBufferAddLit(buf, "/>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ab9a7d66f8..3cb68c5d0a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1997,7 +1997,7 @@ struct _virDomainShmemDef {
virDomainShmemRole role;
struct {
bool enabled;
- virDomainChrSourceDef chr;
+ virDomainChrSourceDef *chr;
} server;
struct {
bool enabled;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f97cdc70ab..60336947b5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9368,7 +9368,7 @@ qemuBuildShmemCommandLine(virLogManager *logManager,
if (shmem->server.enabled) {
chardev = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
- &shmem->server.chr,
+ shmem->server.chr,
shmem->info.alias, qemuCaps,
cdevflags);
if (!chardev)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fb203bc830..e69215cee2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9964,11 +9964,11 @@ void
qemuDomainPrepareShmemChardev(virDomainShmemDef *shmem)
{
if (!shmem->server.enabled ||
- shmem->server.chr.data.nix.path)
+ shmem->server.chr->data.nix.path)
return;
- shmem->server.chr.data.nix.path =
g_strdup_printf("/var/lib/libvirt/shmem-%s-sock",
- shmem->name);
+ shmem->server.chr->data.nix.path =
g_strdup_printf("/var/lib/libvirt/shmem-%s-sock",
+ shmem->name);
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3df0a1f17a..0e798dc237 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3064,7 +3064,7 @@ qemuDomainAttachShmemDevice(virQEMUDriver *driver,
qemuDomainObjEnterMonitor(driver, vm);
if (shmem->server.enabled) {
- if (qemuHotplugChardevAttach(priv->mon, charAlias, &shmem->server.chr)
< 0)
+ if (qemuHotplugChardevAttach(priv->mon, charAlias, shmem->server.chr) <
0)
goto exit_monitor;
} else {
if (qemuMonitorAddObject(priv->mon, &props, &memAlias) < 0)
--
2.31.1
1:49 a.m.
New subject: [PATCH 09/16] conf: Convert 'chr' in virDomainShmemDef to proper pointer
On Thu, Nov 18, 2021 at 17:33:34 +0100, Peter Krempa wrote:
The main reason is to ensure that the private data are properly
allocated for every instance.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_audit.c | 2 +-
src/conf/domain_conf.c | 17 ++++++++++-------
src/conf/domain_conf.h | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 6 +++---
src/qemu/qemu_hotplug.c | 2 +-
6 files changed, 17 insertions(+), 14 deletions(-)
[...]
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ab9a7d66f8..3cb68c5d0a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1997,7 +1997,7 @@ struct _virDomainShmemDef {
virDomainShmemRole role;
struct {
bool enabled;
- virDomainChrSourceDef chr;
+ virDomainChrSourceDef *chr;
} server;
struct {
bool enabled;
Apparently I've misplaced my hack to build the apparmor security driver
on my dev box, so the following diff is needed to build properly:
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 218e07bfb0..b7ffb5e2c3 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1172,8 +1172,8 @@ get_files(vahControl * ctl)
* When the server path is enabled, use it - otherwise fallback to
* model dependent defaults. */
if (shmem->server.enabled &&
- shmem->server.chr.data.nix.path) {
- if (vah_add_file(&buf, shmem->server.chr.data.nix.path,
+ shmem->server.chr->data.nix.path) {
+ if (vah_add_file(&buf, shmem->server.chr->data.nix.path,
"rw") != 0)
goto cleanup;
} else {
4:36 a.m.
New subject: [PATCH 09/16] conf: Convert 'chr' in virDomainShmemDef to proper pointer
On a Friday in 2021, Peter Krempa wrote:
On Thu, Nov 18, 2021 at 17:33:34 +0100, Peter Krempa wrote:
> The main reason is to ensure that the private data are properly
> allocated for every instance.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/conf/domain_audit.c | 2 +-
> src/conf/domain_conf.c | 17 ++++++++++-------
> src/conf/domain_conf.h | 2 +-
> src/qemu/qemu_command.c | 2 +-
> src/qemu/qemu_domain.c | 6 +++---
> src/qemu/qemu_hotplug.c | 2 +-
> 6 files changed, 17 insertions(+), 14 deletions(-)
[...]
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ab9a7d66f8..3cb68c5d0a 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1997,7 +1997,7 @@ struct _virDomainShmemDef {
> virDomainShmemRole role;
> struct {
> bool enabled;
> - virDomainChrSourceDef chr;
> + virDomainChrSourceDef *chr;
> } server;
> struct {
> bool enabled;
Apparently I've misplaced my hack to build the apparmor security driver
on my dev box, so the following diff is needed to build properly:
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 10/16] qemuDomainAddChardevTLSObjects: Refactor cleanup
Switch to automatic memory clearing for the two virJSONValues and remove
the 'cleanup' label and 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0e798dc237..c4f11edf6b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1863,25 +1863,22 @@ qemuDomainAddChardevTLSObjects(virQEMUDriver *driver,
char **tlsAlias,
const char **secAlias)
{
- int ret = -1;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivate *priv = vm->privateData;
qemuDomainChrSourcePrivate *chrSourcePriv;
qemuDomainSecretInfo *secinfo = NULL;
- virJSONValue *tlsProps = NULL;
- virJSONValue *secProps = NULL;
+ g_autoptr(virJSONValue) tlsProps = NULL;
+ g_autoptr(virJSONValue) secProps = NULL;
/* NB: This may alter haveTLS based on cfg */
qemuDomainPrepareChardevSourceTLS(dev, cfg);
if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP ||
- dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) {
- ret = 0;
- goto cleanup;
- }
+ dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES)
+ return 0;
if (qemuDomainSecretChardevPrepare(cfg, priv, devAlias, dev) < 0)
- goto cleanup;
+ return -1;
if ((chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev)))
secinfo = chrSourcePriv->secinfo;
@@ -1890,27 +1887,22 @@ qemuDomainAddChardevTLSObjects(virQEMUDriver *driver,
*secAlias = secinfo->alias;
if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(charAlias)))
- goto cleanup;
+ return -1;
if (qemuDomainGetTLSObjects(secinfo,
cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
*tlsAlias, &tlsProps, &secProps) < 0)
- goto cleanup;
+ return -1;
+
dev->data.tcp.tlscreds = true;
if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
&secProps, &tlsProps) < 0)
- goto cleanup;
-
- ret = 0;
-
- cleanup:
- virJSONValueFree(tlsProps);
- virJSONValueFree(secProps);
+ return -1;
- return ret;
+ return 0;
}
--
2.31.1
4:37 a.m.
New subject: [PATCH 10/16] qemuDomainAddChardevTLSObjects: Refactor cleanup
On a Thursday in 2021, Peter Krempa wrote:
Switch to automatic memory clearing for the two virJSONValues and
remove
the 'cleanup' label and 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 11/16] qemuDomainEnsurePCIAddress: Don't pass virQEMUDriver explicitly
The function is already getting 'virDomainObj' which has already the
driver pointer present.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain_address.c | 13 +++++--------
src/qemu/qemu_domain_address.h | 8 +++-----
src/qemu/qemu_hotplug.c | 21 ++++++++++-----------
3 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index c43ad23cf5..9abe2b84c8 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -3034,8 +3034,7 @@ qemuAssignMemoryDeviceSlot(virDomainMemoryDef *mem,
int
-qemuDomainAssignMemoryDeviceSlot(virQEMUDriver *driver,
- virDomainObj *vm,
+qemuDomainAssignMemoryDeviceSlot(virDomainObj *vm,
virDomainMemoryDef *mem)
{
g_autoptr(virBitmap) slotmap = NULL;
@@ -3052,7 +3051,7 @@ qemuDomainAssignMemoryDeviceSlot(virQEMUDriver *driver,
case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
- return qemuDomainEnsurePCIAddress(vm, &dev, driver);
+ return qemuDomainEnsurePCIAddress(vm, &dev);
break;
case VIR_DOMAIN_MEMORY_MODEL_NONE:
@@ -3232,8 +3231,7 @@ qemuDomainAssignAddresses(virDomainDef *def,
*/
int
qemuDomainEnsurePCIAddress(virDomainObj *obj,
- virDomainDeviceDef *dev,
- virQEMUDriver *driver)
+ virDomainDeviceDef *dev)
{
qemuDomainObjPrivate *priv = obj->privateData;
virDomainDeviceInfo *info = virDomainDeviceGetInfo(dev);
@@ -3241,7 +3239,7 @@ qemuDomainEnsurePCIAddress(virDomainObj *obj,
if (!info)
return 0;
- qemuDomainFillDevicePCIConnectFlags(obj->def, dev, priv->qemuCaps, driver);
+ qemuDomainFillDevicePCIConnectFlags(obj->def, dev, priv->qemuCaps,
priv->driver);
qemuDomainFillDevicePCIExtensionFlags(dev, info, priv->qemuCaps);
@@ -3272,7 +3270,6 @@ qemuDomainEnsureVirtioAddress(bool *releaseAddr,
virDomainDeviceInfo *info = virDomainDeviceGetInfo(dev);
qemuDomainObjPrivate *priv = vm->privateData;
virDomainCCWAddressSet *ccwaddrs = NULL;
- virQEMUDriver *driver = priv->driver;
int ret = -1;
if (!info->type) {
@@ -3289,7 +3286,7 @@ qemuDomainEnsureVirtioAddress(bool *releaseAddr,
goto cleanup;
} else if (!info->type ||
info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (qemuDomainEnsurePCIAddress(vm, dev, driver) < 0)
+ if (qemuDomainEnsurePCIAddress(vm, dev) < 0)
goto cleanup;
*releaseAddr = true;
}
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index df7e7fca99..4c360a0f70 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -45,9 +45,8 @@ int qemuDomainAssignAddresses(virDomainDef *def,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int qemuDomainEnsurePCIAddress(virDomainObj *obj,
- virDomainDeviceDef *dev,
- virQEMUDriver *driver)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+ virDomainDeviceDef *dev)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void qemuDomainFillDeviceIsolationGroup(virDomainDef *def,
virDomainDeviceDef *dev)
@@ -56,8 +55,7 @@ void qemuDomainFillDeviceIsolationGroup(virDomainDef *def,
void qemuDomainReleaseDeviceAddress(virDomainObj *vm,
virDomainDeviceInfo *info);
-int qemuDomainAssignMemoryDeviceSlot(virQEMUDriver *driver,
- virDomainObj *vm,
+int qemuDomainAssignMemoryDeviceSlot(virDomainObj *vm,
virDomainMemoryDef *mem);
void qemuDomainReleaseMemoryDeviceSlot(virDomainObj *vm,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c4f11edf6b..d44523fe7e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1254,7 +1254,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
!net->info.addr.ccw.assigned) < 0)
goto cleanup;
- } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) {
+ } else if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) {
goto cleanup;
}
@@ -1706,7 +1706,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver,
/* Isolation groups are only relevant for pSeries guests */
qemuDomainFillDeviceIsolationGroup(vm->def, &dev);
- if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ if (qemuDomainEnsurePCIAddress(vm, &dev) < 0)
goto error;
releaseaddr = true;
@@ -2131,8 +2131,7 @@ qemuDomainChrRemove(virDomainDef *vmdef,
*/
static int
qemuDomainAttachChrDeviceAssignAddr(virDomainObj *vm,
- virDomainChrDef *chr,
- virQEMUDriver *driver)
+ virDomainChrDef *chr)
{
virDomainDef *def = vm->def;
qemuDomainObjPrivate *priv = vm->privateData;
@@ -2146,7 +2145,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObj *vm,
} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
- if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ if (qemuDomainEnsurePCIAddress(vm, &dev) < 0)
return -1;
return 1;
@@ -2204,7 +2203,7 @@ int qemuDomainAttachChrDevice(virQEMUDriver *driver,
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
goto cleanup;
- if ((rc = qemuDomainAttachChrDeviceAssignAddr(vm, chr, driver)) < 0)
+ if ((rc = qemuDomainAttachChrDeviceAssignAddr(vm, chr)) < 0)
goto cleanup;
if (rc == 1)
need_release = true;
@@ -2442,7 +2441,7 @@ qemuDomainAttachMemory(virQEMUDriver *driver,
if (qemuDomainDefValidateMemoryHotplug(vm->def, mem) < 0)
goto cleanup;
- if (qemuDomainAssignMemoryDeviceSlot(driver, vm, mem) < 0)
+ if (qemuDomainAssignMemoryDeviceSlot(vm, mem) < 0)
goto cleanup;
releaseaddr = true;
@@ -2777,7 +2776,7 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriver *driver,
if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ if (qemuDomainEnsurePCIAddress(vm, &dev) < 0)
goto cleanup;
} else if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
@@ -2870,7 +2869,7 @@ qemuDomainAttachMediatedDevice(virQEMUDriver *driver,
switch (hostdev->source.subsys.u.mdev.model) {
case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
- if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ if (qemuDomainEnsurePCIAddress(vm, &dev) < 0)
return -1;
break;
case VIR_MDEV_MODEL_TYPE_VFIO_CCW: {
@@ -3039,7 +3038,7 @@ qemuDomainAttachShmemDevice(virQEMUDriver *driver,
if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
- (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0))
+ (qemuDomainEnsurePCIAddress(vm, &dev) < 0))
return -1;
if (!(devProps = qemuBuildShmemDevProps(vm->def, shmem)))
@@ -3136,7 +3135,7 @@ qemuDomainAttachWatchdog(virQEMUDriver *driver,
return -1;
if (watchdog->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) {
- if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ if (qemuDomainEnsurePCIAddress(vm, &dev) < 0)
goto cleanup;
releaseAddress = true;
} else {
--
2.31.1
4:37 a.m.
New subject: [PATCH 11/16] qemuDomainEnsurePCIAddress: Don't pass virQEMUDriver explicitly
On a Thursday in 2021, Peter Krempa wrote:
The function is already getting 'virDomainObj' which has
already the
driver pointer present.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain_address.c | 13 +++++--------
src/qemu/qemu_domain_address.h | 8 +++-----
src/qemu/qemu_hotplug.c | 21 ++++++++++-----------
3 files changed, 18 insertions(+), 24 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 12/16] qemuDomainAttachChrDeviceAssignAddr: Simplify return value handling
Rather than returning a different error code if the device address needs
to be released pass in the 'need_release' flag via a pointer.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d44523fe7e..5e44210aff 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2125,13 +2125,12 @@ qemuDomainChrRemove(virDomainDef *vmdef,
return ret;
}
-/* Returns 1 if the address will need to be released later,
- * -1 on error
- * 0 otherwise
- */
+
+
static int
qemuDomainAttachChrDeviceAssignAddr(virDomainObj *vm,
- virDomainChrDef *chr)
+ virDomainChrDef *chr,
+ bool *need_release)
{
virDomainDef *def = vm->def;
qemuDomainObjPrivate *priv = vm->privateData;
@@ -2147,13 +2146,16 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObj *vm,
chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
if (qemuDomainEnsurePCIAddress(vm, &dev) < 0)
return -1;
- return 1;
+ *need_release = true;
+ return 0;
} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
if (virDomainUSBAddressEnsure(priv->usbaddrs, &chr->info) < 0)
return -1;
- return 1;
+
+ *need_release = true;
+ return 0;
} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) {
@@ -2176,7 +2178,7 @@ int qemuDomainAttachChrDevice(virQEMUDriver *driver,
virDomainObj *vm,
virDomainChrDef *chr)
{
- int ret = -1, rc;
+ int ret = -1;
qemuDomainObjPrivate *priv = vm->privateData;
virErrorPtr orig_err;
virDomainDef *vmdef = vm->def;
@@ -2203,10 +2205,8 @@ int qemuDomainAttachChrDevice(virQEMUDriver *driver,
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
goto cleanup;
- if ((rc = qemuDomainAttachChrDeviceAssignAddr(vm, chr)) < 0)
+ if (qemuDomainAttachChrDeviceAssignAddr(vm, chr, &need_release) < 0)
goto cleanup;
- if (rc == 1)
- need_release = true;
if (qemuDomainNamespaceSetupChardev(vm, chr, &teardowndevice) < 0)
goto cleanup;
--
2.31.1
4:41 a.m.
New subject: [PATCH 12/16] qemuDomainAttachChrDeviceAssignAddr: Simplify return value handling
On a Thursday in 2021, Peter Krempa wrote:
Rather than returning a different error code if the device address
needs
to be released pass in the 'need_release' flag via a pointer.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 13/16] qemuDomainAttachChrDevice: Drop 'dev' variable
It's referenced only once and it's a shortcut to the chardev source thus
can be used directly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5e44210aff..3373ec2cdf 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2184,7 +2184,6 @@ int qemuDomainAttachChrDevice(virQEMUDriver *driver,
virDomainDef *vmdef = vm->def;
g_autoptr(virJSONValue) devprops = NULL;
g_autoptr(virJSONValue) netdevprops = NULL;
- virDomainChrSourceDef *dev = chr->source;
g_autofree char *charAlias = NULL;
bool chardevAttached = false;
bool teardowncgroup = false;
@@ -2233,7 +2232,7 @@ int qemuDomainAttachChrDevice(virQEMUDriver *driver,
if (qemuDomainChrPreInsert(vmdef, chr) < 0)
goto cleanup;
- if (qemuDomainAddChardevTLSObjects(driver, vm, dev,
+ if (qemuDomainAddChardevTLSObjects(driver, vm, chr->source,
chr->info.alias, charAlias,
&tlsAlias, &secAlias) < 0)
goto audit;
--
2.31.1
4:42 a.m.
New subject: [PATCH 13/16] qemuDomainAttachChrDevice: Drop 'dev' variable
On a Thursday in 2021, Peter Krempa wrote:
It's referenced only once and it's a shortcut to the chardev
source thus
can be used directly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 14/16] qemuBuildStorageSourceAttachPrepare(Drive|Chardev): Unexport
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_command.h | 7 -------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 60336947b5..f3b02d3438 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -11104,7 +11104,7 @@ qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu)
* Prepare qemuBlockStorageSourceAttachData *for use with the old approach
* using -drive/drive_add. See qemuBlockStorageSourceAttachPrepareBlockdev.
*/
-qemuBlockStorageSourceAttachData *
+static qemuBlockStorageSourceAttachData *
qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDef *disk,
virQEMUCaps *qemuCaps)
{
@@ -11127,7 +11127,7 @@ qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDef *disk,
* Prepare qemuBlockStorageSourceAttachData *for vhost-user disk
* to be used with -chardev.
*/
-qemuBlockStorageSourceAttachData *
+static qemuBlockStorageSourceAttachData *
qemuBuildStorageSourceAttachPrepareChardev(virDomainDiskDef *disk)
{
g_autoptr(qemuBlockStorageSourceAttachData) data = NULL;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index c544c9ec67..7dec2cb4af 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -106,13 +106,6 @@ qemuBuildNicDevProps(virDomainDef *def,
char *qemuDeviceDriveHostAlias(virDomainDiskDef *disk);
bool qemuDiskBusIsSD(int bus);
-qemuBlockStorageSourceAttachData *
-qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDef *disk,
- virQEMUCaps *qemuCaps);
-
-qemuBlockStorageSourceAttachData *
-qemuBuildStorageSourceAttachPrepareChardev(virDomainDiskDef *disk);
-
int
qemuBuildStorageSourceAttachPrepareCommon(virStorageSource *src,
qemuBlockStorageSourceAttachData *data);
--
2.31.1
4:42 a.m.
New subject: [PATCH 14/16] qemuBuildStorageSourceAttachPrepare(Drive|Chardev): Unexport
On a Thursday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_command.h | 7 -------
2 files changed, 2 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 15/16] conf: Properly instantiate virDomainChrSourceDef in virDomainTPMDef
'virDomainChrSourceDef' contains private data so
'virDomainChrSourceDefNew'
must be used to allocate it. 'virDomainTPMDef' was using it directly
which won't work with the chardev helper functions.
Convert it to a pointer to properly allocate private data.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_audit.c | 4 ++--
src/conf/domain_conf.c | 14 +++++++++-----
src/conf/domain_conf.h | 4 ++--
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_command.c | 6 +++---
src/qemu/qemu_namespace.c | 2 +-
src/qemu/qemu_tpm.c | 10 +++++-----
src/security/security_dac.c | 6 +++---
src/security/security_selinux.c | 6 +++---
tests/qemuxml2argvtest.c | 6 +++---
10 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 69c5792b07..17a01c51ba 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -536,7 +536,7 @@ virDomainAuditTPM(virDomainObj *vm, virDomainTPMDef *tpm,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- path = tpm->data.passthrough.source.data.file.path;
+ path = tpm->data.passthrough.source->data.file.path;
if (!(device = virAuditEncode("device", VIR_AUDIT_STR(path)))) {
VIR_WARN("OOM while encoding audit message");
goto cleanup;
@@ -547,7 +547,7 @@ virDomainAuditTPM(virDomainObj *vm, virDomainTPMDef *tpm,
virt, reason, vmname, uuidstr, device);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- path = tpm->data.emulator.source.data.nix.path;
+ path = tpm->data.emulator.source->data.nix.path;
if (!(device = virAuditEncode("device", VIR_AUDIT_STR(path)))) {
VIR_WARN("OOM while encoding audit message");
goto cleanup;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 52f513f488..7231d8fc3f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3211,10 +3211,10 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- virDomainChrSourceDefClear(&def->data.passthrough.source);
+ virObjectUnref(def->data.passthrough.source);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- virDomainChrSourceDefClear(&def->data.emulator.source);
+ virObjectUnref(def->data.emulator.source);
g_free(def->data.emulator.storagepath);
g_free(def->data.emulator.logfile);
break;
@@ -11831,13 +11831,17 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+ if (!(def->data.passthrough.source = virDomainChrSourceDefNew(xmlopt)))
+ goto error;
path = virXPathString("string(./backend/device/@path)", ctxt);
if (!path)
path = g_strdup(VIR_DOMAIN_TPM_DEFAULT_DEVICE);
- def->data.passthrough.source.data.file.path = g_steal_pointer(&path);
- def->data.passthrough.source.type = VIR_DOMAIN_CHR_TYPE_DEV;
+ def->data.passthrough.source->type = VIR_DOMAIN_CHR_TYPE_DEV;
+ def->data.passthrough.source->data.file.path = g_steal_pointer(&path);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ if (!(def->data.emulator.source = virDomainChrSourceDefNew(xmlopt)))
+ goto error;
secretuuid = virXPathString("string(./backend/encryption/@secret)",
ctxt);
if (secretuuid) {
if (virUUIDParse(secretuuid, def->data.emulator.secretuuid) < 0) {
@@ -25456,7 +25460,7 @@ virDomainTPMDefFormat(virBuffer *buf,
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
virBufferEscapeString(buf, "<device path='%s'/>\n",
- def->data.passthrough.source.data.file.path);
+ def->data.passthrough.source->data.file.path);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</backend>\n");
break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3cb68c5d0a..c1b2a814aa 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1381,10 +1381,10 @@ struct _virDomainTPMDef {
int version; /* virDomainTPMVersion */
union {
struct {
- virDomainChrSourceDef source;
+ virDomainChrSourceDef *source;
} passthrough;
struct {
- virDomainChrSourceDef source;
+ virDomainChrSourceDef *source;
char *storagepath;
char *logfile;
unsigned char secretuuid[VIR_UUID_BUFLEN];
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 471cbc3b8f..1e7b562b33 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -340,7 +340,7 @@ qemuSetupTPMCgroup(virDomainObj *vm,
switch (dev->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- ret = qemuSetupChrSourceCgroup(vm, &dev->data.passthrough.source);
+ ret = qemuSetupChrSourceCgroup(vm, dev->data.passthrough.source);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
case VIR_DOMAIN_TPM_TYPE_LAST:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f3b02d3438..623e3a20a9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9947,7 +9947,7 @@ qemuBuildTPMBackendStr(virCommand *cmd,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- tpmdev = tpm->data.passthrough.source.data.file.path;
+ tpmdev = tpm->data.passthrough.source->data.file.path;
if (!(cancel_path = virTPMCreateCancelPath(tpmdev)))
return NULL;
@@ -9972,7 +9972,7 @@ qemuBuildTPMBackendStr(virCommand *cmd,
virBufferAddLit(&buf, ",chardev=chrtpm");
*chardev = g_strdup_printf("socket,id=chrtpm,path=%s",
- tpm->data.emulator.source.data.nix.path);
+ tpm->data.emulator.source->data.nix.path);
break;
case VIR_DOMAIN_TPM_TYPE_LAST:
@@ -10041,7 +10041,7 @@ qemuBuildTPMProxyCommandLine(virCommand *cmd,
if (virJSONValueObjectAdd(&props,
"s:driver",
virDomainTPMModelTypeToString(tpm->model),
"s:id", tpm->info.alias,
- "s:host-path",
tpm->data.passthrough.source.data.file.path,
+ "s:host-path",
tpm->data.passthrough.source->data.file.path,
NULL) < 0)
return -1;
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index f1aaca86b1..23b1160c5e 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -422,7 +422,7 @@ qemuDomainSetupTPM(virDomainTPMDef *dev,
{
switch (dev->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- *paths = g_slist_prepend(*paths,
g_strdup(dev->data.passthrough.source.data.file.path));
+ *paths = g_slist_prepend(*paths,
g_strdup(dev->data.passthrough.source->data.file.path));
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 7d05394356..62f54f56ab 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -332,11 +332,11 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm,
return -1;
/* create the socket filename */
- if (!tpm->data.emulator.source.data.nix.path &&
- !(tpm->data.emulator.source.data.nix.path =
+ if (!tpm->data.emulator.source->data.nix.path &&
+ !(tpm->data.emulator.source->data.nix.path =
qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName)))
return -1;
- tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ tpm->data.emulator.source->type = VIR_DOMAIN_CHR_TYPE_UNIX;
return 0;
}
@@ -716,7 +716,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
secretuuid) < 0)
goto error;
- unlink(tpm->data.emulator.source.data.nix.path);
+ unlink(tpm->data.emulator.source->data.nix.path);
cmd = virCommandNew(swtpm);
if (!cmd)
@@ -726,7 +726,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
virCommandAddArgList(cmd, "socket", "--daemon",
"--ctrl", NULL);
virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
- tpm->data.emulator.source.data.nix.path);
+ tpm->data.emulator.source->data.nix.path);
virCommandAddArg(cmd, "--tpmstate");
virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 1733d63410..e9e316551e 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1686,12 +1686,12 @@ virSecurityDACSetTPMFileLabel(virSecurityManager *mgr,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
ret = virSecurityDACSetChardevLabelHelper(mgr, def,
- &tpm->data.passthrough.source,
+ tpm->data.passthrough.source,
false, false);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
ret = virSecurityDACSetChardevLabelHelper(mgr, def,
- &tpm->data.emulator.source,
+ tpm->data.emulator.source,
false, false);
break;
case VIR_DOMAIN_TPM_TYPE_LAST:
@@ -1712,7 +1712,7 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManager *mgr,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
ret = virSecurityDACRestoreChardevLabelHelper(mgr, def,
-
&tpm->data.passthrough.source,
+ tpm->data.passthrough.source,
false, false);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 622a8f4c02..840a05844e 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1637,7 +1637,7 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManager *mgr,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- tpmdev = tpm->data.passthrough.source.data.file.path;
+ tpmdev = tpm->data.passthrough.source->data.file.path;
rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel, false);
if (rc < 0)
return -1;
@@ -1656,7 +1656,7 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManager *mgr,
}
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- tpmdev = tpm->data.emulator.source.data.nix.path;
+ tpmdev = tpm->data.emulator.source->data.nix.path;
rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel, false);
if (rc < 0)
return -1;
@@ -1685,7 +1685,7 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManager *mgr,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- tpmdev = tpm->data.passthrough.source.data.file.path;
+ tpmdev = tpm->data.passthrough.source->data.file.path;
rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev, false);
if ((cancel_path = virTPMCreateCancelPath(tpmdev)) != NULL) {
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 161e7efa62..1d0d6e14ba 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -450,9 +450,9 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv,
if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
continue;
- VIR_FREE(vm->def->tpms[i]->data.emulator.source.data.file.path);
- vm->def->tpms[i]->data.emulator.source.data.file.path =
g_strdup("/dev/test");
- vm->def->tpms[i]->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_FILE;
+ VIR_FREE(vm->def->tpms[i]->data.emulator.source->data.file.path);
+ vm->def->tpms[i]->data.emulator.source->data.file.path =
g_strdup("/dev/test");
+ vm->def->tpms[i]->data.emulator.source->type =
VIR_DOMAIN_CHR_TYPE_FILE;
}
for (i = 0; i < vm->def->nvideos; i++) {
--
2.31.1
4:43 a.m.
New subject: [PATCH 15/16] conf: Properly instantiate virDomainChrSourceDef in virDomainTPMDef
On a Thursday in 2021, Peter Krempa wrote:
'virDomainChrSourceDef' contains private data so
'virDomainChrSourceDefNew'
must be used to allocate it. 'virDomainTPMDef' was using it directly
which won't work with the chardev helper functions.
Convert it to a pointer to properly allocate private data.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_audit.c | 4 ++--
src/conf/domain_conf.c | 14 +++++++++-----
src/conf/domain_conf.h | 4 ++--
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_command.c | 6 +++---
src/qemu/qemu_namespace.c | 2 +-
src/qemu/qemu_tpm.c | 10 +++++-----
src/security/security_dac.c | 6 +++---
src/security/security_selinux.c | 6 +++---
tests/qemuxml2argvtest.c | 6 +++---
10 files changed, 32 insertions(+), 28 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:33 a.m.
New subject: [PATCH 16/16] qemuxml2argvtest: Fix type for faked chardev backing a TPM
The test filled the chardev type to VIR_DOMAIN_CHR_TYPE_FILE and thus
set the 'data.emulator.source->data.file.path' pointer, but the
commandline formatter is unconditionally expecting VIR_DOMAIN_CHR_TYPE_UNIX
and thus reading 'data.emulator.source->data.nix.path'. Since it's an
union it happened to land in the correct place. Fix the faked data.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvtest.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1d0d6e14ba..a5d162958e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -450,9 +450,9 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv,
if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
continue;
- VIR_FREE(vm->def->tpms[i]->data.emulator.source->data.file.path);
- vm->def->tpms[i]->data.emulator.source->data.file.path =
g_strdup("/dev/test");
- vm->def->tpms[i]->data.emulator.source->type =
VIR_DOMAIN_CHR_TYPE_FILE;
+ VIR_FREE(vm->def->tpms[i]->data.emulator.source->data.nix.path);
+ vm->def->tpms[i]->data.emulator.source->type =
VIR_DOMAIN_CHR_TYPE_UNIX;
+ vm->def->tpms[i]->data.emulator.source->data.nix.path =
g_strdup("/dev/test");
}
for (i = 0; i < vm->def->nvideos; i++) {
--
2.31.1
4:44 a.m.
New subject: [PATCH 16/16] qemuxml2argvtest: Fix type for faked chardev backing a TPM
On a Thursday in 2021, Peter Krempa wrote:
The test filled the chardev type to VIR_DOMAIN_CHR_TYPE_FILE and thus
set the 'data.emulator.source->data.file.path' pointer, but the
commandline formatter is unconditionally expecting VIR_DOMAIN_CHR_TYPE_UNIX
and thus reading 'data.emulator.source->data.nix.path'. Since it's an
union it happened to land in the correct place. Fix the faked data.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvtest.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1134
days inactive
1135
days old
33 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa