External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-23 06:05:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

qemu: Remove double unlock for domains

Browse Source 
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@redhat.com>
This commit is contained in:
Martin Kletzander 2015-07-15 09:07:50 +02:00
parent b7102e9031
commit c43c661fe4
2 changed files with 21 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/qemu/qemu_domain.c
View File

@ -2593,6 +2593,19 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
virObjectRef(vm); virObjectRef(vm);
virDomainObjListRemove(driver->domains, 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. This is
* just a workaround for the qemu driver.
*
* XXX: Ideally, the global handling of domain objects and object
* lists would be refactored so we don't need hacks like
* this, but since that requires refactor of all drivers,
* it's a work for another day.
*/
virObjectLock(vm);
virObjectUnref(cfg); virObjectUnref(cfg);
if (haveJob) if (haveJob)

19
src/qemu/qemu_process.c
View File

@ -295,12 +295,12 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
if (priv->beingDestroyed) { if (priv->beingDestroyed) {
VIR_DEBUG("Domain is being destroyed, EOF is expected"); VIR_DEBUG("Domain is being destroyed, EOF is expected");
goto unlock; goto cleanup;
} }
if (!virDomainObjIsActive(vm)) { if (!virDomainObjIsActive(vm)) {
VIR_DEBUG("Domain %p is not active, ignoring EOF", vm); VIR_DEBUG("Domain %p is not active, ignoring EOF", vm);
goto unlock; goto cleanup;
} }
if (priv->monJSON && !priv->gotShutdown) { if (priv->monJSON && !priv->gotShutdown) {
@ -323,15 +323,11 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
qemuProcessStop(driver, vm, stopReason, stopFlags); qemuProcessStop(driver, vm, stopReason, stopFlags);
virDomainAuditStop(vm, auditReason); virDomainAuditStop(vm, auditReason);
if (!vm->persistent) { if (!vm->persistent)
qemuDomainRemoveInactive(driver, vm); qemuDomainRemoveInactive(driver, vm);
goto cleanup;
}
unlock:
virObjectUnlock(vm);
cleanup: cleanup:
virObjectUnlock(vm);
if (event) if (event)
qemuDomainEventQueue(driver, event); qemuDomainEventQueue(driver, event);
} }
@ -5703,6 +5699,8 @@ qemuProcessAutoDestroy(virDomainObjPtr dom,
VIR_DEBUG("vm=%s, conn=%p", dom->def->name, conn); VIR_DEBUG("vm=%s, conn=%p", dom->def->name, conn);
virObjectRef(dom);
if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN)
stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
@ -5727,15 +5725,14 @@ qemuProcessAutoDestroy(virDomainObjPtr dom,
qemuDomainObjEndJob(driver, dom); qemuDomainObjEndJob(driver, dom);
if (!dom->persistent) { if (!dom->persistent)
qemuDomainRemoveInactive(driver, dom); qemuDomainRemoveInactive(driver, dom);
dom = NULL;
}
if (event) if (event)
qemuDomainEventQueue(driver, event); qemuDomainEventQueue(driver, event);
cleanup: cleanup:
virDomainObjEndAPI(&dom);
return dom; return dom;
} }