On Fri, Jul 28, 2017 at 12:38:57PM -0400, John Ferlan wrote:
> Now that virObjectLockWrite exists to handle the virObjectRWLockable
> objects, let's restore virObjectLock to only handle virObjectLockable
> type locks. There still exists the possibility that the input @anyobj
> isn't a valid object and the resource isn't truly locked, but that
> also exists before commit id '77f4593b'.
>
> This also restores some logic that commit id '77f4593b' removed
> with respect to a common code path that commit id '10c2bb2b' had
> introduced as virObjectGetLockableObj.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/util/virobject.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index c48f88c..f9047a1 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -367,13 +367,28 @@ virObjectRef(void *anyobj)
> }
>
>
> +static virObjectLockablePtr
> +virObjectGetLockableObj(void *anyobj)
> +{
> + virObjectPtr obj;
> +
> + if (virObjectIsClass(anyobj, virObjectLockableClass))
> + return anyobj;
> +
> + obj = anyobj;
> + VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
> + anyobj, obj ? obj->klass->name : "(unknown)");
> +
> + return NULL;
> +}
> +
> +
> /**
> * virObjectLock:
> * @anyobj: any instance of virObjectLockable or virObjectRWLockable
> *
> * 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.
> + * virObjectUnlock.
> *
> * The caller is expected to have acquired a reference
> * on the object before locking it (eg virObjectRef).
> @@ -383,18 +398,12 @@ virObjectRef(void *anyobj)
> void
> virObjectLock(void *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)");
> - }
> + virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
> +
> + if (!obj)
> + return;
> +
> + virMutexLock(&obj->lock);
Again I'd prefer to see
virObjectLockablePtr obj = anyobj;
virMutexLock(&obj->lock);
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
