qemu: process: Ensure that 'beingDestroyed' gets cleared only after VM id is reset

Prevent the possibility that a VM could be considered as alive while
inside qemuProcessStop.

A recently fixed bug which unlocked the domain object while inside
qemuProcessStop showed that there's possibility to confuse the state of
the VM to be considered active while 'qemuProcessStop' is processing
shutdown of the VM. Ensure that this doesn't happen by clearing the
'beingDestroyed' flag only after the VM id is cleared.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4a7cbda16eac5f945c2d8eab9979c510ae69c0bd..ae6594e10e2f4dfb1b3c580995eaf72a2c7a6844 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8419,29 +8419,31 @@ qemuProcessBeginStopJob(virDomainObj *vm,
 {
     qemuDomainObjPrivate *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
-     */
+     * BeginJob/ProcessKill API or any other code path before 'vm->def->id' is
+     * cleared inside qemuProcessStop */
     priv->beingDestroyed = true;
 
     if (qemuProcessKill(vm, killFlags) < 0)
-        goto cleanup;
+        goto error;
 
     /* Wake up anything waiting on domain condition */
     VIR_DEBUG("waking up all jobs waiting on the domain condition");
     virDomainObjBroadcast(vm);
 
     if (virDomainObjBeginJob(vm, job) < 0)
-        goto cleanup;
+        goto error;
 
-    ret = 0;
+    /* priv->beingDestroyed is deliberately left set to 'true' here. Caller
+     * is supposed to call qemuProcessStop, which will reset it after
+     * 'vm->def->id' is set to -1 */
+    return 0;
 
- cleanup:
+ error:
     priv->beingDestroyed = false;
-    return ret;
+    return -1;
 }
 
 
@@ -8528,7 +8530,13 @@ void qemuProcessStop(virQEMUDriver *driver,
 
     qemuDBusStop(driver, vm);
 
+    /* Only after this point we can reset 'priv->beingDestroyed' so that
+     * there's no point at which the VM could be considered as alive between
+     * entering the destroy job and this point where the active "flag" is
+     * cleared.
+     */
     vm->def->id = -1;
+    priv->beingDestroyed = false;
 
     /* Wake up anything waiting on domain condition */
     virDomainObjBroadcast(vm);