Re: [libvirt] [PATCH 2/7] util: Introduce and use virObjectLockWrite

Instead of making virObjectLock be the entry point for two different types of locks, let's create a virObjectLockWrite API which will be able to return status and force the (new) consumers of the RWLock to make sure the lock is really obtained when the "right" object type is passed. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virdomainobjlist.c | 18 ++++++++++++++---- src/libvirt_private.syms | 1 + src/util/virobject.c | 32 ++++++++++++++++++++++++++++++++ src/util/virobject.h | 4 ++++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index fed4029..08a51cc 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -330,7 +330,8 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, { virDomainObjPtr ret; - virObjectLock(doms); + if (virObjectLockWrite(doms) < 0) + return NULL; ret = virDomainObjListAddLocked(doms, def, xmlopt, flags, oldDef); virObjectUnlock(doms); return ret; @@ -352,7 +353,9 @@ void virDomainObjListRemove(virDomainObjListPtr doms, virObjectRef(dom); virObjectUnlock(dom); - virObjectLock(doms); + /* We really shouldn't ignore this, + * but that ship sailed a long time ago */ + ignore_value(virObjectLockWrite(doms)); virObjectLock(dom); virHashRemoveEntry(doms->objs, uuidstr); virHashRemoveEntry(doms->objsName, dom->def->name); @@ -397,9 +400,13 @@ virDomainObjListRename(virDomainObjListPtr doms, * hold a lock on dom but not refcount it. */ virObjectRef(dom); virObjectUnlock(dom); - virObjectLock(doms); + rc = virObjectLockWrite(doms); virObjectLock(dom); virObjectUnref(dom); + if (rc < 0) { + VIR_FREE(old_name); + return ret; + } if (virHashLookup(doms->objsName, new_name) != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -576,7 +583,10 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) return rc; - virObjectLock(doms); + if (virObjectLockWrite(doms) < 0) { + VIR_DIR_CLOSE(dir); + return -1; + } while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virDomainObjPtr dom; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 37b815c..f1a6510 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2306,6 +2306,7 @@ virObjectListFreeCount; virObjectLock; virObjectLockableNew; virObjectLockRead; +virObjectLockWrite; virObjectNew; virObjectRef; virObjectRWLockableNew; diff --git a/src/util/virobject.c b/src/util/virobject.c index 73de4d3..c48f88c 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -429,6 +429,38 @@ virObjectLockRead(void *anyobj) /** + * virObjectLockWrite: + * @anyobj: any instance of virObjectRWLockable + * + * Acquire a write lock on @anyobj. The lock must be + * released by virObjectUnlock. + * + * The caller is expected to have acquired a reference + * on the object before locking it (eg virObjectRef). + * The object must be unlocked before releasing this + * reference. + * + * Returns 0 on success, -1 on failure + */ +int +virObjectLockWrite(void *anyobj) +{ + if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { + virObjectRWLockablePtr obj = anyobj; + virRWLockWrite(&obj->lock); + return 0; + } else { + virObjectPtr obj = anyobj; + VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance", + anyobj, obj ? obj->klass->name : "(unknown)"); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to obtain rwlock for object=%p"), anyobj); + } + return -1; +}