On 01/12/2015 09:33 AM, Luyao Huang wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1176503 > > When guest start failed, libvirt will keep the current vm->def, > this will make a issue that we cannot get a right xml after guest > start failed. And don't call the stop/release hook to do some > other clean work. > > Call virLXCProcessCleanup to help us clean the source and call > the hooks if start a vm failed. > > Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx> > --- > v2: use virLXCProcessCleanup to free the source and call the hook. > > src/lxc/lxc_process.c | 69 ++++++++++++++++++++++----------------------------- > 1 file changed, 29 insertions(+), 40 deletions(-) > There's been some changes to virLXCProcessStart since this was written - a revisit/rework is in order. John > diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c > index d7eb8bc..7ba0da8 100644 > --- a/src/lxc/lxc_process.c > +++ b/src/lxc/lxc_process.c > @@ -1006,6 +1006,7 @@ int virLXCProcessStart(virConnectPtr conn, > virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); > virCgroupPtr selfcgroup; > int status; > + bool need_stop = false; > > if (virCgroupNewSelf(&selfcgroup) < 0) > return -1; > @@ -1265,18 +1266,20 @@ int virLXCProcessStart(virConnectPtr conn, > goto cleanup; > } > > + need_stop = true; > + > if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid, > vm->def->resource ? > 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; > } > > priv->stopReason = VIR_DOMAIN_EVENT_STOPPED_FAILED; > @@ -1296,17 +1299,17 @@ int virLXCProcessStart(virConnectPtr conn, > _("guest failed to start: %s"), out); > } > > - 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); > @@ -1318,7 +1321,7 @@ int virLXCProcessStart(virConnectPtr conn, > * it with the live status XML instead. This is a (currently > * harmless) inconsistency we should fix one day */ > if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) > - goto error; > + goto cleanup; > > /* finally we can call the 'started' hook script if any */ > if (virHookPresent(VIR_HOOK_DRIVER_LXC)) { > @@ -1334,47 +1337,38 @@ 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) { > - 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) { > + 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); > @@ -1390,11 +1384,6 @@ int virLXCProcessStart(virConnectPtr conn, > } > > return rc; > - > - error: > - err = virSaveLastError(); > - virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); > - goto cleanup; > } > > struct virLXCProcessAutostartData { > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list