[PATCHv2] lxc: fix show the wrong xml when guest start failed

[Luyao Huang <lhuang@xxxxxxxxxx>]

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(-)

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 {
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list