qemu: Process monitor EOF in a job

Stopping a domain without a job risks a race condition with another
thread which started a job a which does not expect anyone else to be
messing around with the same domain object.

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

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 29f58643935efb0427b143454b1e774063652575..8359b1afd08cda06b483f28b6a5fac66837eac3d 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -234,6 +234,7 @@ typedef enum {
     QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED,
     QEMU_PROCESS_EVENT_SERIAL_CHANGED,
     QEMU_PROCESS_EVENT_BLOCK_JOB,
+    QEMU_PROCESS_EVENT_MONITOR_EOF,
 
     QEMU_PROCESS_EVENT_LAST
 } qemuProcessEventType;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index de366851534187e3edb3789cdba76f0671017684..8b8cb1a97c8a6b86432734d3bb9937459c55f994 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4537,13 +4537,59 @@ processBlockJobEvent(virQEMUDriverPtr driver,
 }
 
 
+static void
+processMonitorEOFEvent(virQEMUDriverPtr driver,
+                       virDomainObjPtr vm)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    int eventReason = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN;
+    int stopReason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+    const char *auditReason = "shutdown";
+    unsigned int stopFlags = 0;
+    virObjectEventPtr event = NULL;
+
+    if (qemuProcessBeginStopJob(driver, vm, QEMU_JOB_DESTROY, true) < 0)
+        return;
+
+    if (!virDomainObjIsActive(vm)) {
+        VIR_DEBUG("Domain %p '%s' is not active, ignoring EOF",
+                  vm, vm->def->name);
+        goto endjob;
+    }
+
+    if (priv->monJSON && !priv->gotShutdown) {
+        VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; "
+                  "assuming the domain crashed", vm->def->name);
+        eventReason = VIR_DOMAIN_EVENT_STOPPED_FAILED;
+        stopReason = VIR_DOMAIN_SHUTOFF_CRASHED;
+        auditReason = "failed";
+    }
+
+    if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) {
+        stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
+        qemuMigrationErrorSave(driver, vm->def->name,
+                               qemuMonitorLastError(priv->mon));
+    }
+
+    event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
+                                              eventReason);
+    qemuProcessStop(driver, vm, stopReason, stopFlags);
+    virDomainAuditStop(vm, auditReason);
+    qemuDomainEventQueue(driver, event);
+
+ endjob:
+    qemuDomainObjEndJob(driver, vm);
+    qemuDomainRemoveInactive(driver, vm);
+}
+
+
 static void qemuProcessEventHandler(void *data, void *opaque)
 {
     struct qemuProcessEvent *processEvent = data;
     virDomainObjPtr vm = processEvent->vm;
     virQEMUDriverPtr driver = opaque;
 
-    VIR_DEBUG("vm=%p", vm);
+    VIR_DEBUG("vm=%p, event=%d", vm, processEvent->eventType);
 
     virObjectLock(vm);
 
@@ -4570,6 +4616,9 @@ static void qemuProcessEventHandler(void *data, void *opaque)
                              processEvent->action,
                              processEvent->status);
         break;
+    case QEMU_PROCESS_EVENT_MONITOR_EOF:
+        processMonitorEOFEvent(driver, vm);
+        break;
     case QEMU_PROCESS_EVENT_LAST:
         break;
     }

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 58a74754fece55a3bdfa97f00a75a8bfcfd5a3ac..cb87412a8d045521ebf214b2fa56f471a61bf6ff 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -914,6 +914,15 @@ qemuMonitorOpenFD(virDomainObjPtr vm,
 }
 
 
+void
+qemuMonitorUnregister(qemuMonitorPtr mon)
+{
+    if (mon->watch) {
+        virEventRemoveHandle(mon->watch);
+        mon->watch = 0;
+    }
+}
+
 void
 qemuMonitorClose(qemuMonitorPtr mon)
 {
@@ -927,10 +936,7 @@ qemuMonitorClose(qemuMonitorPtr mon)
     qemuMonitorSetDomainLog(mon, NULL, NULL, NULL);
 
     if (mon->fd >= 0) {
-        if (mon->watch) {
-            virEventRemoveHandle(mon->watch);
-            mon->watch = 0;
-        }
+        qemuMonitorUnregister(mon);
         VIR_FORCE_CLOSE(mon->fd);
     }
 

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d1a667aec66c16a0b8505a88381aa3a951bfd310..6a2a985ad8980ba96af74af0560ea802ea9ed1c6 100644 (file)
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -244,6 +244,8 @@ qemuMonitorPtr qemuMonitorOpenFD(virDomainObjPtr vm,
                                  void *opaque)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
 
+void qemuMonitorUnregister(qemuMonitorPtr mon)
+    ATTRIBUTE_NONNULL(1);
 void qemuMonitorClose(qemuMonitorPtr mon);
 
 virErrorPtr qemuMonitorLastError(qemuMonitorPtr mon);

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fd301c3e1fad719495ffc20ffb5ede7a17ad248f..a372c3da1eba63c8aa0fddda32627d806da9393d 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -286,54 +286,39 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                             void *opaque)
 {
     virQEMUDriverPtr driver = opaque;
-    virObjectEventPtr event = NULL;
     qemuDomainObjPrivatePtr priv;
-    int eventReason = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN;
-    int stopReason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
-    const char *auditReason = "shutdown";
-    unsigned int stopFlags = 0;
-
-    VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name);
+    struct qemuProcessEvent *processEvent;
 
     virObjectLock(vm);
 
-    priv = vm->privateData;
+    VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name);
 
+    priv = vm->privateData;
     if (priv->beingDestroyed) {
         VIR_DEBUG("Domain is being destroyed, EOF is expected");
         goto cleanup;
     }
 
-    if (!virDomainObjIsActive(vm)) {
-        VIR_DEBUG("Domain %p is not active, ignoring EOF", vm);
+    if (VIR_ALLOC(processEvent) < 0)
         goto cleanup;
-    }
 
-    if (priv->monJSON && !priv->gotShutdown) {
-        VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; "
-                  "assuming the domain crashed", vm->def->name);
-        eventReason = VIR_DOMAIN_EVENT_STOPPED_FAILED;
-        stopReason = VIR_DOMAIN_SHUTOFF_CRASHED;
-        auditReason = "failed";
-    }
+    processEvent->eventType = QEMU_PROCESS_EVENT_MONITOR_EOF;
+    processEvent->vm = vm;
 
-    if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) {
-        stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
-        qemuMigrationErrorSave(driver, vm->def->name,
-                               qemuMonitorLastError(priv->mon));
+    virObjectRef(vm);
+    if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
+        ignore_value(virObjectUnref(vm));
+        VIR_FREE(processEvent);
+        goto cleanup;
     }
 
-    event = virDomainEventLifecycleNewFromObj(vm,
-                                     VIR_DOMAIN_EVENT_STOPPED,
-                                     eventReason);
-    qemuProcessStop(driver, vm, stopReason, stopFlags);
-    virDomainAuditStop(vm, auditReason);
-
-    qemuDomainRemoveInactive(driver, vm);
+    /* We don't want this EOF handler to be called over and over while the
+     * thread is waiting for a job.
+     */
+    qemuMonitorUnregister(mon);
 
  cleanup:
     virObjectUnlock(vm);
-    qemuDomainEventQueue(driver, event);
 }