[libvirt] [PATCH 0/7] qemu_command: clean up more labels (virtio-fs epopee)

Ján Tomko (7): qemuBuildHostNetStr: remove unused cfg qemuBuildHostNetStr: remove unused 'driver' argument qemuBuildHostNetStr: remove unnecessary cleanup label qemuBuildSoundCommandLine: reduce scope of codecstr qemuBuildMemoryBackendProps: remove useless cleanup label qemuBuildMemoryBackendProps: use 'rc' instead of ret. qemuBuildMemoryCellBackendStr: remove useless ret variable src/qemu/qemu_command.c | 87 ++++++++++++++++++----------------------- src/qemu/qemu_command.h | 1 - src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 38 insertions(+), 52 deletions(-) -- 2.19.2

As of commit 2d80fbb14dffa45fe3fcd2c3f29ce54857bb766c this variable is unused. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c3931b6744..fbc9a73035 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3904,7 +3904,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, char * qemuBuildHostNetStr(virDomainNetDefPtr net, - virQEMUDriverPtr driver, + virQEMUDriverPtr driver ATTRIBUTE_UNUSED, char **tapfd, size_t tapfdSize, char **vhostfd, @@ -3913,7 +3913,6 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, bool is_tap = false; VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER; virDomainNetType netType = virDomainNetGetActualType(net); - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); size_t i; char *ret = NULL; @@ -4045,7 +4044,6 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, ret = virBufferContentAndReset(&buf); cleanup: - virObjectUnref(cfg); return ret; } -- 2.19.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_command.h | 1 - src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fbc9a73035..7c48a21c23 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3904,7 +3904,6 @@ qemuBuildNicDevStr(virDomainDefPtr def, char * qemuBuildHostNetStr(virDomainNetDefPtr net, - virQEMUDriverPtr driver ATTRIBUTE_UNUSED, char **tapfd, size_t tapfdSize, char **vhostfd, @@ -8589,7 +8588,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, if (chardev) virCommandAddArgList(cmd, "-chardev", chardev, NULL); - if (!(host = qemuBuildHostNetStr(net, driver, + if (!(host = qemuBuildHostNetStr(net, tapfdName, tapfdSize, vhostfdName, vhostfdSize))) goto cleanup; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 6f97e7bc0c..1569eac284 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -83,7 +83,6 @@ qemuBuildChrDeviceStr(char **deviceStr, virQEMUCapsPtr qemuCaps); char *qemuBuildHostNetStr(virDomainNetDefPtr net, - virQEMUDriverPtr driver, char **tapfd, size_t tapfdSize, char **vhostfd, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 63acb9c451..b0e2febad7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1296,7 +1296,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, goto cleanup; } - if (!(netstr = qemuBuildHostNetStr(net, driver, + if (!(netstr = qemuBuildHostNetStr(net, tapfdName, tapfdSize, vhostfdName, vhostfdSize))) goto cleanup; -- 2.19.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7c48a21c23..06ac184f0d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3913,13 +3913,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER; virDomainNetType netType = virDomainNetGetActualType(net); size_t i; - char *ret = NULL; if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("scripts are not supported on interfaces of type %s"), virDomainNetTypeToString(netType)); - goto cleanup; + return NULL; } switch (netType) { @@ -3983,7 +3982,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, const char *prefix = ""; if (!(addr = virSocketAddrFormat(&ip->address))) - goto cleanup; + return NULL; if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) prefix = "net="; @@ -4039,11 +4038,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, virBufferTrim(&buf, ",", -1); if (virBufferCheckError(&buf) < 0) - goto cleanup; + return NULL; - ret = virBufferContentAndReset(&buf); - cleanup: - return ret; + return virBufferContentAndReset(&buf); } -- 2.19.2

Copy the declaration into the smallest blocks it's used in and mark it as VIR_AUTOFREE. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 06ac184f0d..bd6beb0e52 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4471,9 +4471,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, str); if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 || sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) { - char *codecstr = NULL; for (j = 0; j < sound->ncodecs; j++) { + VIR_AUTOFREE(char *) codecstr = NULL; virCommandAddArg(cmd, "-device"); if (!(codecstr = qemuBuildSoundCodecStr(sound, sound->codecs[j], @@ -4482,9 +4482,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, } virCommandAddArg(cmd, codecstr); - VIR_FREE(codecstr); } if (j == 0) { + VIR_AUTOFREE(char *) codecstr = NULL; virDomainSoundCodecDef codec = { VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX, 0 @@ -4497,7 +4497,6 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, } virCommandAddArg(cmd, codecstr); - VIR_FREE(codecstr); } } } -- 2.19.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 43 ++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bd6beb0e52..262f4d0594 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3429,7 +3429,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, useHugepage = false; } else if (useHugepage && pagesize == 0) { if (qemuBuildMemoryGetDefaultPagesize(cfg, &pagesize) < 0) - goto cleanup; + return -1; } if (!(props = virJSONValueNewObject())) @@ -3441,70 +3441,70 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, if (useHugepage && (virJSONValueObjectAdd(props, "b:hugetlb", useHugepage, NULL) < 0 || virJSONValueObjectAdd(props, "U:hugetlbsize", pagesize << 10, NULL) < 0)) { - goto cleanup; + return -1; } if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0) - goto cleanup; + return -1; } else if (useHugepage || mem->nvdimmPath || memAccess || def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) { if (mem->nvdimmPath) { if (VIR_STRDUP(memPath, mem->nvdimmPath) < 0) - goto cleanup; + return -1; if (!priv->memPrealloc) prealloc = true; } else if (useHugepage) { if (qemuGetDomainHupageMemPath(def, cfg, pagesize, &memPath) < 0) - goto cleanup; + return -1; if (!priv->memPrealloc) prealloc = true; } else { /* We can have both pagesize and mem source. If that's the case, * prefer hugepages as those are more specific. */ if (qemuGetMemoryBackingPath(def, cfg, mem->info.alias, &memPath) < 0) - goto cleanup; + return -1; } if (virJSONValueObjectAdd(props, "B:prealloc", prealloc, "s:mem-path", memPath, NULL) < 0) - goto cleanup; + return -1; if (!mem->nvdimmPath && discard == VIR_TRISTATE_BOOL_YES) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this QEMU doesn't support memory discard")); - goto cleanup; + return -1; } if (virJSONValueObjectAdd(props, "B:discard-data", true, NULL) < 0) - goto cleanup; + return -1; } if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0) - goto cleanup; + return -1; } else { backendType = "memory-backend-ram"; } if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0) - goto cleanup; + return -1; if (mem->alignsize) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvdimm align property is not available " "with this QEMU binary")); - goto cleanup; + return -1; } if (virJSONValueObjectAdd(props, "U:align", mem->alignsize * 1024, NULL) < 0) - goto cleanup; + return -1; } if (mem->nvdimmPmem) { @@ -3512,10 +3512,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvdimm pmem property is not available " "with this QEMU binary")); - goto cleanup; + return -1; } if (virJSONValueObjectAdd(props, "s:pmem", "on", NULL) < 0) - goto cleanup; + return -1; } if (mem->sourceNodes) { @@ -3523,17 +3523,17 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, } else { if (virDomainNumatuneMaybeGetNodeset(def->numa, priv->autoNodeset, &nodemask, mem->targetNode) < 0) - goto cleanup; + return -1; } if (nodemask) { if (!virNumaNodesetIsAvailable(nodemask)) - goto cleanup; + return -1; if (virJSONValueObjectAdd(props, "m:host-nodes", nodemask, "S:policy", qemuNumaPolicyTypeToString(mode), NULL) < 0) - goto cleanup; + return -1; } /* If none of the following is requested... */ @@ -3553,19 +3553,19 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this qemu doesn't support the " "memory-backend-file object")); - goto cleanup; + return -1; } else if (STREQ(backendType, "memory-backend-ram") && !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this qemu doesn't support the " "memory-backend-ram object")); - goto cleanup; + return -1; } else if (STREQ(backendType, "memory-backend-memory") && !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this qemu doesn't support the " "memory-backend-memfd object")); - goto cleanup; + return -1; } ret = 0; @@ -3575,7 +3575,6 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, &props))) ret = -1; - cleanup: return ret; } -- 2.19.2

Do not overwrite the 'ret' value more than once. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 262f4d0594..528e11d5a0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3331,7 +3331,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, VIR_AUTOFREE(char *) memPath = NULL; bool prealloc = false; virBitmapPtr nodemask = NULL; - int ret = -1; + int rc; VIR_AUTOPTR(virJSONValue) props = NULL; bool nodeSpecified = virDomainNumatuneNodeSpecified(def->numa, mem->targetNode); unsigned long long pagesize = mem->pagesize; @@ -3545,7 +3545,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, !force) { /* report back that using the new backend is not necessary * to achieve the desired configuration */ - ret = 1; + rc = 1; } else { /* otherwise check the required capability */ if (STREQ(backendType, "memory-backend-file") && @@ -3568,14 +3568,14 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, return -1; } - ret = 0; + rc = 0; } if (!(*backendProps = qemuMonitorCreateObjectPropsWrap(backendType, alias, &props))) - ret = -1; + return -1; - return ret; + return rc; } -- 2.19.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 528e11d5a0..18b3914ca9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3588,14 +3588,13 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, { VIR_AUTOPTR(virJSONValue) props = NULL; VIR_AUTOFREE(char *) alias = NULL; - int ret = -1; int rc; virDomainMemoryDef mem = { 0 }; unsigned long long memsize = virDomainNumaGetNodeMemorySize(def->numa, cell); if (virAsprintf(&alias, "ram-node%zu", cell) < 0) - goto cleanup; + return -1; mem.size = memsize; mem.targetNode = cell; @@ -3603,16 +3602,12 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, if ((rc = qemuBuildMemoryBackendProps(&props, alias, cfg, priv, def, &mem, false)) < 0) - goto cleanup; + return -1; if (virQEMUBuildObjectCommandlineFromJSON(buf, props) < 0) - goto cleanup; - - ret = rc; - - cleanup: + return -1; - return ret; + return rc; } -- 2.19.2

On 8/26/19 4:44 PM, Ján Tomko wrote:
Ján Tomko (7): qemuBuildHostNetStr: remove unused cfg qemuBuildHostNetStr: remove unused 'driver' argument qemuBuildHostNetStr: remove unnecessary cleanup label qemuBuildSoundCommandLine: reduce scope of codecstr qemuBuildMemoryBackendProps: remove useless cleanup label qemuBuildMemoryBackendProps: use 'rc' instead of ret. qemuBuildMemoryCellBackendStr: remove useless ret variable
src/qemu/qemu_command.c | 87 ++++++++++++++++++----------------------- src/qemu/qemu_command.h | 1 - src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 38 insertions(+), 52 deletions(-)
Patch #3 has some rebase conflicts after the slirp work, but they are straight forward Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
participants (2)
-
Cole Robinson
-
Ján Tomko