From 8e6492f298896ff4a8554c0741538fbec707f340 Mon Sep 17 00:00:00 2001 From: Luyao Huang Date: Wed, 4 Feb 2015 21:42:56 +0800 Subject: [PATCH] lxc: Fix container cleanup for LXCProcessStart Jumping to the cleanup label prior to starting the container failed to properly clean everything up that is handled by the virLXCProcessCleanup which is called if virLXCProcessStop is called on failure after the container properly starts. Most importantly is prior to this patch none of the stop/release hooks, host device reattachment, and network cleanup (that is reverse of virLXCProcessSetupInterfaces). Signed-off-by: Luyao Huang --- src/lxc/lxc_process.c | 78 ++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 45 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index c7b47624a5..1e90c16975 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1023,6 +1023,7 @@ int virLXCProcessStart(virConnectPtr conn, int status; char *pidfile = NULL; bool clearSeclabel = false; + bool need_stop = false; if (virCgroupNewSelf(&selfcgroup) < 0) return -1; @@ -1274,6 +1275,7 @@ int virLXCProcessStart(virConnectPtr conn, goto cleanup; } + need_stop = true; priv->stopReason = VIR_DOMAIN_EVENT_STOPPED_FAILED; priv->wantReboot = false; vm->def->id = vm->pid; @@ -1282,20 +1284,20 @@ int virLXCProcessStart(virConnectPtr conn, if (VIR_CLOSE(handshakefds[1]) < 0) { virReportSystemError(errno, "%s", _("could not close handshake fd")); - goto error; + goto cleanup; } if (virCommandHandshakeWait(cmd) < 0) - goto error; + goto cleanup; /* Write domain status to disk for the controller to * read when it starts */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto error; + goto cleanup; /* Allow the child to exec the controller */ if (virCommandHandshakeNotify(cmd) < 0) - goto error; + goto cleanup; if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); @@ -1308,7 +1310,7 @@ int virLXCProcessStart(virConnectPtr conn, _("guest failed to start: %s"), out); } - goto error; + goto cleanup; } /* We know the cgroup must exist by this synchronization @@ -1320,13 +1322,13 @@ int virLXCProcessStart(virConnectPtr conn, vm->def->resource->partition : NULL, -1, &priv->cgroup) < 0) - goto error; + goto cleanup; if (!priv->cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No valid cgroup for machine %s"), vm->def->name); - goto error; + goto cleanup; } /* And we can get the first monitor connection now too */ @@ -1339,17 +1341,17 @@ int virLXCProcessStart(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, _("guest failed to start: %s"), ebuf); } - goto error; + goto cleanup; } if (autoDestroy && virCloseCallbacksSet(driver->closeCallbacks, vm, conn, lxcProcessAutoDestroy) < 0) - goto error; + goto cleanup; if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, false) < 0) - goto error; + goto cleanup; /* We don't need the temporary NIC names anymore, clear them */ virLXCProcessCleanInterfaces(vm->def); @@ -1368,48 +1370,39 @@ int virLXCProcessStart(virConnectPtr conn, * If the script raised an error abort the launch */ if (hookret < 0) - goto error; + goto cleanup; } rc = 0; cleanup: - if (rc != 0 && !err) - err = virSaveLastError(); - virCommandFree(cmd); if (VIR_CLOSE(logfd) < 0) { virReportSystemError(errno, "%s", _("could not close logfile")); rc = -1; } - for (i = 0; i < nveths; i++) { - if (rc != 0 && veths[i]) - ignore_value(virNetDevVethDelete(veths[i])); - VIR_FREE(veths[i]); - } if (rc != 0) { - if (vm->newDef) { - virDomainDefFree(vm->newDef); - vm->newDef = NULL; - } - if (priv->monitor) { - virObjectUnref(priv->monitor); - priv->monitor = NULL; - } - virDomainConfVMNWFilterTeardown(vm); - - virSecurityManagerRestoreAllLabel(driver->securityManager, - vm->def, false); - virSecurityManagerReleaseLabel(driver->securityManager, vm->def); - /* Clear out dynamically assigned labels */ - if (vm->def->nseclabels && - (vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC || - clearSeclabel)) { - VIR_FREE(vm->def->seclabels[0]->model); - VIR_FREE(vm->def->seclabels[0]->label); - VIR_FREE(vm->def->seclabels[0]->imagelabel); - VIR_DELETE_ELEMENT(vm->def->seclabels, 0, vm->def->nseclabels); + err = virSaveLastError(); + if (need_stop) { + virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); + } else { + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm->def, false); + virSecurityManagerReleaseLabel(driver->securityManager, vm->def); + /* Clear out dynamically assigned labels */ + if (vm->def->nseclabels && + (vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC || + clearSeclabel)) { + VIR_FREE(vm->def->seclabels[0]->model); + VIR_FREE(vm->def->seclabels[0]->label); + VIR_FREE(vm->def->seclabels[0]->imagelabel); + VIR_DELETE_ELEMENT(vm->def->seclabels, 0, vm->def->nseclabels); + } + virLXCProcessCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); } } + virCommandFree(cmd); + for (i = 0; i < nveths; i++) + VIR_FREE(veths[i]); for (i = 0; i < nttyFDs; i++) VIR_FORCE_CLOSE(ttyFDs[i]); VIR_FREE(ttyFDs); @@ -1426,11 +1419,6 @@ int virLXCProcessStart(virConnectPtr conn, } return rc; - - error: - err = virSaveLastError(); - virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); - goto cleanup; } struct virLXCProcessAutostartData {