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 <jfer...@redhat.com> > --- > 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list