[libvirt] [PATCH] qemu: Don't report failure to destroy a destroyed domain

Jiri Denemark posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/4c11ed7afb910805e788105ae6414e7071eb4e9e.1504898734.git.jdenemar@redhat.com
src/qemu/qemu_driver.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
[libvirt] [PATCH] qemu: Don't report failure to destroy a destroyed domain
Posted by Jiri Denemark 6 years, 7 months ago
When destroying a domain libvirt marks it internally with a
beingDestroyed flag to make sure the qemuDomainDestroyFlags API itself
cleans up after the domain rather than letting an uninformed EOF handler
do it. However, when the domain is being started at the moment libvirt
was asked to destroy it, only the starting thread can properly clean up
after the domain and thus it ignores the beingDestroyed flag. Once
qemuDomainDestroyFlags finally gets a job, the domain may not be running
anymore, which should not be reported as an error if the domain has been
starting up.

https://bugzilla.redhat.com/show_bug.cgi?id=1445600

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_driver.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6255d89310..a25daae866 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2224,6 +2224,9 @@ qemuDomainDestroyFlags(virDomainPtr dom,
     virObjectEventPtr event = NULL;
     qemuDomainObjPrivatePtr priv;
     unsigned int stopFlags = 0;
+    int state;
+    int reason;
+    bool starting;
 
     virCheckFlags(VIR_DOMAIN_DESTROY_GRACEFUL, -1);
 
@@ -2235,13 +2238,29 @@ qemuDomainDestroyFlags(virDomainPtr dom,
     if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
+    if (!virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("domain is not running"));
+        goto cleanup;
+    }
+
+    state = virDomainObjGetState(vm, &reason);
+    starting = (state == VIR_DOMAIN_PAUSED &&
+                reason == VIR_DOMAIN_PAUSED_STARTING_UP &&
+                !priv->beingDestroyed);
+
     if (qemuProcessBeginStopJob(driver, vm, QEMU_JOB_DESTROY,
                                 !(flags & VIR_DOMAIN_DESTROY_GRACEFUL)) < 0)
         goto cleanup;
 
     if (!virDomainObjIsActive(vm)) {
-        virReportError(VIR_ERR_OPERATION_INVALID,
-                       "%s", _("domain is not running"));
+        if (starting) {
+            VIR_DEBUG("Domain %s is not running anymore", vm->def->name);
+            ret = 0;
+        } else {
+            virReportError(VIR_ERR_OPERATION_INVALID,
+                           "%s", _("domain is not running"));
+        }
         goto endjob;
     }
 
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't report failure to destroy a destroyed domain
Posted by Martin Kletzander 6 years, 7 months ago
On Fri, Sep 08, 2017 at 09:25:34PM +0200, Jiri Denemark wrote:
>When destroying a domain libvirt marks it internally with a
>beingDestroyed flag to make sure the qemuDomainDestroyFlags API itself
>cleans up after the domain rather than letting an uninformed EOF handler
>do it. However, when the domain is being started at the moment libvirt
>was asked to destroy it, only the starting thread can properly clean up
>after the domain and thus it ignores the beingDestroyed flag. Once
>qemuDomainDestroyFlags finally gets a job, the domain may not be running
>anymore, which should not be reported as an error if the domain has been
>starting up.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1445600
>

Kinda hairy, but I understood it after reading the bug as well.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list