[...]
> /**
> * 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)");
> + }
> }
>
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
