[libvirt] [PATCH 0/6] use VIR_AUTO* all around qemu_driver.c
I am messing a lot with qemu_driver.c and other QEMU files due to a multifunction PCI feature I'm working on, ending up sometimes moving code here and there, sometimes copy and pasting parts of it, and spreading all these 'vices', namely using char* and VIR_FREE() and virQEMUDriverConfigPtr with virObjectUnref(), knowing that there is a better way of doing it. Instead of changing just the code I was working on, I took a leap and ended up changing the whole qemu_driver.c file. So here it is. The changes were split to make it reviewing a bit saner. The commiter can squash these in fewer patches if desirable. This was done on top of master commit bc1e4389f5. Daniel Henrique Barboza (6): qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 1/3 qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 2/3 qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3 qemu_driver: use VIR_AUTOFREE() with strings 1/3 qemu_driver: use VIR_AUTOFREE() with strings 2/3 qemu_driver: use VIR_AUTOFREE() with strings 3/3 src/qemu/qemu_driver.c | 519 ++++++++++++++--------------------------- 1 file changed, 170 insertions(+), 349 deletions(-) -- 2.21.0
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label. This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. Since there are a lot of references to change, let's do it in 3 steps. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 94 +++++++++++++----------------------------- 1 file changed, 29 insertions(+), 65 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e041a8bac..990c64cf95 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -264,7 +264,7 @@ qemuAutostartDomain(virDomainObjPtr vm, { virQEMUDriverPtr driver = opaque; int flags = 0; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); int ret = -1; if (cfg->autoStartBypassCache) @@ -296,7 +296,6 @@ qemuAutostartDomain(virDomainObjPtr vm, ret = 0; cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -370,7 +369,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) char **names; virSecurityManagerPtr mgr = NULL; virSecurityManagerPtr stack = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; if (cfg->securityDefaultConfined) @@ -430,7 +429,6 @@ qemuSecurityInit(virQEMUDriverPtr driver) } driver->securityManager = stack; - virObjectUnref(cfg); return 0; error: @@ -438,7 +436,6 @@ qemuSecurityInit(virQEMUDriverPtr driver) _("Failed to initialize security drivers")); virObjectUnref(stack); virObjectUnref(mgr); - virObjectUnref(cfg); return -1; } @@ -1131,7 +1128,7 @@ static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) static int qemuStateReload(void) { - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; if (!qemu_driver) @@ -1147,7 +1144,6 @@ qemuStateReload(void) caps, qemu_driver->xmlopt, qemuNotifyLoadDomain, qemu_driver); cleanup: - virObjectUnref(cfg); virObjectUnref(caps); return 0; } @@ -1169,7 +1165,7 @@ qemuStateStop(void) int state; virDomainPtr *domains = NULL; unsigned int *flags = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(qemu_driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(qemu_driver); if (!(conn = virConnectOpen(cfg->uri))) goto cleanup; @@ -1208,7 +1204,6 @@ qemuStateStop(void) } VIR_FREE(flags); virObjectUnref(conn); - virObjectUnref(cfg); return ret; } @@ -1266,20 +1261,16 @@ qemuStateCleanup(void) static int qemuConnectURIProbe(char **uri) { - virQEMUDriverConfigPtr cfg = NULL; - int ret = -1; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; if (qemu_driver == NULL) return 0; cfg = virQEMUDriverGetConfig(qemu_driver); if (VIR_STRDUP(*uri, cfg->uri) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virObjectUnref(cfg); - return ret; + return 0; } static virDrvOpenStatus qemuConnectOpen(virConnectPtr conn, @@ -1956,7 +1947,7 @@ static int qemuDomainSuspend(virDomainPtr dom) qemuDomainObjPrivatePtr priv; virDomainPausedReason reason; int state; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; if (!(vm = qemuDomObjFromDomain(dom))) return -1; @@ -1999,7 +1990,6 @@ static int qemuDomainSuspend(virDomainPtr dom) cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -2011,7 +2001,7 @@ static int qemuDomainResume(virDomainPtr dom) int ret = -1; int state; int reason; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; if (!(vm = qemuDomObjFromDomain(dom))) return -1; @@ -2057,7 +2047,6 @@ static int qemuDomainResume(virDomainPtr dom) cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -2438,7 +2427,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1, r; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -2543,7 +2532,6 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -2566,7 +2554,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1, r; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -2629,7 +2617,6 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -3141,27 +3128,21 @@ qemuOpenFile(virQEMUDriverPtr driver, int oflags, bool *needUnlink) { - int ret = -1; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); uid_t user = cfg->user; gid_t group = cfg->group; bool dynamicOwnership = cfg->dynamicOwnership; virSecurityLabelDefPtr seclabel; - virObjectUnref(cfg); - /* TODO: Take imagelabel into account? */ if (vm && (seclabel = virDomainDefGetSecurityLabelDef(vm->def, "dac")) != NULL && seclabel->label != NULL && (virParseOwnershipIds(seclabel->label, &user, &group) < 0)) - goto cleanup; - - ret = qemuOpenFileAs(user, group, dynamicOwnership, - path, oflags, needUnlink); + return -1; - cleanup: - return ret; + return qemuOpenFileAs(user, group, dynamicOwnership, + path, oflags, needUnlink); } static int @@ -3326,7 +3307,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, unsigned int flags, qemuDomainAsyncJob asyncJob) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); bool needUnlink = false; int ret = -1; int fd = -1; @@ -3389,7 +3370,6 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (qemuFileWrapperFDClose(vm, wrapperFd) < 0) ret = -1; virFileWrapperFdFree(wrapperFd); - virObjectUnref(cfg); if (ret < 0 && needUnlink) unlink(path); @@ -3617,7 +3597,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, char *compressedpath = NULL; int ret = -1; virDomainObjPtr vm = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | @@ -3644,7 +3624,6 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, cleanup: virDomainObjEndAPI(&vm); VIR_FREE(compressedpath); - virObjectUnref(cfg); return ret; } @@ -3658,14 +3637,11 @@ static char * qemuDomainManagedSavePath(virQEMUDriverPtr driver, virDomainObjPtr vm) { char *ret; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); - if (virAsprintf(&ret, "%s/%s.save", cfg->saveDir, vm->def->name) < 0) { - virObjectUnref(cfg); + if (virAsprintf(&ret, "%s/%s.save", cfg->saveDir, vm->def->name) < 0) return NULL; - } - virObjectUnref(cfg); return ret; } @@ -3673,7 +3649,7 @@ static int qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; int compressed; char *compressedpath = NULL; virDomainObjPtr vm; @@ -3719,7 +3695,6 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) virDomainObjEndAPI(&vm); VIR_FREE(name); VIR_FREE(compressedpath); - virObjectUnref(cfg); return ret; } @@ -3917,7 +3892,7 @@ doCoreDump(virQEMUDriverPtr driver, int directFlag = 0; unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; const char *memory_dump_format = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); char *compressedpath = NULL; /* We reuse "save" flag for "dump" here. Then, we can support the same @@ -3999,7 +3974,6 @@ doCoreDump(virQEMUDriverPtr driver, if (ret != 0) unlink(path); VIR_FREE(compressedpath); - virObjectUnref(cfg); return ret; } @@ -4131,7 +4105,7 @@ qemuDomainScreenshot(virDomainPtr dom, const char *videoAlias = NULL; char *ret = NULL; bool unlink_tmp = false; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(0, NULL); @@ -4223,7 +4197,6 @@ qemuDomainScreenshot(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -4236,7 +4209,7 @@ getAutoDumpPath(virQEMUDriverPtr driver, char timestr[100]; struct tm time_info; time_t curtime = time(NULL); - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; if (!domname) return NULL; @@ -4251,7 +4224,6 @@ getAutoDumpPath(virQEMUDriverPtr driver, domname, timestr)); - virObjectUnref(cfg); VIR_FREE(domname); return dumpfile; } @@ -4262,7 +4234,7 @@ processWatchdogEvent(virQEMUDriverPtr driver, int action) { int ret; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); char *dumpfile = getAutoDumpPath(driver, vm); unsigned int flags = VIR_DUMP_MEMORY_ONLY; @@ -4303,7 +4275,6 @@ processWatchdogEvent(virQEMUDriverPtr driver, qemuDomainObjEndAsyncJob(driver, vm); cleanup: - virObjectUnref(cfg); VIR_FREE(dumpfile); } @@ -4313,7 +4284,7 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, unsigned int flags) { int ret = -1; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); char *dumpfile = getAutoDumpPath(driver, vm); if (!dumpfile) @@ -4326,7 +4297,6 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, "%s", _("Dump failed")); cleanup: VIR_FREE(dumpfile); - virObjectUnref(cfg); return ret; } @@ -4355,13 +4325,13 @@ processGuestPanicEvent(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virObjectEventPtr event = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); bool removeInactive = false; unsigned long flags = VIR_DUMP_MEMORY_ONLY; if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_DUMP, VIR_DOMAIN_JOB_OPERATION_DUMP, flags) < 0) - goto cleanup; + return; if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Ignoring GUEST_PANICKED event from inactive domain %s", @@ -4428,9 +4398,6 @@ processGuestPanicEvent(virQEMUDriverPtr driver, qemuDomainObjEndAsyncJob(driver, vm); if (removeInactive) qemuDomainRemoveInactiveJob(driver, vm); - - cleanup: - virObjectUnref(cfg); } @@ -4439,14 +4406,14 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *devAlias) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virDomainDeviceDef dev; VIR_DEBUG("Removing device %s from domain %p %s", devAlias, vm, vm->def->name); if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + return; if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain is not running"); @@ -4469,9 +4436,6 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, endjob: qemuDomainObjEndJob(driver, vm); - - cleanup: - virObjectUnref(cfg); } -- 2.21.0
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label. This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. Since there are a lot of references to change, let's do it in 3 steps. This is step 2. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 78 ++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 990c64cf95..c7ed662ff1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4640,7 +4640,7 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *devAlias) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev; virDomainNetDefPtr def; @@ -4739,7 +4739,6 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, cleanup: virNetDevRxFilterFree(hostFilter); virNetDevRxFilterFree(guestFilter); - virObjectUnref(cfg); } @@ -4749,7 +4748,7 @@ processSerialChangedEvent(virQEMUDriverPtr driver, const char *devAlias, bool connected) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virDomainChrDeviceState newstate; virObjectEventPtr event = NULL; virDomainDeviceDef dev; @@ -4782,7 +4781,7 @@ processSerialChangedEvent(virQEMUDriverPtr driver, } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + return; if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain is not running"); @@ -4823,9 +4822,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver, endjob: qemuDomainObjEndJob(driver, vm); - - cleanup: - virObjectUnref(cfg); } @@ -5098,21 +5094,20 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, virDomainDefPtr persistentDef, unsigned int nvcpus) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); unsigned int topologycpus; - int ret = -1; if (def) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("maximum vcpu count of a live domain can't be modified")); - goto cleanup; + return -1; } if (virDomainNumaGetCPUCountTotal(persistentDef->numa) > nvcpus) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Number of CPUs in <numa> exceeds the desired " "maximum vcpu count")); - goto cleanup; + return -1; } if (virDomainDefGetVcpusTopology(persistentDef, &topologycpus) == 0 && @@ -5121,23 +5116,19 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, * setting may be corrected via this API */ virReportError(VIR_ERR_INVALID_ARG, "%s", _("CPU topology doesn't match the desired vcpu count")); - goto cleanup; + return -1; } /* ordering information may become invalid, thus clear it */ virDomainDefVcpuOrderClear(persistentDef); if (virDomainDefSetVcpusMax(persistentDef, nvcpus, driver->xmlopt) < 0) - goto cleanup; + return -1; if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - virObjectUnref(cfg); - return ret; + return 0; } @@ -5302,7 +5293,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, int ret = -1; virBitmapPtr pcpumap = NULL; virDomainVcpuDefPtr vcpuinfo = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -5359,7 +5350,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); virBitmapFree(pcpumap); - virObjectUnref(cfg); return ret; } @@ -5422,7 +5412,7 @@ qemuDomainPinEmulator(virDomainPtr dom, int ret = -1; qemuDomainObjPrivatePtr priv; virBitmapPtr pcpumap = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virObjectEventPtr event = NULL; char *str = NULL; virTypedParameterPtr eventParams = NULL; @@ -5516,7 +5506,6 @@ qemuDomainPinEmulator(virDomainPtr dom, VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -5887,7 +5876,7 @@ qemuDomainPinIOThread(virDomainPtr dom, { int ret = -1; virQEMUDriverPtr driver = dom->conn->privateData; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virDomainObjPtr vm; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -6011,7 +6000,6 @@ qemuDomainPinIOThread(virDomainPtr dom, VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -6360,7 +6348,7 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, virDomainIOThreadAction action, unsigned int flags) { - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -6460,7 +6448,6 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, qemuDomainObjEndJob(driver, vm); cleanup: - virObjectUnref(cfg); return ret; } @@ -6971,7 +6958,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, int intermediatefd = -1; virCommandPtr cmd = NULL; char *errbuf = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeaderPtr header = &data->header; qemuDomainSaveCookiePtr cookie = NULL; @@ -7093,7 +7080,6 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, VIR_FREE(errbuf); if (qemuSecurityRestoreSavedStateLabel(driver, vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); - virObjectUnref(cfg); return ret; } @@ -7518,13 +7504,11 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, virCommandPtr cmd = NULL; char *ret = NULL; size_t i; - virQEMUDriverConfigPtr cfg; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps = NULL; virCheckFlags(0, NULL); - cfg = virQEMUDriverGetConfig(driver); - if (virConnectDomainXMLToNativeEnsureACL(conn) < 0) goto cleanup; @@ -7584,7 +7568,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, virCommandFree(cmd); virObjectUnref(vm); virObjectUnref(caps); - virObjectUnref(cfg); return ret; } @@ -7771,7 +7754,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virObjectEventPtr event = NULL; - virQEMUDriverConfigPtr cfg; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -7781,8 +7764,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (flags & VIR_DOMAIN_DEFINE_VALIDATE) parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; - cfg = virQEMUDriverGetConfig(driver); - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -7839,7 +7820,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); virObjectUnref(caps); - virObjectUnref(cfg); return dom; } @@ -7860,7 +7840,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, int ret = -1; int nsnapshots; int ncheckpoints; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA | @@ -7980,7 +7960,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, VIR_FREE(name); virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); - virObjectUnref(cfg); return ret; } @@ -8797,7 +8776,7 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, { qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr vmdef = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virDomainDeviceDefPtr devConf = NULL; virDomainDeviceDefPtr devLive = NULL; int ret = -1; @@ -8882,7 +8861,6 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, virDomainDefFree(vmdef); virDomainDeviceDefFree(devConf); virDomainDeviceDefFree(devLive); - virObjectUnref(cfg); virObjectUnref(caps); return ret; @@ -8943,7 +8921,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; unsigned int parse_flags = 0; @@ -9044,7 +9022,6 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virDomainDeviceDefFree(dev); virDomainObjEndAPI(&vm); virObjectUnref(caps); - virObjectUnref(cfg); virNWFilterUnlockFilterUpdates(); return ret; } @@ -9057,7 +9034,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virCapsPtr caps = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; virDomainDefPtr vmdef = NULL; @@ -9136,7 +9113,6 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, cleanup: virObjectUnref(caps); - virObjectUnref(cfg); if (dev != dev_copy) virDomainDeviceDefFree(dev_copy); virDomainDeviceDefFree(dev); @@ -9153,7 +9129,7 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virCapsPtr caps = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainDefPtr vmdef = NULL; @@ -9214,7 +9190,6 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, ret = 0; cleanup: virDomainDefFree(vmdef); - virObjectUnref(cfg); virObjectUnref(caps); return ret; } @@ -9323,7 +9298,7 @@ static int qemuDomainSetAutostart(virDomainPtr dom, virDomainObjPtr vm; char *configFile = NULL, *autostartLink = NULL; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; if (!(vm = qemuDomObjFromDomain(dom))) return -1; @@ -9388,7 +9363,6 @@ static int qemuDomainSetAutostart(virDomainPtr dom, VIR_FREE(configFile); VIR_FREE(autostartLink); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -9635,7 +9609,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -9835,7 +9809,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -9947,7 +9920,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, bool set_swap_hard_limit = false; bool set_hard_limit = false; bool set_soft_limit = false; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; int rc; int ret = -1; qemuDomainObjPrivatePtr priv; @@ -10072,7 +10045,6 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } -- 2.21.0
On Wed, Sep 18, 2019 at 11:56:54AM -0300, Daniel Henrique Barboza wrote:
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label.
This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable.
Since there are a lot of references to change, let's do it in 3 steps. This is step 2.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- ...
@@ -6360,7 +6348,7 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, virDomainIOThreadAction action, unsigned int flags) { - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -6460,7 +6448,6 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, qemuDomainObjEndJob(driver, vm);
cleanup:
This 'cleanup' label can be dropped. Erik
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label. This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. This is the last part of this change. All but one* instance of virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. * qemuStateInitialize: we can't auto-unref the pointer since we're initializing the qemu_driver object with it. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 57 ++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c7ed662ff1..bc2e2ccfc2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10209,7 +10209,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virDomainDefPtr persistentDef; virDomainObjPtr vm = NULL; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virBitmapPtr nodeset = NULL; virDomainNumatuneMemMode config_mode; @@ -10321,7 +10321,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, cleanup: virBitmapFree(nodeset); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -10426,7 +10425,7 @@ qemuDomainSetPerfEvents(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; size_t i; virDomainObjPtr vm = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -10518,7 +10517,6 @@ qemuDomainSetPerfEvents(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -10705,7 +10703,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, long long value_l; int ret = -1; int rc; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virObjectEventPtr event = NULL; @@ -10999,7 +10997,6 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (eventNparams) virTypedParamsFree(eventParams, eventNparams); virObjectUnref(caps); - virObjectUnref(cfg); return ret; } #undef SCHED_RANGE_CHECK @@ -11664,7 +11661,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, int ret = -1; virDomainNetDefPtr net = NULL, persistentNet = NULL; virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; bool inboundSpecified = false, outboundSpecified = false; int actualType; bool qosSupported = true; @@ -11859,7 +11856,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virNetDevBandwidthFree(bandwidth); virNetDevBandwidthFree(newBandwidth); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -12121,7 +12117,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, char *tmp = NULL; int fd = -1, ret = -1; qemuDomainObjPrivatePtr priv; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1); @@ -12192,7 +12188,6 @@ qemuDomainMemoryPeek(virDomainPtr dom, unlink(tmp); VIR_FREE(tmp); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -12409,7 +12404,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; virDomainDiskDefPtr disk; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuBlockStatsPtr entry = NULL; virCheckFlags(0, -1); @@ -12498,7 +12493,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, cleanup: VIR_FREE(entry); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -12970,7 +12964,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, unsigned int flags) { virQEMUDriverPtr driver = dconn->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; @@ -13044,7 +13038,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, VIR_FREE(migrate_disks); VIR_FREE(origname); virDomainDefFree(def); - virObjectUnref(cfg); return ret; } @@ -14614,7 +14607,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, virCommandPtr cmd = NULL; const char *qemuImgPath; virBitmapPtr created = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); int ret = -1; virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); @@ -14702,7 +14695,6 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, } } virBitmapFree(created); - virObjectUnref(cfg); return ret; } @@ -15804,7 +15796,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; bool align_match = true; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virDomainSnapshotState state; @@ -16094,7 +16086,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virDomainObjEndAPI(&vm); VIR_FREE(xml); virObjectUnref(caps); - virObjectUnref(cfg); return snapshot; } @@ -16505,7 +16496,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, int rc; virDomainDefPtr config = NULL; virDomainDefPtr inactiveConfig = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; bool was_stopped = false; qemuDomainSaveCookiePtr cookie; @@ -16935,7 +16926,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virObjectEventStateQueue(driver->domainEventState, event2); virDomainObjEndAPI(&vm); virObjectUnref(caps); - virObjectUnref(cfg); virNWFilterUnlockFilterUpdates(); virCPUDefFree(origCPU); virDomainDefFree(config); @@ -16996,7 +16986,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virQEMUMomentReparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); int external = 0; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY | @@ -17088,7 +17078,6 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -17216,7 +17205,7 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE; unsigned int parse_flags = 0; virDomainMomentObjPtr other = NULL; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virJSONValuePtr actions = NULL; @@ -17358,7 +17347,6 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, virDomainObjEndAPI(&vm); VIR_FREE(xml); virObjectUnref(caps); - virObjectUnref(cfg); return checkpoint; } @@ -17532,7 +17520,7 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint, virQEMUMomentRemove rem; virQEMUMomentReparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY); - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN | VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY | @@ -17622,7 +17610,6 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -18017,7 +18004,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainDiskDefPtr disk = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT); bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC); qemuBlockJobDataPtr job = NULL; @@ -18122,7 +18109,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom, cleanup: virObjectUnref(job); - virObjectUnref(cfg); virDomainObjEndAPI(&vm); return ret; } @@ -19393,7 +19379,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, bool supportMaxOptions = true; bool supportGroupNameOption = true; bool supportMaxLengthOptions = true; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virObjectEventPtr event = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; @@ -19714,7 +19700,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); - virObjectUnref(cfg); return ret; } @@ -19964,7 +19949,7 @@ qemuDomainSetMetadata(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; int ret = -1; @@ -20000,7 +19985,6 @@ qemuDomainSetMetadata(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); virObjectUnref(caps); - virObjectUnref(cfg); return ret; } @@ -21757,7 +21741,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virHashTablePtr nodestats = NULL; virJSONValuePtr nodedata = NULL; qemuDomainObjPrivatePtr priv = dom->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); bool fetchnodedata = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_NAMED_BLOCK_NODES) && !blockdev; @@ -21814,7 +21798,6 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virHashFree(stats); virHashFree(nodestats); virJSONValueFree(nodedata); - virObjectUnref(cfg); return ret; } @@ -22513,7 +22496,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm, void *opaque) { virQEMUDriverPtr driver = opaque; - virQEMUDriverConfigPtr cfg = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; @@ -22602,7 +22585,6 @@ qemuDomainRenameCallback(virDomainObjPtr vm, VIR_FREE(new_dom_name); virObjectEventStateQueue(driver->domainEventState, event_old); virObjectEventStateQueue(driver->domainEventState, event_new); - virObjectUnref(cfg); return ret; rollback: @@ -23065,7 +23047,7 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv; virDomainObjPtr vm = NULL; virDomainDefPtr def = NULL; @@ -23122,7 +23104,6 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } -- 2.21.0
On Wed, Sep 18, 2019 at 11:56:55AM -0300, Daniel Henrique Barboza wrote:
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label.
This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable.
This is the last part of this change. All but one* instance of virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable.
* qemuStateInitialize: we can't auto-unref the pointer since we're initializing the qemu_driver object with it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> ---
I know you focused on virQEMUDriverConfigPtr primarily, but since you're following up with VIR_AUTOFREE and touching qemu_driver only, I'd like to do a better job and use VIR_AUTOUNREF at many more places across the file for: virCapsPtr caps virConnectPtr conn qemuDomainSaveCookiePtr cookie virQEMUCapsPtr qemuCaps qemuBlockJobDataPtr job virDomainCapsPtr domCaps virNetworkPtr network (there may be a few more...) Erik
On 9/26/19 6:18 AM, Erik Skultety wrote:
On Wed, Sep 18, 2019 at 11:56:55AM -0300, Daniel Henrique Barboza wrote:
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label.
This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable.
This is the last part of this change. All but one* instance of virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable.
* qemuStateInitialize: we can't auto-unref the pointer since we're initializing the qemu_driver object with it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- I know you focused on virQEMUDriverConfigPtr primarily, but since you're following up with VIR_AUTOFREE and touching qemu_driver only, I'd like to do a better job and use VIR_AUTOUNREF at many more places across the file for:
virCapsPtr caps virConnectPtr conn qemuDomainSaveCookiePtr cookie virQEMUCapsPtr qemuCaps qemuBlockJobDataPtr job virDomainCapsPtr domCaps virNetworkPtr network
No problem. Should I squash the first 3 patches of this series into a single one that will touch only virQEMUDriverConfigPtr and then make one patch for each pointer type that's changed? Thanks, DHB
(there may be a few more...)
Erik
On Thu, Sep 26, 2019 at 01:15:16PM -0300, Daniel Henrique Barboza wrote:
On 9/26/19 6:18 AM, Erik Skultety wrote:
On Wed, Sep 18, 2019 at 11:56:55AM -0300, Daniel Henrique Barboza wrote:
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label.
This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable.
This is the last part of this change. All but one* instance of virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable.
* qemuStateInitialize: we can't auto-unref the pointer since we're initializing the qemu_driver object with it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- I know you focused on virQEMUDriverConfigPtr primarily, but since you're following up with VIR_AUTOFREE and touching qemu_driver only, I'd like to do a better job and use VIR_AUTOUNREF at many more places across the file for:
virCapsPtr caps virConnectPtr conn qemuDomainSaveCookiePtr cookie virQEMUCapsPtr qemuCaps qemuBlockJobDataPtr job virDomainCapsPtr domCaps virNetworkPtr network
No problem. Should I squash the first 3 patches of this series into a single
I would have squashed those 3 into a single one before pushing anyway.
one that will touch only virQEMUDriverConfigPtr and then make one patch for each pointer type that's changed?
Ultimately I'd have all the VIR_AUTOUNREF changes touching the same file in a single patch, but for review purposes it may be even better to have them split in order to separate the changes I've already reviewed and the ones you'll introduce. Erik
VIR_AUTOFREE is a beautiful macro. Let's use it across the board inside qemu_driver.c to make the code a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a whole 'cleanup' label. This is a huge change due to the amount of char * declared in this file, thus let's split it in 3. This is the first part. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 85 +++++++++++++----------------------------- 1 file changed, 26 insertions(+), 59 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc2e2ccfc2..29b8bbe9d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -445,11 +445,9 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, void *data) { char *baseDir = (char *)data; - char *snapDir = NULL; + VIR_AUTOFREE(char *) snapDir = NULL; DIR *dir = NULL; struct dirent *entry; - char *xmlStr; - char *fullpath; virDomainSnapshotDefPtr def = NULL; virDomainMomentObjPtr snap = NULL; virDomainMomentObjPtr current = NULL; @@ -484,6 +482,9 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, goto cleanup; while ((direrr = virDirRead(dir, &entry, NULL)) > 0) { + VIR_AUTOFREE(char *) xmlStr = NULL; + VIR_AUTOFREE(char *) fullpath = NULL; + /* NB: ignoring errors, so one malformed config doesn't kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name); @@ -499,7 +500,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, virReportSystemError(errno, _("Failed to read snapshot file %s"), fullpath); - VIR_FREE(fullpath); continue; } @@ -512,8 +512,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse snapshot XML from file '%s'"), fullpath); - VIR_FREE(fullpath); - VIR_FREE(xmlStr); continue; } @@ -527,9 +525,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, vm->def->name); current = snap; } - - VIR_FREE(fullpath); - VIR_FREE(xmlStr); } if (direrr < 0) virReportError(VIR_ERR_INTERNAL_ERROR, @@ -556,7 +551,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, ret = 0; cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(snapDir); virObjectUnref(caps); virObjectUnlock(vm); return ret; @@ -568,11 +562,9 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, void *data) { char *baseDir = (char *)data; - char *chkDir = NULL; + VIR_AUTOFREE(char *) chkDir = NULL; DIR *dir = NULL; struct dirent *entry; - char *xmlStr; - char *fullpath; virDomainCheckpointDefPtr def = NULL; virDomainMomentObjPtr chk = NULL; virDomainMomentObjPtr current = NULL; @@ -603,6 +595,9 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, goto cleanup; while ((direrr = virDirRead(dir, &entry, NULL)) > 0) { + VIR_AUTOFREE(char *) xmlStr = NULL; + VIR_AUTOFREE(char *) fullpath = NULL; + /* NB: ignoring errors, so one malformed config doesn't kill the whole process */ VIR_INFO("Loading checkpoint file '%s'", entry->d_name); @@ -618,7 +613,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, virReportSystemError(errno, _("Failed to read checkpoint file %s"), fullpath); - VIR_FREE(fullpath); continue; } @@ -631,8 +625,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse checkpoint XML from file '%s'"), fullpath); - VIR_FREE(fullpath); - VIR_FREE(xmlStr); virObjectUnref(def); continue; } @@ -640,9 +632,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, chk = virDomainCheckpointAssignDef(vm->checkpoints, def); if (chk == NULL) virObjectUnref(def); - - VIR_FREE(fullpath); - VIR_FREE(xmlStr); } if (direrr < 0) virReportError(VIR_ERR_INTERNAL_ERROR, @@ -667,7 +656,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, ret = 0; cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(chkDir); virObjectUnref(caps); virObjectUnlock(vm); return ret; @@ -726,12 +714,11 @@ qemuStateInitialize(bool privileged, virStateInhibitCallback callback, void *opaque) { - char *driverConf = NULL; + VIR_AUTOFREE(char *) driverConf = NULL; virQEMUDriverConfigPtr cfg; uid_t run_uid = -1; gid_t run_gid = -1; - char *hugepagePath = NULL; - char *memoryBackingPath = NULL; + VIR_AUTOFREE(char *) memoryBackingPath = NULL; size_t i; if (VIR_ALLOC(qemu_driver) < 0) @@ -771,7 +758,6 @@ qemuStateInitialize(bool privileged, if (virQEMUDriverConfigLoadFile(cfg, driverConf, privileged) < 0) goto error; - VIR_FREE(driverConf); if (virQEMUDriverConfigValidate(cfg) < 0) goto error; @@ -895,7 +881,7 @@ qemuStateInitialize(bool privileged, goto error; if (privileged) { - char *channeldir; + VIR_AUTOFREE(char *) channeldir = NULL; if (chown(cfg->libDir, cfg->user, cfg->group) < 0) { virReportSystemError(errno, @@ -948,10 +934,8 @@ qemuStateInitialize(bool privileged, _("unable to set ownership of '%s' to %d:%d"), channeldir, (int)cfg->user, (int)cfg->group); - VIR_FREE(channeldir); goto error; } - VIR_FREE(channeldir); if (chown(cfg->channelTargetDir, cfg->user, cfg->group) < 0) { virReportSystemError(errno, _("unable to set ownership of '%s' to %d:%d"), @@ -1002,6 +986,8 @@ qemuStateInitialize(bool privileged, * it, since we can't assume the root mount point has permissions that * will let our spawned QEMU instances use it. */ for (i = 0; i < cfg->nhugetlbfs; i++) { + VIR_AUTOFREE(char *) hugepagePath = NULL; + hugepagePath = qemuGetBaseHugepagePath(&cfg->hugetlbfs[i]); if (!hugepagePath) @@ -1017,7 +1003,6 @@ qemuStateInitialize(bool privileged, virFileUpdatePerm(cfg->hugetlbfs[i].mnt_dir, 0, S_IXGRP | S_IXOTH) < 0) goto error; - VIR_FREE(hugepagePath); } if (qemuGetMemoryBackingBasePath(cfg, &memoryBackingPath) < 0) @@ -1034,7 +1019,6 @@ qemuStateInitialize(bool privileged, virFileUpdatePerm(memoryBackingPath, 0, S_IXGRP | S_IXOTH) < 0) goto error; - VIR_FREE(memoryBackingPath); if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew())) goto error; @@ -1099,9 +1083,6 @@ qemuStateInitialize(bool privileged, return VIR_DRV_STATE_INIT_COMPLETE; error: - VIR_FREE(driverConf); - VIR_FREE(hugepagePath); - VIR_FREE(memoryBackingPath); qemuStateCleanup(); return VIR_DRV_STATE_INIT_ERROR; } @@ -1447,8 +1428,8 @@ static int qemuGetSchedInfo(unsigned long long *cpuWait, pid_t pid, pid_t tid) { - char *proc = NULL; - char *data = NULL; + VIR_AUTOFREE(char *) proc = NULL; + VIR_AUTOFREE(char *) data = NULL; char **lines = NULL; size_t i; int ret = -1; @@ -1512,8 +1493,6 @@ qemuGetSchedInfo(unsigned long long *cpuWait, ret = 0; cleanup: - VIR_FREE(data); - VIR_FREE(proc); virStringListFree(lines); return ret; } @@ -1523,7 +1502,7 @@ static int qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, pid_t pid, int tid) { - char *proc; + VIR_AUTOFREE(char *) proc = NULL; FILE *pidinfo; unsigned long long usertime = 0, systime = 0; long rss = 0; @@ -1540,7 +1519,6 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, return -1; pidinfo = fopen(proc, "r"); - VIR_FREE(proc); /* See 'man proc' for information about what all these fields are. We're * only interested in a very few of them */ @@ -3000,7 +2978,7 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, size_t cookie_len = 0; int ret = -1; size_t zerosLen = 0; - char *zeros = NULL; + VIR_AUTOFREE(char *) zeros = NULL; xml_len = strlen(data->xml) + 1; if (data->cookie) @@ -3057,7 +3035,6 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, ret = 0; cleanup: - VIR_FREE(zeros); return ret; } @@ -3388,7 +3365,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, int compressed, const char *compressedpath, const char *xmlin, unsigned int flags) { - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; bool was_running = false; int ret = -1; virObjectEventPtr event = NULL; @@ -3469,7 +3446,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, if (!(data = virQEMUSaveDataNew(xml, cookie, was_running, compressed, driver->xmlopt))) goto endjob; - xml = NULL; ret = qemuDomainSaveMemory(driver, vm, path, data, compressedpath, flags, QEMU_ASYNC_JOB_SAVE); @@ -3505,7 +3481,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, cleanup: virObjectUnref(cookie); - VIR_FREE(xml); virQEMUSaveDataFree(data); virObjectEventStateQueue(driver->domainEventState, event); virObjectUnref(caps); @@ -3594,7 +3569,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, { virQEMUDriverPtr driver = dom->conn->privateData; int compressed; - char *compressedpath = NULL; + VIR_AUTOFREE(char *) compressedpath = NULL; int ret = -1; virDomainObjPtr vm = NULL; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; @@ -3623,7 +3598,6 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(compressedpath); return ret; } @@ -3651,9 +3625,9 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) virQEMUDriverPtr driver = dom->conn->privateData; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; int compressed; - char *compressedpath = NULL; + VIR_AUTOFREE(char *) compressedpath = NULL; virDomainObjPtr vm; - char *name = NULL; + VIR_AUTOFREE(char *) name = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | @@ -3693,8 +3667,6 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(name); - VIR_FREE(compressedpath); return ret; } @@ -3704,7 +3676,7 @@ qemuDomainManagedSaveLoad(virDomainObjPtr vm, void *opaque) { virQEMUDriverPtr driver = opaque; - char *name; + VIR_AUTOFREE(char *) name = NULL; int ret = -1; virObjectLock(vm); @@ -3717,7 +3689,6 @@ qemuDomainManagedSaveLoad(virDomainObjPtr vm, ret = 0; cleanup: virObjectUnlock(vm); - VIR_FREE(name); return ret; } @@ -3749,7 +3720,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; - char *name = NULL; + VIR_AUTOFREE(char *) name = NULL; virCheckFlags(0, -1); @@ -3773,7 +3744,6 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) ret = 0; cleanup: - VIR_FREE(name); virDomainObjEndAPI(&vm); return ret; } @@ -3893,7 +3863,7 @@ doCoreDump(virQEMUDriverPtr driver, unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; const char *memory_dump_format = NULL; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); - char *compressedpath = NULL; + VIR_AUTOFREE(char *) compressedpath = NULL; /* We reuse "save" flag for "dump" here. Then, we can support the same * format in "save" and "dump". This path doesn't need the compression @@ -3973,7 +3943,6 @@ doCoreDump(virQEMUDriverPtr driver, virFileWrapperFdFree(wrapperFd); if (ret != 0) unlink(path); - VIR_FREE(compressedpath); return ret; } @@ -4099,7 +4068,7 @@ qemuDomainScreenshot(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; - char *tmp = NULL; + VIR_AUTOFREE(char *) tmp = NULL; int tmp_fd = -1; size_t i; const char *videoAlias = NULL; @@ -4191,7 +4160,6 @@ qemuDomainScreenshot(virDomainPtr dom, VIR_FORCE_CLOSE(tmp_fd); if (unlink_tmp) unlink(tmp); - VIR_FREE(tmp); qemuDomainObjEndJob(driver, vm); @@ -4205,7 +4173,7 @@ getAutoDumpPath(virQEMUDriverPtr driver, virDomainObjPtr vm) { char *dumpfile = NULL; - char *domname = virDomainDefGetShortName(vm->def); + VIR_AUTOFREE(char *domname) = virDomainDefGetShortName(vm->def); char timestr[100]; struct tm time_info; time_t curtime = time(NULL); @@ -4224,7 +4192,6 @@ getAutoDumpPath(virQEMUDriverPtr driver, domname, timestr)); - VIR_FREE(domname); return dumpfile; } -- 2.21.0
On Wed, Sep 18, 2019 at 11:56:56AM -0300, Daniel Henrique Barboza wrote:
VIR_AUTOFREE is a beautiful macro. Let's use it across the board inside qemu_driver.c to make the code a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a whole 'cleanup' label.
This is a huge change due to the amount of char * declared in this file, thus let's split it in 3. This is the first part.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- ...
@@ -3000,7 +2978,7 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, size_t cookie_len = 0; int ret = -1; size_t zerosLen = 0; - char *zeros = NULL; + VIR_AUTOFREE(char *) zeros = NULL;
xml_len = strlen(data->xml) + 1; if (data->cookie) @@ -3057,7 +3035,6 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, ret = 0;
cleanup:
^This cleanup can be dropped. Erik
VIR_AUTOFREE is a beautiful macro. Let's use it across the board inside qemu_driver.c to make the code a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a whole 'cleanup' label. This is a huge change due to the amount of char * declared in this file, thus let's split it in 3. This is the second part. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 86 +++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 29b8bbe9d9..489698108d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4202,11 +4202,11 @@ processWatchdogEvent(virQEMUDriverPtr driver, { int ret; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); - char *dumpfile = getAutoDumpPath(driver, vm); + VIR_AUTOFREE(char *) dumpfile = getAutoDumpPath(driver, vm); unsigned int flags = VIR_DUMP_MEMORY_ONLY; if (!dumpfile) - goto cleanup; + return; switch (action) { case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: @@ -4214,7 +4214,7 @@ processWatchdogEvent(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_DUMP, VIR_DOMAIN_JOB_OPERATION_DUMP, flags) < 0) { - goto cleanup; + return; } if (virDomainObjCheckActive(vm) < 0) @@ -4235,14 +4235,11 @@ processWatchdogEvent(virQEMUDriverPtr driver, "%s", _("Resuming after dump failed")); break; default: - goto cleanup; + return; } endjob: qemuDomainObjEndAsyncJob(driver, vm); - - cleanup: - VIR_FREE(dumpfile); } static int @@ -4252,18 +4249,16 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, { int ret = -1; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); - char *dumpfile = getAutoDumpPath(driver, vm); + VIR_AUTOFREE(char *) dumpfile = getAutoDumpPath(driver, vm); if (!dumpfile) - goto cleanup; + return -1; flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0; if ((ret = doCoreDump(driver, vm, dumpfile, flags, VIR_DOMAIN_CORE_DUMP_FORMAT_RAW)) < 0) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Dump failed")); - cleanup: - VIR_FREE(dumpfile); return ret; } @@ -4273,14 +4268,11 @@ qemuProcessGuestPanicEventInfo(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMonitorEventPanicInfoPtr info) { - char *msg = qemuMonitorGuestPanicEventInfoFormatMsg(info); - char *timestamp = virTimeStringNow(); + VIR_AUTOFREE(char *) msg = qemuMonitorGuestPanicEventInfoFormatMsg(info); + VIR_AUTOFREE(char *) timestamp = virTimeStringNow(); if (msg && timestamp) qemuDomainLogAppendMessage(driver, vm, "%s: panic %s\n", timestamp, msg); - - VIR_FREE(timestamp); - VIR_FREE(msg); } @@ -5176,7 +5168,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, virDomainVcpuDefPtr vcpuinfo; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_vcpu = NULL; - char *str = NULL; + VIR_AUTOFREE(char *) str = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; virTypedParameterPtr eventParams = NULL; @@ -5240,7 +5232,6 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, cleanup: virBitmapFree(tmpmap); virCgroupFree(&cgroup_vcpu); - VIR_FREE(str); virObjectEventStateQueue(driver->domainEventState, event); return ret; } @@ -5381,7 +5372,7 @@ qemuDomainPinEmulator(virDomainPtr dom, virBitmapPtr pcpumap = NULL; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virObjectEventPtr event = NULL; - char *str = NULL; + VIR_AUTOFREE(char *) str = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; int eventMaxparams = 0; @@ -5470,7 +5461,6 @@ qemuDomainPinEmulator(virDomainPtr dom, if (cgroup_emulator) virCgroupFree(&cgroup_emulator); virObjectEventStateQueue(driver->domainEventState, event); - VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); return ret; @@ -5852,7 +5842,7 @@ qemuDomainPinIOThread(virDomainPtr dom, virCgroupPtr cgroup_iothread = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; - char *str = NULL; + VIR_AUTOFREE(char *) str = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; int eventMaxparams = 0; @@ -5964,7 +5954,6 @@ qemuDomainPinIOThread(virDomainPtr dom, if (cgroup_iothread) virCgroupFree(&cgroup_iothread); virObjectEventStateQueue(driver->domainEventState, event); - VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); return ret; @@ -5976,7 +5965,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, unsigned int iothread_id) { qemuDomainObjPrivatePtr priv = vm->privateData; - char *alias = NULL; + VIR_AUTOFREE(char *) alias = NULL; size_t idx; int ret = -1; unsigned int orig_niothreads = vm->def->niothreadids; @@ -6052,7 +6041,6 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, } virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, "update", ret == 0); - VIR_FREE(alias); virJSONValueFree(props); return ret; @@ -6097,7 +6085,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; size_t idx; - char *alias = NULL; + VIR_AUTOFREE(char *) alias = NULL; int rc = -1; int ret = -1; unsigned int orig_niothreads = vm->def->niothreadids; @@ -6146,7 +6134,6 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, } virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, "update", rc == 0); - VIR_FREE(alias); return ret; exit_monitor: @@ -6924,7 +6911,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, virObjectEventPtr event; int intermediatefd = -1; virCommandPtr cmd = NULL; - char *errbuf = NULL; + VIR_AUTOFREE(char *) errbuf = NULL; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeaderPtr header = &data->header; qemuDomainSaveCookiePtr cookie = NULL; @@ -7044,7 +7031,6 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, cleanup: virObjectUnref(cookie); virCommandFree(cmd); - VIR_FREE(errbuf); if (qemuSecurityRestoreSavedStateLabel(driver, vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); return ret; @@ -7060,7 +7046,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; - char *xmlout = NULL; + VIR_AUTOFREE(char *) xmlout = NULL; const char *newxml = dxml; int fd = -1; int ret = -1; @@ -7145,7 +7131,6 @@ qemuDomainRestoreFlags(virConnectPtr conn, ret = -1; virFileWrapperFdFree(wrapperFd); virQEMUSaveDataFree(data); - VIR_FREE(xmlout); if (vm && ret < 0) qemuDomainRemoveInactiveJob(driver, vm); virDomainObjEndAPI(&vm); @@ -7268,7 +7253,7 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; char *ret = NULL; virDomainDefPtr def = NULL; int fd = -1; @@ -7305,7 +7290,6 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) virDomainDefFree(def); VIR_FORCE_CLOSE(fd); virDomainObjEndAPI(&vm); - VIR_FREE(path); return ret; } @@ -7316,7 +7300,7 @@ qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, virQEMUDriverPtr driver = dom->conn->privateData; virConnectPtr conn = dom->conn; virDomainObjPtr vm; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; int ret = -1; if (!(vm = qemuDomObjFromDomain(dom))) @@ -7338,7 +7322,6 @@ qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(path); return ret; } @@ -7357,7 +7340,7 @@ qemuDomainObjRestore(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; int fd = -1; int ret = -1; - char *xmlout = NULL; + VIR_AUTOFREE(char *) xmlout = NULL; virQEMUSaveDataPtr data = NULL; virFileWrapperFdPtr wrapperFd = NULL; @@ -7414,7 +7397,6 @@ qemuDomainObjRestore(virConnectPtr conn, cleanup: virQEMUSaveDataFree(data); - VIR_FREE(xmlout); virDomainDefFree(def); VIR_FORCE_CLOSE(fd); if (virFileWrapperFdClose(wrapperFd) < 0) @@ -7580,7 +7562,7 @@ qemuDomainObjStart(virConnectPtr conn, qemuDomainAsyncJob asyncJob) { int ret = -1; - char *managed_save; + VIR_AUTOFREE(char *) managed_save = NULL; bool start_paused = (flags & VIR_DOMAIN_START_PAUSED) != 0; bool autodestroy = (flags & VIR_DOMAIN_START_AUTODESTROY) != 0; bool bypass_cache = (flags & VIR_DOMAIN_START_BYPASS_CACHE) != 0; @@ -7598,7 +7580,7 @@ qemuDomainObjStart(virConnectPtr conn, managed_save = qemuDomainManagedSavePath(driver, vm); if (!managed_save) - goto cleanup; + return ret; if (virFileExists(managed_save)) { if (force_boot) { @@ -7606,7 +7588,7 @@ qemuDomainObjStart(virConnectPtr conn, virReportSystemError(errno, _("cannot remove managed save file %s"), managed_save); - goto cleanup; + return ret; } vm->hasManagedSave = false; } else { @@ -7622,11 +7604,11 @@ qemuDomainObjStart(virConnectPtr conn, else vm->hasManagedSave = false; - goto cleanup; + return ret; } else if (ret < 0) { VIR_WARN("Unable to restore from managed state %s. " "Maybe the file is corrupted?", managed_save); - goto cleanup; + return ret; } else { VIR_WARN("Ignoring incomplete managed state %s", managed_save); priv->job.current->operation = op; @@ -7653,8 +7635,6 @@ qemuDomainObjStart(virConnectPtr conn, } } - cleanup: - VIR_FREE(managed_save); return ret; } @@ -7803,7 +7783,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; virObjectEventPtr event = NULL; - char *name = NULL; + VIR_AUTOFREE(char *) name = NULL; int ret = -1; int nsnapshots; int ncheckpoints; @@ -7924,7 +7904,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(name); virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); return ret; @@ -9263,7 +9242,8 @@ static int qemuDomainSetAutostart(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - char *configFile = NULL, *autostartLink = NULL; + VIR_AUTOFREE(char *) configFile = NULL; + VIR_AUTOFREE(char *) autostartLink = NULL; int ret = -1; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; @@ -9327,8 +9307,6 @@ static int qemuDomainSetAutostart(virDomainPtr dom, ret = 0; cleanup: - VIR_FREE(configFile); - VIR_FREE(autostartLink); virDomainObjEndAPI(&vm); return ret; } @@ -10108,7 +10086,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, { virCgroupPtr cgroup_temp = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - char *nodeset_str = NULL; + VIR_AUTOFREE(char *) nodeset_str = NULL; virDomainNumatuneMemMode mode; size_t i = 0; int ret = -1; @@ -10158,7 +10136,6 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(nodeset_str); virCgroupFree(&cgroup_temp); return ret; @@ -10301,7 +10278,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; qemuDomainObjPrivatePtr priv; - char *nodeset = NULL; + VIR_AUTOFREE(char *) nodeset = NULL; int ret = -1; virDomainDefPtr def = NULL; bool live = false; @@ -10365,7 +10342,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, ret = 0; cleanup: - VIR_FREE(nodeset); virDomainObjEndAPI(&vm); return ret; } @@ -11233,7 +11209,7 @@ qemuDomainBlockResize(virDomainPtr dom, virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; int ret = -1; - char *device = NULL; + VIR_AUTOFREE(char *) device = NULL; const char *nodename = NULL; virDomainDiskDefPtr disk = NULL; @@ -11305,7 +11281,6 @@ qemuDomainBlockResize(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(device); virDomainObjEndAPI(&vm); return ret; } @@ -12014,7 +11989,7 @@ qemuDomainBlockPeek(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainDiskDefPtr disk = NULL; virDomainObjPtr vm; - char *tmpbuf = NULL; + VIR_AUTOFREE(char *) tmpbuf = NULL; ssize_t nread; int ret = -1; @@ -12069,7 +12044,6 @@ qemuDomainBlockPeek(virDomainPtr dom, if (disk) virStorageFileDeinit(disk->src); virDomainObjEndAPI(&vm); - VIR_FREE(tmpbuf); return ret; } -- 2.21.0
VIR_AUTOFREE is a beautiful macro. Let's use it across the board inside qemu_driver.c to make the code a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a whole 'cleanup' label. This is the last part of this change. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 119 ++++++++++++++--------------------------- 1 file changed, 41 insertions(+), 78 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 489698108d..bd05aa9e11 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12055,7 +12055,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - char *tmp = NULL; + VIR_AUTOFREE(char *) tmp = NULL; int fd = -1, ret = -1; qemuDomainObjPrivatePtr priv; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; @@ -12127,7 +12127,6 @@ qemuDomainMemoryPeek(virDomainPtr dom, VIR_FORCE_CLOSE(fd); if (tmp) unlink(tmp); - VIR_FREE(tmp); virDomainObjEndAPI(&vm); return ret; } @@ -12293,7 +12292,7 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver, int ret = -1; int fd = -1; struct stat sb; - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; ssize_t len; if ((rc = qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, @@ -12329,7 +12328,6 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver, ret = 1; cleanup: - VIR_FREE(buf); qemuDomainStorageCloseStat(src, &fd); return ret; } @@ -12550,7 +12548,7 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + VIR_AUTOFREE(char *) origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12585,7 +12583,6 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12607,7 +12604,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + VIR_AUTOFREE(char *) origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12652,7 +12649,6 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12853,7 +12849,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + VIR_AUTOFREE(char *) origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12888,7 +12884,6 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12914,7 +12909,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, int nbdPort = 0; int nmigrate_disks; const char **migrate_disks = NULL; - char *origname = NULL; + VIR_AUTOFREE(char *) origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12977,7 +12972,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); VIR_FREE(migrate_disks); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12997,7 +12991,7 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + VIR_AUTOFREE(char *) origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -13026,7 +13020,6 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -13046,7 +13039,7 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr dconn, virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; - char *origname = NULL; + VIR_AUTOFREE(char *) origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -13085,7 +13078,6 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -13407,7 +13399,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; bool vfio = qemuHostdevHostSupportsPassthroughVFIO(); virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; @@ -13457,7 +13449,6 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, cleanup: virPCIDeviceFree(pci); virNodeDeviceDefFree(def); - VIR_FREE(xml); return ret; } @@ -13475,7 +13466,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; xml = virNodeDeviceGetXMLDesc(dev, 0); @@ -13501,7 +13492,6 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) virPCIDeviceFree(pci); cleanup: virNodeDeviceDefFree(def); - VIR_FREE(xml); return ret; } @@ -13513,7 +13503,7 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; xml = virNodeDeviceGetXMLDesc(dev, 0); @@ -13539,7 +13529,6 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) virPCIDeviceFree(pci); cleanup: virNodeDeviceDefFree(def); - VIR_FREE(xml); return ret; } @@ -15229,7 +15218,6 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - char *backingStoreStr; virDomainDiskDefPtr persistdisk; VIR_AUTOUNREF(virStorageSourcePtr) terminator = NULL; bool supportsCreate; @@ -15275,13 +15263,13 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, * block commit still works */ if (reuse) { if (supportsBacking) { + VIR_AUTOFREE(char *) backingStoreStr = NULL; + if (virStorageFileGetBackingStoreStr(dd->src, &backingStoreStr) < 0) return -1; if (backingStoreStr != NULL) { if (virStorageIsRelative(backingStoreStr)) VIR_STEAL_PTR(dd->relPath, backingStoreStr); - else - VIR_FREE(backingStoreStr); } } } else { @@ -15728,7 +15716,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, { virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virDomainMomentObjPtr current = NULL; @@ -16025,7 +16013,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(xml); virObjectUnref(caps); return snapshot; } @@ -17029,9 +17016,8 @@ qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps, virDomainObjPtr vm, virDomainCheckpointDefPtr def) { - int ret = -1; size_t i; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; /* Easiest way to clone inactive portion of vm->def is via @@ -17043,10 +17029,10 @@ qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps, priv->qemuCaps, VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) - goto cleanup; + return -1; if (virDomainCheckpointAlignDisks(def) < 0) - goto cleanup; + return -1; for (i = 0; i < def->ndisks; i++) { virDomainCheckpointDiskDefPtr disk = &def->disks[i]; @@ -17061,15 +17047,11 @@ qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps, disk->name, virStorageFileFormatTypeToString( vm->def->disks[i]->src->format)); - goto cleanup; + return -1; } } - ret = 0; - - cleanup: - VIR_FREE(xml); - return ret; + return 0; } static int @@ -17139,7 +17121,7 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, { virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; virDomainMomentObjPtr chk = NULL; virDomainCheckpointPtr checkpoint = NULL; bool update_current = true; @@ -17286,7 +17268,6 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, cleanup: virJSONValueFree(actions); virDomainObjEndAPI(&vm); - VIR_FREE(xml); virObjectUnref(caps); return checkpoint; } @@ -18174,7 +18155,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, virDomainDiskDefPtr disk; int ret = -1; virDomainObjPtr vm; - char *device = NULL; + VIR_AUTOFREE(char *) device = NULL; unsigned long long speed = bandwidth; virCheckFlags(VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, -1); @@ -18219,7 +18200,6 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(device); virDomainObjEndAPI(&vm); return ret; @@ -19310,7 +19290,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo info; - char *drivealias = NULL; + VIR_AUTOFREE(char *) drivealias = NULL; const char *qdevid = NULL; int ret = -1; size_t i; @@ -19637,7 +19617,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, cleanup: VIR_FREE(info.group_name); - VIR_FREE(drivealias); virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); @@ -19658,7 +19637,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo reply = {0}; - char *drivealias = NULL; + VIR_AUTOFREE(char *) drivealias = NULL; const char *qdevid = NULL; int ret = -1; int maxparams; @@ -19795,7 +19774,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, cleanup: VIR_FREE(reply.group_name); - VIR_FREE(drivealias); virDomainObjEndAPI(&vm); return ret; } @@ -21593,12 +21571,10 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, bool blockdev) { - char *alias = NULL; virStorageSourcePtr n; const char *frontendalias; const char *backendalias; const char *backendstoragealias; - int ret = -1; /* * This helps to keep logs clean from error messages on getting stats @@ -21615,6 +21591,8 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, } for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { + VIR_AUTOFREE(char *) alias = NULL; + if (blockdev) { frontendalias = QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName; backendalias = n->nodeformat; @@ -21623,7 +21601,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, /* alias may be NULL if the VM is not running */ if (disk->info.alias && !(alias = qemuDomainStorageAlias(disk->info.alias, n->id))) - goto cleanup; + return -1; qemuDomainGetStatsOneBlockRefreshNamed(n, alias, stats, nodestats); @@ -21634,37 +21612,32 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, if (qemuDomainGetStatsBlockExportHeader(disk, n, *recordnr, records, nrecords) < 0) - goto cleanup; + return -1; /* The following stats make sense only for the frontend device */ if (n == disk->src) { if (qemuDomainGetStatsBlockExportFrontend(frontendalias, stats, *recordnr, records, nrecords) < 0) - goto cleanup; + return -1; } if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, backendalias, n, *recordnr, stats) < 0) - goto cleanup; + return -1; if (qemuDomainGetStatsBlockExportBackendStorage(backendstoragealias, stats, *recordnr, records, nrecords) < 0) - goto cleanup; + return -1; - VIR_FREE(alias); (*recordnr)++; if (!visitBacking) break; } - ret = 0; - - cleanup: - VIR_FREE(alias); - return ret; + return 0; } @@ -22441,12 +22414,12 @@ qemuDomainRenameCallback(virDomainObjPtr vm, virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; - char *new_dom_name = NULL; - char *old_dom_name = NULL; - char *new_dom_cfg_file = NULL; - char *old_dom_cfg_file = NULL; - char *new_dom_autostart_link = NULL; - char *old_dom_autostart_link = NULL; + VIR_AUTOFREE(char *) new_dom_name = NULL; + VIR_AUTOFREE(char *) old_dom_name = NULL; + VIR_AUTOFREE(char *) new_dom_cfg_file = NULL; + VIR_AUTOFREE(char *) old_dom_cfg_file = NULL; + VIR_AUTOFREE(char *) new_dom_autostart_link = NULL; + VIR_AUTOFREE(char *) old_dom_autostart_link = NULL; virCheckFlags(0, ret); @@ -22506,7 +22479,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm, if (virFileIsLink(old_dom_autostart_link) && unlink(old_dom_autostart_link) < 0) { virReportSystemError(errno, - _("Failed to delete symlink '%s'"), + _("Failed to delete symlink '%s'"), old_dom_autostart_link); goto rollback; } @@ -22518,12 +22491,6 @@ qemuDomainRenameCallback(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(old_dom_autostart_link); - VIR_FREE(new_dom_autostart_link); - VIR_FREE(old_dom_cfg_file); - VIR_FREE(new_dom_cfg_file); - VIR_FREE(old_dom_name); - VIR_FREE(new_dom_name); virObjectEventStateQueue(driver->domainEventState, event_old); virObjectEventStateQueue(driver->domainEventState, event_new); return ret; @@ -22628,7 +22595,7 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, virBitmapPtr vcpus = NULL; virBitmapPtr online = NULL; virBitmapPtr offlinable = NULL; - char *tmp = NULL; + VIR_AUTOFREE(char *) tmp = NULL; size_t i; int ret = -1; @@ -22657,7 +22624,6 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, goto cleanup; \ if (virTypedParamsAddString(&par, &npar, &maxpar, #name, tmp) < 0) \ goto cleanup; \ - VIR_FREE(tmp) ADD_BITMAP(vcpus); ADD_BITMAP(online); @@ -22671,7 +22637,6 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, ret = 0; cleanup: - VIR_FREE(tmp); virBitmapFree(vcpus); virBitmapFree(online); virBitmapFree(offlinable); @@ -22899,7 +22864,7 @@ qemuDomainSetBlockThreshold(virDomainPtr dom, qemuDomainObjPrivatePtr priv; virDomainObjPtr vm = NULL; virStorageSourcePtr src; - char *nodename = NULL; + VIR_AUTOFREE(char *) nodename = NULL; int rc; int ret = -1; @@ -22953,7 +22918,6 @@ qemuDomainSetBlockThreshold(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(nodename); virDomainObjEndAPI(&vm); return ret; } @@ -23133,7 +23097,7 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, unsigned int flags) { int ret = -1; - char *tmp = NULL; + VIR_AUTOFREE(char *) tmp = NULL; int maxpar = 0; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -23158,7 +23122,6 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, ret = 0; endjob: - VIR_FREE(tmp); qemuDomainObjEndJob(driver, vm); return ret; } -- 2.21.0
On Wed, Sep 18, 2019 at 11:56:58AM -0300, Daniel Henrique Barboza wrote:
VIR_AUTOFREE is a beautiful macro. Let's use it across the board inside qemu_driver.c to make the code a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a whole 'cleanup' label.
This is the last part of this change.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- ...
@@ -22506,7 +22479,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm, if (virFileIsLink(old_dom_autostart_link) && unlink(old_dom_autostart_link) < 0) { virReportSystemError(errno, - _("Failed to delete symlink '%s'"), + _("Failed to delete symlink '%s'"),
^Undesirable hunk Same as with VIR_AUTOUNREF, this focuses on strings only, I'd extend this also for other primitive scalar pointers: cpuwait in qemuDomainGetStatsVcpu flags in qemuStateStop There are other occasions at which we use VIR_FREE and could be replaced, but those are not scalar type pointers and I the policy I believe we settled with is not to use VIR_AUTOFREE with compound types, instead a dedicated VIR_AUTOPTR should be created. Erik
participants (2)
-
Daniel Henrique Barboza -
Erik Skultety