Re: [libvirt] [PATCH 2/3] virobject: Introduce virObjectRWLockable
[...]
/**
* 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)");
+ }
}
[...]