Re: [libvirt] [PATCH] qemu: qemuDomainRename and virDomainObjListNumOfDomains ABBA deadlock fix

A pretty nasty deadlock occurs while trying to rename a VM in parallel with virDomainObjListNumOfDomains. The short description of the problem is as follows: Thread #1: qemuDomainRename: ------> aquires domain lock by qemuDomObjFromDomain ---------> waits for domain list lock in any of the listed functions: - virDomainObjListFindByName - virDomainObjListRenameAddNew - virDomainObjListRenameRemove Thread #2: virDomainObjListNumOfDomains: ------> aquires domain list lock ---------> waits for domain lock in virDomainObjListCount

The patch establishes a single order of taking locks: driver->domains list first, then a particular VM. This is done by implementing a set of virDomainObjListXxxLocked functions working with driver->domains that assume that list lock is already aquired by calling functions. Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com&gt; --- src/conf/virdomainobjlist.c | 74 +++++++++++++++++++++++++++++++++++++-------- src/conf/virdomainobjlist.h | 9 ++++++ src/libvirt_private.syms | 4 +++ src/qemu/qemu_driver.c | 40 +++++++++++++++++++++--- 4 files changed, 110 insertions(+), 17 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 7568f93..89e28a8 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -132,21 +132,19 @@ virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms, static virDomainObjPtr -virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, +virDomainObjListFindByUUIDInternalLocked(virDomainObjListPtr doms, const unsigned char *uuid, bool ref)

{ char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr obj; - virObjectLock(doms); virUUIDFormat(uuid, uuidstr); obj = virHashLookup(doms->objs, uuidstr); - if (ref) { + if (ref) virObjectRef(obj); - virObjectUnlock(doms); - } + if (obj) { virObjectLock(obj); if (obj->removing) { @@ -156,8 +154,19 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, obj = NULL; } } - if (!ref) - virObjectUnlock(doms); + return obj; +} + +static virDomainObjPtr +virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, + const unsigned char *uuid, + bool ref) +{ + virDomainObjPtr obj; + + virObjectLock(doms); + obj = virDomainObjListFindByUUIDInternalLocked(doms, uuid, ref); + virObjectUnlock(doms);