[PATCH 00/16] chardev handling cleanups (chardev refactors part 1)
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
Use the customary '< 0' check for return value. Signed-off-by: Peter Krempa <pkrempa@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
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@redhat.com> --- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
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@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
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@redhat.com> --- src/qemu/qemu_hotplug.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use 'qemuMonitorJSONMakeCommandInternal' instead of 'qemuMonitorJSONMakeCommand' + 'virJSONValueObjectAppend'. Signed-off-by: Peter Krempa <pkrempa@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
On a Thursday in 2021, Peter Krempa wrote:
Use 'qemuMonitorJSONMakeCommandInternal' instead of 'qemuMonitorJSONMakeCommand' + 'virJSONValueObjectAppend'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Move the addition of the command wrapper to qemuMonitorJSONAttachCharDev and rename the function to qemuMonitorJSONAttachCharDevGetProps. Signed-off-by: Peter Krempa <pkrempa@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
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@redhat.com> --- src/qemu/qemu_monitor_json.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use 'virReportEnumRangeError' for the invalid cases and keep the original error for known but unsupported chardevs. Signed-off-by: Peter Krempa <pkrempa@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
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@redhat.com> --- src/qemu/qemu_monitor_json.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use 'virJSONValueObjectAdd' instead of the step-by-step manual JSON object building. Signed-off-by: Peter Krempa <pkrempa@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
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@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@redhat.com> Jano
The function is already returning JSON properties, rename it accordingly. Signed-off-by: Peter Krempa <pkrempa@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
On a Thursday in 2021, Peter Krempa wrote:
The function is already returning JSON properties, rename it accordingly.
Signed-off-by: Peter Krempa <pkrempa@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@redhat.com> Jano
Add the missing cases. Signed-off-by: Peter Krempa <pkrempa@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
On a Thursday in 2021, Peter Krempa wrote:
Add the missing cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The main reason is to ensure that the private data are properly allocated for every instance. Signed-off-by: Peter Krempa <pkrempa@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
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@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 {
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@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@redhat.com> Jano
Switch to automatic memory clearing for the two virJSONValues and remove the 'cleanup' label and 'ret' variable. Signed-off-by: Peter Krempa <pkrempa@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
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@redhat.com> --- src/qemu/qemu_hotplug.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The function is already getting 'virDomainObj' which has already the driver pointer present. Signed-off-by: Peter Krempa <pkrempa@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
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@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@redhat.com> Jano
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@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
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@redhat.com> --- src/qemu/qemu_hotplug.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
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@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
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@redhat.com> --- src/qemu/qemu_hotplug.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@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
On a Thursday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@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@redhat.com> Jano
'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@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
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@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@redhat.com> Jano
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@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
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@redhat.com> --- tests/qemuxml2argvtest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa