[libvirt] [PATCH 6/9] qemu: Process monitor EOF in a job

Jiri Denemark jdenemar at redhat.com
Tue Feb 16 14:36:58 UTC 2016


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 at redhat.com>
---
 src/qemu/qemu_domain.h  |  1 +
 src/qemu/qemu_driver.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/qemu/qemu_monitor.c | 14 ++++++++++----
 src/qemu/qemu_monitor.h |  2 ++
 src/qemu/qemu_process.c | 45 +++++++++++++++----------------------------
 5 files changed, 78 insertions(+), 35 deletions(-)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 5812174..8655ba8 100644
--- 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 0bb0759..dc448fb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4563,13 +4563,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);
 
@@ -4596,6 +4642,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 6b23e88..c3583ff 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -915,6 +915,15 @@ qemuMonitorOpenFD(virDomainObjPtr vm,
 
 
 void
+qemuMonitorUnregister(qemuMonitorPtr mon)
+{
+    if (mon->watch) {
+        virEventRemoveHandle(mon->watch);
+        mon->watch = 0;
+    }
+}
+
+void
 qemuMonitorClose(qemuMonitorPtr mon)
 {
     if (!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 9d7d5f3..21cb256 100644
--- 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 2f9df7c..e3d9f3d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -284,54 +284,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;
+    struct qemuProcessEvent *processEvent;
+
+    virObjectLock(vm);
 
     VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name);
 
-    virObjectLock(vm);
-
     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);
 }
 
 
-- 
2.7.1




More information about the libvir-list mailing list