On 12/01/2011 07:39 AM, Eric Blake wrote:
On 11/29/2011 10:57 PM, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
Detected by Coverity. Leak introduced in commit 8866eed.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/uml/uml_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 073f362..fe6d5ba 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1051,6 +1051,7 @@ static int umlStartVMDaemon(virConnectPtr conn, VIR_FREE(vm->def->consoles[i]->info.alias); if (virAsprintf(&vm->def->consoles[i]->info.alias, "console%zu", i)< 0) { virReportOOMError(); + VIR_FORCE_CLOSE(logfd); goto cleanup; Good catch, but while reading the function, I found another bug - if we failed to add the domain to autodestroy, we still returned success but without setting the domain to transient. I'm pushing this more comprehensive fix: Indeed, the return value hasn't been stored in 'ret' if we failed to add domain to autodestroy. Eric, thanks. diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c index fe6d5ba..faea313 100644 --- i/src/uml/uml_driver.c +++ w/src/uml/uml_driver.c @@ -1033,56 +1033,51 @@ static int umlStartVMDaemon(virConnectPtr conn, /* Do this upfront, so any part of the startup process can add * runtime state to vm->def that won't be persisted. This let's us * report implicit runtime defaults in the XML, like vnc listen/socket */ VIR_DEBUG("Setting current domain def as transient"); if (virDomainObjSetDefTransient(driver->caps, vm, true)< 0) { VIR_FORCE_CLOSE(logfd); return -1; }
- if (!(cmd = umlBuildCommandLine(conn, driver, vm))) { - VIR_FORCE_CLOSE(logfd); - virDomainConfVMNWFilterTeardown(vm); - umlCleanupTapDevices(vm); - return -1; - } + if (!(cmd = umlBuildCommandLine(conn, driver, vm))) + goto cleanup;
for (i = 0 ; i< vm->def->nconsoles ; i++) { VIR_FREE(vm->def->consoles[i]->info.alias); if (virAsprintf(&vm->def->consoles[i]->info.alias, "console%zu", i)< 0) { virReportOOMError(); - VIR_FORCE_CLOSE(logfd); goto cleanup; } }
virCommandWriteArgLog(cmd, logfd);
priv->monitor = -1;
virCommandClearCaps(cmd); virCommandSetOutputFD(cmd,&logfd); virCommandSetErrorFD(cmd,&logfd); virCommandDaemonize(cmd);
ret = virCommandRun(cmd, NULL); - VIR_FORCE_CLOSE(logfd); if (ret< 0) goto cleanup;
if (autoDestroy&& - umlProcessAutoDestroyAdd(driver, vm, conn)< 0) + (ret = umlProcessAutoDestroyAdd(driver, vm, conn))< 0) goto cleanup;
ret = virDomainObjSetDefTransient(driver->caps, vm, false); cleanup: + VIR_FORCE_CLOSE(logfd); virCommandFree(cmd);
if (ret< 0) { virDomainConfVMNWFilterTeardown(vm); umlCleanupTapDevices(vm); if (vm->newDef) { virDomainDefFree(vm->def); vm->def = vm->newDef; vm->def->id = -1; vm->newDef = NULL;