[PATCH REBASE 5/5] qemu: fix races in beingDestroyed usage

[Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>]

Clearing beingDestroyed right after acquiring job condition is racy.
It is not known when EOF will be delivired. Let's keep this flag
set. This makes possible to make a clear distinction between monitor
errors/eofs and domain being destroyed in qemuDomainObjWait.

Also let's move setting destroyed flag out of qemuProcessBeginStopJob
as the function is called from eof handler too.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
---
 src/qemu/qemu_domain.c  |  4 ++--
 src/qemu/qemu_domain.h  |  2 +-
 src/qemu/qemu_driver.c  |  8 +++++++-
 src/qemu/qemu_process.c | 24 ++++++++++++------------
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1f40ff1..431901c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11913,9 +11913,9 @@ qemuDomainObjWait(virDomainObjPtr vm, unsigned long long until)
         return -1;
     }
 
-    if (!virDomainObjIsActive(vm)) {
+    if (priv->destroyed) {
         virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                       _("domain is not running"));
+                       _("domain is destroyed"));
         return -1;
     }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 494ed35..69112ea 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -269,7 +269,7 @@ struct _qemuDomainObjPrivate {
     bool agentError;
 
     bool gotShutdown;
-    bool beingDestroyed;
+    bool destroyed;
     char *pidfile;
 
     virDomainPCIAddressSetPtr pciaddrs;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 03969d8..4356c0d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2227,7 +2227,13 @@ qemuDomainDestroyFlags(virDomainPtr dom,
     state = virDomainObjGetState(vm, &reason);
     starting = (state == VIR_DOMAIN_PAUSED &&
                 reason == VIR_DOMAIN_PAUSED_STARTING_UP &&
-                !priv->beingDestroyed);
+                !priv->destroyed);
+
+    /* We need to prevent monitor EOF callback from doing our work (and
+     * sending misleading events) while the vm is unlocked inside
+     * BeginJob/ProcessKill API
+     */
+    priv->destroyed = true;
 
     if (qemuProcessBeginStopJob(driver, vm, QEMU_JOB_DESTROY,
                                 !(flags & VIR_DOMAIN_DESTROY_GRACEFUL)) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d76809e..689fc8b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -143,8 +143,8 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
         goto unlock;
     }
 
-    if (priv->beingDestroyed) {
-        VIR_DEBUG("Domain is being destroyed, agent EOF is expected");
+    if (priv->destroyed) {
+        VIR_DEBUG("Domain is destroyed, agent EOF is expected");
         goto unlock;
     }
 
@@ -286,6 +286,7 @@ qemuProcessNotifyMonitorError(virDomainObjPtr vm,
     virFreeError(err);
 }
 
+
 /*
  * This is a callback registered with a qemuMonitorPtr instance,
  * and to be invoked when the monitor console hits an end of file
@@ -308,8 +309,8 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon,
     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");
+    if (priv->destroyed) {
+        VIR_DEBUG("Domain is destroyed, EOF is expected");
         goto cleanup;
     }
 
@@ -5750,6 +5751,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
     virResetError(&priv->monError);
     priv->monStart = 0;
     priv->gotShutdown = false;
+    priv->destroyed = false;
 
     VIR_DEBUG("Updating guest CPU definition");
     if (qemuProcessUpdateGuestCPU(vm->def, priv->qemuCaps, caps, flags) < 0)
@@ -6490,16 +6492,9 @@ qemuProcessBeginStopJob(virQEMUDriverPtr driver,
                         qemuDomainJob job,
                         bool forceKill)
 {
-    qemuDomainObjPrivatePtr priv = vm->privateData;
     unsigned int killFlags = forceKill ? VIR_QEMU_PROCESS_KILL_FORCE : 0;
     int ret = -1;
 
-    /* We need to prevent monitor EOF callback from doing our work (and
-     * sending misleading events) while the vm is unlocked inside
-     * BeginJob/ProcessKill API
-     */
-    priv->beingDestroyed = true;
-
     if (qemuProcessKill(vm, killFlags) < 0)
         goto cleanup;
 
@@ -6509,7 +6504,6 @@ qemuProcessBeginStopJob(virQEMUDriverPtr driver,
     ret = 0;
 
  cleanup:
-    priv->beingDestroyed = false;
     return ret;
 }
 
@@ -7088,6 +7082,12 @@ qemuProcessAutoDestroy(virDomainObjPtr dom,
 
     VIR_DEBUG("Killing domain");
 
+    /* We need to prevent monitor EOF callback from doing our work (and
+     * sending misleading events) while the vm is unlocked inside
+     * BeginJob/ProcessKill API
+     */
+    priv->destroyed = true;
+
     if (qemuProcessBeginStopJob(driver, dom, QEMU_JOB_DESTROY, true) < 0)
         return;
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list