]> xenbits.xensource.com Git - libvirt.git/commitdiff
qemu: Don't leave beingDestroyed=true on inactive domain
authorJiri Denemark <jdenemar@redhat.com>
Thu, 11 Jul 2024 11:49:09 +0000 (13:49 +0200)
committerJiri Denemark <jdenemar@redhat.com>
Fri, 12 Jul 2024 09:27:03 +0000 (11:27 +0200)
Recent commit v10.4.0-87-gd9935a5c4f made a reasonable change to only
reset beingDestroyed back to false when vm->def->id is reset to make
sure other code can detect a domain is (about to become) inactive. It
even added a comment saying any caller of qemuProcessBeginStopJob is
supposed to call qemuProcessStop to clear beingDestroyed. But not every
caller really does so because they first call qemuProcessBeginStopJob
and then check whether a domain is still running. If not the
qemuProcessStop call is skipped leaving beingDestroyed=true. In case of
a persistent domain this may block incoming migrations of such domain as
the migration code would think the domain died unexpectedly (even though
it's still running).

The qemuProcessBeginStopJob function is a wrapper around
virDomainObjBeginJob, but virDomainObjEndJob was used directly for
cleanup. This patch introduces a new qemuProcessEndStopJob wrapper
around virDomainObjEndJob to properly undo everything
qemuProcessBeginStopJob did.

https://issues.redhat.com/browse/RHEL-43309

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
src/qemu/qemu_driver.c
src/qemu/qemu_process.c
src/qemu/qemu_process.h

index f898d856671be651889626b75088892d22cd9b12..9f3013e231907ef3614a59287cb8659a4f1eb071 100644 (file)
@@ -2102,7 +2102,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
  endjob:
     if (ret == 0)
         qemuDomainRemoveInactive(driver, vm, 0, false);
-    virDomainObjEndJob(vm);
+    qemuProcessEndStopJob(vm);
 
  cleanup:
     virDomainObjEndAPI(&vm);
@@ -3888,7 +3888,7 @@ processMonitorEOFEvent(virQEMUDriver *driver,
 
  endjob:
     qemuDomainRemoveInactive(driver, vm, 0, false);
-    virDomainObjEndJob(vm);
+    qemuProcessEndStopJob(vm);
 }
 
 
index 7cbe521a6e40c5471c29b99410bb893192e5466d..25dfd042724ef5dbb610becb60766309c08e6b3c 100644 (file)
@@ -8421,7 +8421,8 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags)
  * qemuProcessBeginStopJob:
  *
  * Stop all current jobs by killing the domain and start a new one for
- * qemuProcessStop.
+ * qemuProcessStop. The caller has to make sure qemuProcessEndStopJob is
+ * called to properly cleanup the job.
  */
 int
 qemuProcessBeginStopJob(virDomainObj *vm,
@@ -8448,8 +8449,9 @@ qemuProcessBeginStopJob(virDomainObj *vm,
         goto error;
 
     /* 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 */
+     * is supposed to call qemuProcessStop (which will reset it after
+     * 'vm->def->id' is set to -1) and/or qemuProcessEndStopJob to do proper
+     * cleanup. */
     return 0;
 
  error:
@@ -8458,6 +8460,16 @@ qemuProcessBeginStopJob(virDomainObj *vm,
 }
 
 
+void
+qemuProcessEndStopJob(virDomainObj *vm)
+{
+    if (!virDomainObjIsActive(vm))
+        QEMU_DOMAIN_PRIVATE(vm)->beingDestroyed = false;
+
+    virDomainObjEndJob(vm);
+}
+
+
 void qemuProcessStop(virQEMUDriver *driver,
                      virDomainObj *vm,
                      virDomainShutoffReason reason,
@@ -8800,7 +8812,7 @@ qemuProcessAutoDestroy(virDomainObj *dom,
 
     qemuDomainRemoveInactive(driver, dom, 0, false);
 
-    virDomainObjEndJob(dom);
+    qemuProcessEndStopJob(dom);
 
     virObjectEventStateQueue(driver->domainEventState, event);
 }
index c1ea9492151e51ed547cdade01e44965c79134cb..cb67bfcd2dfe82d0bede19d744ac383d38758154 100644 (file)
@@ -169,6 +169,7 @@ typedef enum {
 int qemuProcessBeginStopJob(virDomainObj *vm,
                             virDomainJob job,
                             bool forceKill);
+void qemuProcessEndStopJob(virDomainObj *vm);
 void qemuProcessStop(virQEMUDriver *driver,
                      virDomainObj *vm,
                      virDomainShutoffReason reason,