From 045d712c4bb7ce2923a4e2508dc43b0448cea357 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Fri, 28 Jul 2017 12:03:50 -0400 Subject: [PATCH] util: Introduce and use virObjectRWUnlock Rather than overload virObjectUnlock as commit id '77f4593b' has done, create a separate virObjectRWUnlock API that will force the consumers to make the proper decision regarding unlocking the RWLock's. Similar to the RWLockRead and RWLockWrite, use the virObjectGetRWLockableObj helper. This restores the virObjectUnlock code to using the virObjectGetLockableObj. Signed-off-by: John Ferlan --- src/conf/virdomainobjlist.c | 36 ++++++++++++++--------------- src/libvirt_private.syms | 1 + src/util/virobject.c | 45 ++++++++++++++++++++++++------------- src/util/virobject.h | 4 ++++ 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 573032f14b..d874133a78 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -122,7 +122,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms, obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL); if (ref) { virObjectRef(obj); - virObjectUnlock(doms); + virObjectRWUnlock(doms); } if (obj) { virObjectLock(obj); @@ -134,7 +134,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms, } } if (!ref) - virObjectUnlock(doms); + virObjectRWUnlock(doms); return obj; } @@ -166,7 +166,7 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, obj = virHashLookup(doms->objs, uuidstr); if (ref) { virObjectRef(obj); - virObjectUnlock(doms); + virObjectRWUnlock(doms); } if (obj) { virObjectLock(obj); @@ -178,7 +178,7 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, } } if (!ref) - virObjectUnlock(doms); + virObjectRWUnlock(doms); return obj; } @@ -207,7 +207,7 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, virObjectRWLockRead(doms); obj = virHashLookup(doms->objsName, name); virObjectRef(obj); - virObjectUnlock(doms); + virObjectRWUnlock(doms); if (obj) { virObjectLock(obj); if (obj->removing) { @@ -329,7 +329,7 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, virObjectRWLockWrite(doms); ret = virDomainObjListAddLocked(doms, def, xmlopt, flags, oldDef); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return ret; } @@ -355,7 +355,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms, virHashRemoveEntry(doms->objsName, dom->def->name); virObjectUnlock(dom); virObjectUnref(dom); - virObjectUnlock(doms); + virObjectRWUnlock(doms); } @@ -420,7 +420,7 @@ virDomainObjListRename(virDomainObjListPtr doms, ret = 0; cleanup: - virObjectUnlock(doms); + virObjectRWUnlock(doms); VIR_FREE(old_name); return ret; } @@ -609,7 +609,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, } VIR_DIR_CLOSE(dir); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return ret; } @@ -655,7 +655,7 @@ virDomainObjListNumOfDomains(virDomainObjListPtr doms, struct virDomainObjListData data = { filter, conn, active, 0 }; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCount, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return data.count; } @@ -699,7 +699,7 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms, 0, maxids, ids }; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return data.numids; } @@ -753,7 +753,7 @@ virDomainObjListGetInactiveNames(virDomainObjListPtr doms, size_t i; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); if (data.oom) { for (i = 0; i < data.numnames; i++) VIR_FREE(data.names[i]); @@ -794,7 +794,7 @@ virDomainObjListForEach(virDomainObjListPtr doms, }; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListHelper, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return data.ret; } @@ -928,12 +928,12 @@ virDomainObjListCollect(virDomainObjListPtr domlist, virObjectRWLockRead(domlist); sa_assert(domlist->objs); if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) { - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); return -1; } virHashForEach(domlist->objs, virDomainObjListCollectIterator, &data); - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); virDomainObjListFilter(&data.vms, &data.nvms, conn, filter, flags); @@ -972,7 +972,7 @@ virDomainObjListConvert(virDomainObjListPtr domlist, if (skip_missing) continue; - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); virReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s' (%s)"), uuidstr, dom->name); @@ -982,12 +982,12 @@ virDomainObjListConvert(virDomainObjListPtr domlist, virObjectRef(vm); if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0) { - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); virObjectUnref(vm); goto error; } } - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); sa_assert(*vms); virDomainObjListFilter(vms, nvms, conn, filter, flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 01a2f3a814..6e4c3e83b9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2313,6 +2313,7 @@ virObjectRef; virObjectRWLockableNew; virObjectRWLockRead; virObjectRWLockWrite; +virObjectRWUnlock; virObjectUnlock; virObjectUnref; diff --git a/src/util/virobject.c b/src/util/virobject.c index c1e4474caf..85e5a537ef 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -428,7 +428,7 @@ virObjectLock(void *anyobj) * @anyobj: any instance of virObjectRWLockable * * Acquire a read lock on @anyobj. The lock must be - * released by virObjectUnlock. + * released by virObjectRWUnlock. * * The caller is expected to have acquired a reference * on the object before locking it (eg virObjectRef). @@ -457,7 +457,7 @@ virObjectRWLockRead(void *anyobj) * @anyobj: any instance of virObjectRWLockable * * Acquire a write lock on @anyobj. The lock must be - * released by virObjectUnlock. + * released by virObjectRWUnlock. * * The caller is expected to have acquired a reference * on the object before locking it (eg virObjectRef). @@ -483,26 +483,39 @@ virObjectRWLockWrite(void *anyobj) /** * virObjectUnlock: - * @anyobj: any instance of virObjectLockable or virObjectRWLockable + * @anyobj: any instance of virObjectLockable * * Release a lock on @anyobj. The lock must have been acquired by - * virObjectLock, virObjectRWLockRead, or virObjectRWLockWrite. + * virObjectLock. */ void virObjectUnlock(void *anyobj) { - if (virObjectIsClass(anyobj, virObjectLockableClass)) { - virObjectLockablePtr obj = anyobj; - virMutexUnlock(&obj->lock); - } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { - virObjectRWLockablePtr obj = anyobj; - virRWLockUnlock(&obj->lock); - } else { - virObjectPtr obj = anyobj; - VIR_WARN("Object %p (%s) is not a virObjectLockable " - "nor virObjectRWLockable instance", - anyobj, obj ? obj->klass->name : "(unknown)"); - } + virObjectLockablePtr obj = virObjectGetLockableObj(anyobj); + + if (!obj) + return; + + virMutexUnlock(&obj->lock); +} + + +/** + * virObjectRWUnlock: + * @anyobj: any instance of virObjectRWLockable + * + * Release a lock on @anyobj. The lock must have been acquired by + * virObjectRWLockRead or virObjectRWLockWrite. + */ +void +virObjectRWUnlock(void *anyobj) +{ + virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj); + + if (!obj) + return; + + virRWLockUnlock(&obj->lock); } diff --git a/src/util/virobject.h b/src/util/virobject.h index 24ee6dda8b..ac6cf22f9e 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -136,6 +136,10 @@ void virObjectUnlock(void *lockableobj) ATTRIBUTE_NONNULL(1); +void +virObjectRWUnlock(void *lockableobj) + ATTRIBUTE_NONNULL(1); + void virObjectListFree(void *list);