[libvirt] [PATCH] qemu: Remove double unlock for domains

[Martin Kletzander]

On Wed, Jul 15, 2015 at 09:17:46AM +0200, Martin Kletzander wrote:
>The virDomainObjListRemove() function unlocks a domain that it's given
>due to legacy code.  And because of that code, which should be
>refactored, that last virObjectUnlock() cannot be just removed.  So
>instead, lock it right back for qemu for now.  All calls to
>qemuDomainRemoveInactive() are followed by code that unlocks the domain
>again, plus the domain should be locked during qemuDomainObjEndJob(), so
>the right place to lock it is right after virDomainObjListRemove().
>
>The only place where this would cause a problem is the autodestroy
>callback, so we need to get another reference there and uref+unlock it
>afterwards.  Luckily, returning NULL from that function doesn't mean an
>error, and only means that it doesn't need to be unlocked anymore.
>
>Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>---

Post-release ping

> src/qemu/qemu_domain.c  | 7 +++++++
> src/qemu/qemu_process.c | 7 ++++---
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 8b050a043995..d6d723112638 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -2593,6 +2593,13 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
>     virObjectRef(vm);
>
>     virDomainObjListRemove(driver->domains, vm);
>+    /*
>+     * virDomainObjListRemove() leaves the domain unlocked so it can
>+     * be unref'd for other drivers that depend on that, but we still
>+     * need to reset a job and we have a reference from the API that
>+     * called this function.  So we need to lock it back.
>+     */
>+    virObjectLock(vm);
>     virObjectUnref(cfg);
>
>     if (haveJob)
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 1c0c734c3811..df38dacdca92 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -5696,6 +5696,8 @@ qemuProcessAutoDestroy(virDomainObjPtr dom,
>
>     VIR_DEBUG("vm=%s, conn=%p", dom->def->name, conn);
>
>+    virObjectRef(dom);
>+
>     if (priv->job.asyncJob) {
>         VIR_DEBUG("vm=%s has long-term job active, cancelling",
>                   dom->def->name);
>@@ -5718,15 +5720,14 @@ qemuProcessAutoDestroy(virDomainObjPtr dom,
>
>     qemuDomainObjEndJob(driver, dom);
>
>-    if (!dom->persistent) {
>+    if (!dom->persistent)
>         qemuDomainRemoveInactive(driver, dom);
>-        dom = NULL;
>-    }
>
>     if (event)
>         qemuDomainEventQueue(driver, event);
>
>  cleanup:
>+    virDomainObjEndAPI(&dom);
>     return dom;
> }
>
>--
>2.4.5
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150803/76b58c8d/attachment-0001.sig>