From c4255703d236874c2cb0f7f5a636315310b440ca Mon Sep 17 00:00:00 2001

Message-Id: <c4255703d236874c2cb0f7f5a636315310b440ca.1380112457.git.jdenemar@redhat.com>

From: Eric Blake <eblake@redhat.com>

Date: Mon, 23 Sep 2013 11:10:05 -0600

Subject: [PATCH] qemu: don't leave shutdown inhibited on attach failure

https://bugzilla.redhat.com/show_bug.cgi?id=1010617

While debugging a failure of 'virsh qemu-attach', I noticed that

we were leaking the count of active domains on failure.  This

means that a libvirtd session that is supposed to quit after

active domains disappear will hang around forever.

* src/qemu/qemu_process.c (qemuProcessAttach): Undo count of

active domains on failure.

Signed-off-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit 93e599750e5d5fff49c41a7f91570498a4b6de71)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/qemu/qemu_process.c | 65 ++++++++++++++++++++++++++-----------------------

 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c

index 1d8df58..843ff76 100644

--- a/src/qemu/qemu_process.c

+++ b/src/qemu/qemu_process.c

@@ -4334,6 +4334,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

     const char *model;

     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

     virCapsPtr caps = NULL;

+    bool active = false;

     VIR_DEBUG("Beginning VM attach process");

@@ -4345,7 +4346,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

     }

     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))

-        goto cleanup;

+        goto error;

     /* 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

@@ -4353,60 +4354,59 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

      */

     VIR_DEBUG("Setting current domain def as transient");

     if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0)

-        goto cleanup;

+        goto error;

     vm->def->id = qemuDriverAllocateID(driver);

     if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)

         driver->inhibitCallback(true, driver->inhibitOpaque);

+    active = true;

     if (virFileMakePath(cfg->logDir) < 0) {

         virReportSystemError(errno,

                              _("cannot create log directory %s"),

                              cfg->logDir);

-        goto cleanup;

+        goto error;

     }

     VIR_FREE(priv->pidfile);

     if (VIR_STRDUP(priv->pidfile, pidfile) < 0)

-        goto cleanup;

+        goto error;

     VIR_DEBUG("Detect security driver config");

     sec_managers = virSecurityManagerGetNested(driver->securityManager);

-    if (sec_managers == NULL) {

-        goto cleanup;

-    }

+    if (sec_managers == NULL)

+        goto error;

     for (i = 0; sec_managers[i]; i++) {

         model = virSecurityManagerGetModel(sec_managers[i]);

         seclabeldef = virDomainDefGetSecurityLabelDef(vm->def, model);

-        if (seclabeldef == NULL) {

-            goto cleanup;

-        }

+        if (seclabeldef == NULL)

+            goto error;

         seclabeldef->type = VIR_DOMAIN_SECLABEL_STATIC;

         if (VIR_ALLOC(seclabel) < 0)

-            goto cleanup;

+            goto error;

         if (virSecurityManagerGetProcessLabel(driver->securityManager,

                                               vm->def, vm->pid, seclabel) < 0)

-            goto cleanup;

+            goto error;

         if (VIR_STRDUP(seclabeldef->model, model) < 0)

-            goto cleanup;

+            goto error;

         if (VIR_STRDUP(seclabeldef->label, seclabel->label) < 0)

-            goto cleanup;

+            goto error;

         VIR_FREE(seclabel);

     }

     VIR_DEBUG("Creating domain log file");

     if ((logfile = qemuDomainCreateLog(driver, vm, false)) < 0)

-        goto cleanup;

+        goto error;

     VIR_DEBUG("Determining emulator version");

     virObjectUnref(priv->qemuCaps);

     if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,

                                                       vm->def->emulator)))

-        goto cleanup;

+        goto error;

     VIR_DEBUG("Preparing monitor state");

     priv->monConfig = monConfig;

@@ -4425,11 +4425,11 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

     if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {

         VIR_DEBUG("Assigning domain PCI addresses");

         if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0)

-            goto cleanup;

+            goto error;

     }

     if ((timestamp = virTimeStringNow()) == NULL) {

-        goto cleanup;

+        goto error;

     } else {

         if (safewrite(logfile, timestamp, strlen(timestamp)) < 0 ||

             safewrite(logfile, ATTACH_POSTFIX, strlen(ATTACH_POSTFIX)) < 0) {

@@ -4446,7 +4446,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

     VIR_DEBUG("Waiting for monitor to show up");

     if (qemuProcessWaitForMonitor(driver, vm, priv->qemuCaps, -1) < 0)

-        goto cleanup;

+        goto error;

     /* Failure to connect to agent shouldn't be fatal */

     if (qemuConnectAgent(driver, vm) < 0) {

@@ -4458,34 +4458,34 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

     VIR_DEBUG("Detecting VCPU PIDs");

     if (qemuProcessDetectVcpuPIDs(driver, vm) < 0)

-        goto cleanup;

+        goto error;

     /* If we have -device, then addresses are assigned explicitly.

      * If not, then we have to detect dynamic ones here */

     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {

         VIR_DEBUG("Determining domain device PCI addresses");

         if (qemuProcessInitPCIAddresses(driver, vm) < 0)

-            goto cleanup;

+            goto error;

     }

     VIR_DEBUG("Getting initial memory amount");

     qemuDomainObjEnterMonitor(driver, vm);

     if (qemuMonitorGetBalloonInfo(priv->mon, &vm->def->mem.cur_balloon) < 0) {

         qemuDomainObjExitMonitor(driver, vm);

-        goto cleanup;

+        goto error;

     }

     if (qemuMonitorGetStatus(priv->mon, &running, &reason) < 0) {

         qemuDomainObjExitMonitor(driver, vm);

-        goto cleanup;

+        goto error;

     }

     if (qemuMonitorGetVirtType(priv->mon, &vm->def->virtType) < 0) {

         qemuDomainObjExitMonitor(driver, vm);

-        goto cleanup;

+        goto error;

     }

     qemuDomainObjExitMonitor(driver, vm);

     if (!virDomainObjIsActive(vm))

-        goto cleanup;

+        goto error;

     if (running) {

         virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,

@@ -4502,7 +4502,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

     VIR_DEBUG("Writing domain status to disk");

     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)

-        goto cleanup;

+        goto error;

     /* Run an hook to allow admins to do some magic */

     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {

@@ -4518,7 +4518,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

          * If the script raised an error abort the launch

          */

         if (hookret < 0)

-            goto cleanup;

+            goto error;

     }

     VIR_FORCE_CLOSE(logfile);

@@ -4529,10 +4529,13 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

     return 0;

-cleanup:

-    /* We jump here if we failed to start the VM for any reason, or

-     * if we failed to initialize the now running VM. kill it off and

-     * pretend we never started it */

+error:

+    /* We jump here if we failed to attach to the VM for any reason.

+     * Leave the domain running, but pretend we never attempted to

+     * attach to it.  */

+    if (active && virAtomicIntDecAndTest(&driver->nactive) &&

+        driver->inhibitCallback)

+        driver->inhibitCallback(false, driver->inhibitOpaque);

     VIR_FORCE_CLOSE(logfile);

     VIR_FREE(seclabel);

     VIR_FREE(sec_managers);

-- 

1.8.3.2