From e6afacb0feabd9bf58331aa0e5259a35378be74e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Wed, 12 Feb 2020 12:26:11 +0000
Subject: [PATCH] qemu: start/stop an event loop thread for domains
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The event loop thread will be responsible for handling
any per-domain I/O operations, most notably the QEMU
monitor and agent sockets.

We start this event loop when launching QEMU, but stopping
the event loop is a little more complicated. The obvious
idea is to stop it in qemuProcessStop(), but if we do that
we risk loosing the final events from the QEMU monitor, as
they might not have been read by the event thread at the
time we tell the thread to stop.

The solution is to delay shutdown of the event thread until
we have seen EOF from the QEMU monitor, and thus we know
there are no further events to process.

Note that this assumes that we don't have events to process
from the QEMU agent.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/qemu/qemu_domain.c  | 33 +++++++++++++++++++++++++++++++++
 src/qemu/qemu_domain.h  |  6 ++++++
 src/qemu/qemu_process.c | 21 +++++++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3d3f796d85..bcdd518be6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2150,6 +2150,33 @@ dbusVMStateHashFree(void *opaque)
 }
 
 
+int
+qemuDomainObjStartWorker(virDomainObjPtr dom)
+{
+    qemuDomainObjPrivatePtr priv = dom->privateData;
+
+    if (!priv->eventThread) {
+        g_autofree char *threadName = g_strdup_printf("vm-%s", dom->def->name);
+        if (!(priv->eventThread = virEventThreadNew(threadName)))
+            return -1;
+    }
+
+    return 0;
+}
+
+
+void
+qemuDomainObjStopWorker(virDomainObjPtr dom)
+{
+    qemuDomainObjPrivatePtr priv = dom->privateData;
+
+    if (priv->eventThread) {
+        g_object_unref(priv->eventThread);
+        priv->eventThread = NULL;
+    }
+}
+
+
 static void *
 qemuDomainObjPrivateAlloc(void *opaque)
 {
@@ -2289,6 +2316,12 @@ qemuDomainObjPrivateFree(void *data)
     virHashFree(priv->blockjobs);
     virHashFree(priv->dbusVMStates);
 
+    /* This should never be non-NULL if we get here, but just in case... */
+    if (priv->eventThread) {
+        VIR_ERROR(_("Unexpected event thread still active during domain deletion"));
+        g_object_unref(priv->eventThread);
+    }
+
     VIR_FREE(priv);
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 3929ee9ca1..099ee59772 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -40,6 +40,7 @@
 #include "logging/log_manager.h"
 #include "virdomainmomentobjlist.h"
 #include "virenum.h"
+#include "vireventthread.h"
 
 #define QEMU_DOMAIN_FORMAT_LIVE_FLAGS \
     (VIR_DOMAIN_XML_SECURE)
@@ -300,6 +301,8 @@ struct _qemuDomainObjPrivate {
 
     virBitmapPtr namespaces;
 
+    virEventThread *eventThread;
+
     qemuMonitorPtr mon;
     virDomainChrSourceDefPtr monConfig;
     bool monError;
@@ -630,6 +633,9 @@ struct _qemuDomainXmlNsDef {
     char **capsdel;
 };
 
+int qemuDomainObjStartWorker(virDomainObjPtr dom);
+void qemuDomainObjStopWorker(virDomainObjPtr dom);
+
 virDomainObjPtr qemuDomainObjFromDomain(virDomainPtr domain);
 
 qemuDomainSaveCookiePtr qemuDomainSaveCookieNew(virDomainObjPtr vm);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 499d39a920..82d2dde15b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -320,6 +320,9 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon,
     qemuDomainDestroyNamespace(driver, vm);
 
  cleanup:
+    /* Now we got EOF we're not expecting more I/O, so we
+     * can finally kill the event thread */
+    qemuDomainObjStopWorker(vm);
     virObjectUnlock(vm);
 }
 
@@ -6908,6 +6911,9 @@ qemuProcessLaunch(virConnectPtr conn,
     if (rv == -1) /* The VM failed to start */
         goto cleanup;
 
+    if (qemuDomainObjStartWorker(vm) < 0)
+        goto cleanup;
+
     VIR_DEBUG("Waiting for monitor to show up");
     if (qemuProcessWaitForMonitor(driver, vm, asyncJob, logCtxt) < 0)
         goto cleanup;
@@ -7390,6 +7396,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
         priv->monConfig = NULL;
     }
 
+    /*
+     * We cannot stop the event thread at this time. When
+     * we are in this code, we may not yet have processed the
+     * STOP event or EOF from the monitor. So the event loop
+     * may have pending input that we need to process still.
+     * The qemuProcessHandleMonitorEOF method will kill
+     * the event thread because at that point we don't
+     * expect any more I/O from the QEMU monitor. We are
+     * assuming we don't need to get any more events from the
+     * QEMU agent at that time.
+     */
+
     /* Remove the master key */
     qemuDomainMasterKeyRemove(priv);
 
@@ -7981,6 +7999,9 @@ qemuProcessReconnect(void *opaque)
         virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS))
         retry = false;
 
+    if (qemuDomainObjStartWorker(obj) < 0)
+        goto error;
+
     VIR_DEBUG("Reconnect monitor to def=%p name='%s' retry=%d",
               obj, obj->def->name, retry);