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