From 5547d2b81c20522da2b5bc1a7f4e47d725d9e010 Mon Sep 17 00:00:00 2001 From: Jiri Denemark Date: Fri, 9 Dec 2011 15:33:13 +0100 Subject: [PATCH] qemu: Disable EOF processing during qemuDomainDestroy When destroying a domain qemuDomainDestroy kills its qemu process and starts a new job, which means it unlocks the domain object and locks it again after some time. Although the object is usually unlocked for a pretty short time, chances are another thread processing an EOF event on qemu monitor is able to lock the object first and does all the cleanup by itself. This leads to wrong shutoff reason and lifecycle event detail and virDomainDestroy API incorrectly reporting failure to destroy an inactive domain. Reported by Charlie Smurthwaite. --- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 10 ++++++++++ src/qemu/qemu_process.c | 26 +++++++++++++++++--------- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e8bcab3c1b..35f9440f19 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -110,6 +110,7 @@ struct _qemuDomainObjPrivate { bool monError; unsigned long long monStart; bool gotShutdown; + bool beingDestroyed; char *pidfile; int nvcpupids; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10a289e08f..ceb9f47cbd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1678,6 +1678,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; virDomainEventPtr event = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -1691,6 +1692,8 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto cleanup; } + priv = vm->privateData; + qemuDomainSetFakeReboot(driver, vm, false); /* Although qemuProcessStop does this already, there may @@ -1700,9 +1703,16 @@ qemuDomainDestroyFlags(virDomainPtr dom, */ qemuProcessKill(vm, false); + /* We need to prevent monitor EOF callback from doing our work (and sending + * misleading events) while the vm is unlocked inside BeginJob API + */ + priv->beingDestroyed = true; + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; + priv->beingDestroyed = false; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d4271d0ccc..9123f4c85e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -128,14 +128,18 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuDriverLock(driver); virDomainObjLock(vm); - if (!virDomainObjIsActive(vm)) { - VIR_DEBUG("Domain %p is not active, ignoring EOF", vm); - virDomainObjUnlock(vm); - qemuDriverUnlock(driver); - return; + priv = vm->privateData; + + if (priv->beingDestroyed) { + VIR_DEBUG("Domain is being destroyed, EOF is expected"); + goto unlock; + } + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain %p is not active, ignoring EOF", vm); + goto unlock; } - priv = vm->privateData; if (priv->monJSON && !priv->gotShutdown) { VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; " "assuming the domain crashed", vm->def->name); @@ -150,11 +154,15 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuProcessStop(driver, vm, 0, stopReason); virDomainAuditStop(vm, auditReason); - if (!vm->persistent) + if (!vm->persistent) { qemuDomainRemoveInactive(driver, vm); - else - virDomainObjUnlock(vm); + goto cleanup; + } +unlock: + virDomainObjUnlock(vm); + +cleanup: if (event) qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver);