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.

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index e8bcab3c1b3ef9fb125e598a2ac92d29222e3946..35f9440f193eedf0eb08b6797624e04b0d3db1f2 100644 (file)
--- 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 10a289e08f433955f542dda2be1ddde07fb0a0a1..ceb9f47cbdc57e6bbddfc7e10dfc1ae0fd20e4a9 100644 (file)
--- 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 d4271d0cccae47e23dad768f690eae18d57f0ae2..9123f4c85e9b610afca8449a89a1f047dc94d4e0 100644 (file)
--- 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);
 
+    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);
-        virDomainObjUnlock(vm);
-        qemuDriverUnlock(driver);
-        return;
+        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);