[...]
/** * virObjectLock: - * @anyobj: any instance of virObjectLockablePtr + * @anyobj: any instance of virObjectLockable or virObjectRWLockable * - * Acquire a lock on @anyobj. The lock must be - * released by virObjectUnlock. + * Acquire a lock on @anyobj. The lock must be released by + * virObjectUnlock. In case the passed object is instance of + * virObjectRWLockable a write lock is acquired. * * The caller is expected to have acquired a reference * on the object before locking it (eg virObjectRef). @@ -340,31 +382,69 @@ virObjectGetLockableObj(void *anyobj) void virObjectLock(void *anyobj) { - virObjectLockablePtr obj = virObjectGetLockableObj(anyobj); + if (virObjectIsClass(anyobj, virObjectLockableClass)) { + virObjectLockablePtr obj = anyobj; + virMutexLock(&obj->lock); + } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { + virObjectRWLockablePtr obj = anyobj; + virRWLockWrite(&obj->lock); + } else { + virObjectPtr obj = anyobj; + VIR_WARN("Object %p (%s) is not a virObjectLockable " + "nor virObjectRWLockable instance", + anyobj, obj ? obj->klass->name : "(unknown)"); + } +}
- if (!obj) - return;
- virMutexLock(&obj->lock); +/** + * virObjectLockRead: + * @anyobj: any instance of virObjectRWLockablePtr + * + * Acquire a read 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. + */ +void +virObjectLockRead(void *anyobj) +{ + if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { + virObjectRWLockablePtr obj = anyobj; + virRWLockRead(&obj->lock); + } else { + virObjectPtr obj = anyobj; + VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance", + anyobj, obj ? obj->klass->name : "(unknown)"); + } }
Hopefully no one "confuses" which object is which or no one starts using virObjectLockRead for a virObjectLockable and expects that read lock to be in place (or error out) or gasp actually wait for a write lock to release the lock as this does not error out. This is a danger in the long term assumption of having a void function that has callers that can make the assumption that upon return the Lock really was taken. Perhaps the better way to attack this would have been to create a virObjectLockWrite() function called only from vir*ObjListAdd and vir*ObjListRemove leaving the other virObjectLock()'s in tact and having the virObjectLock() code make the decision over whether to use the virRWLockRead or virMutexLock depending on the object class. John
/** * virObjectUnlock: - * @anyobj: any instance of virObjectLockablePtr + * @anyobj: any instance of virObjectLockable or virObjectRWLockable * - * Release a lock on @anyobj. The lock must have been - * acquired by virObjectLock. + * Release a lock on @anyobj. The lock must have been acquired by + * virObjectLock or virObjectLockRead. */ void virObjectUnlock(void *anyobj) { - virObjectLockablePtr obj = virObjectGetLockableObj(anyobj); - - if (!obj) - return; - - virMutexUnlock(&obj->lock); + 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)"); + } }
[...]