[PATCH v2] qemuDomainObjWait: Report error when VM is being destroyed

Peter Krempa posted 1 patch 1 year, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/22678872e5565e8e17da177ab52032e2a0c2a03a.1660217122.git.pkrempa@redhat.com
src/qemu/qemu_domain.c    | 12 +++++++++++-
src/qemu/qemu_migration.c |  2 +-
2 files changed, 12 insertions(+), 2 deletions(-)
[PATCH v2] qemuDomainObjWait: Report error when VM is being destroyed
Posted by Peter Krempa 1 year, 8 months ago
Since we started handing the monitor EOF event inside a job any code
which uses virDomainObjWait would no longer properly abort in case when
the VM crashed during the wait.

This is because virDomainObjWait uses virDomainObjIsActive which checks
'vm->def->id' to see if the VM is still active. Unfortunately the domain
id is cleared in qemuProcessStop which is run only inside the job.

To fix this we can use the 'beingDestroyed' flag stored in the VM
private data which is set to true around the time when the condition is
signalled.

Reported-by: Pavel Hrdina <phrdina@redhat.com>
Fixes: 8c9ff9960b29d4703a99efdd1cadcf6f48799cc0
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---

v2: Fix condition in qemuMigrationSrcWaitForCompletion, where we check
    whether the VM is active to use same logic

 src/qemu/qemu_domain.c    | 12 +++++++++++-
 src/qemu/qemu_migration.c |  2 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2caed7315b..43b5ad5d6e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11780,5 +11780,15 @@ qemuDomainRemoveLogs(virQEMUDriver *driver,
 int
 qemuDomainObjWait(virDomainObj *vm)
 {
-    return virDomainObjWait(vm);
+    qemuDomainObjPrivate *priv = vm->privateData;
+
+    if (virDomainObjWait(vm) < 0)
+        return -1;
+
+    if (priv->beingDestroyed) {
+        virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("domain is not running"));
+        return -1;
+    }
+
+    return 0;
 }
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0b48852b9d..8a8e9ab207 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2078,7 +2078,7 @@ qemuMigrationSrcWaitForCompletion(virDomainObj *vm,
             return rv;

         if (qemuDomainObjWait(vm) < 0) {
-            if (virDomainObjIsActive(vm))
+            if (virDomainObjIsActive(vm) && !priv->beingDestroyed)
                 jobData->status = VIR_DOMAIN_JOB_STATUS_FAILED;
             return -2;
         }
-- 
2.37.1
Re: [PATCH v2] qemuDomainObjWait: Report error when VM is being destroyed
Posted by Ján Tomko 1 year, 8 months ago
On a Thursday in 2022, Peter Krempa wrote:
>Since we started handing the monitor EOF event inside a job any code

*handling

>which uses virDomainObjWait would no longer properly abort in case when
>the VM crashed during the wait.
>
>This is because virDomainObjWait uses virDomainObjIsActive which checks
>'vm->def->id' to see if the VM is still active. Unfortunately the domain
>id is cleared in qemuProcessStop which is run only inside the job.
>
>To fix this we can use the 'beingDestroyed' flag stored in the VM
>private data which is set to true around the time when the condition is
>signalled.
>
>Reported-by: Pavel Hrdina <phrdina@redhat.com>
>Fixes: 8c9ff9960b29d4703a99efdd1cadcf6f48799cc0
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano