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

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(a)redhat.com&gt; --- 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; }