Re: [PATCH v2 0/8] Some virObjectRW* adjustments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/01/2017 02:05 AM, John Ferlan wrote:
> v1: https://www.redhat.com/archives/libvir-list/2017-July/msg01313.html
> 
> Differences from v1:
> 
>  * Use the names virObjectRWLockRead, virObjectRWLockWrite and
>    virObjectRWUnlock
> 
>  * Instead of an 'int' return, make the virObjectRWLock{Read|Write} be
>    void returns just like virObject{Lock|Unlock}
> 
>  * Separate out the magic number check for the virObjectIsClass consumers
>    into its own patch and describe the reasons for usage.
> 
>  * Apply the same magic number check to virObject{Ref|Unref} separately.
> 
> BTW: This looks and works eally nice with what I have for common objects...
> 
> John Ferlan (8):
>   util: Rename virObjectLockRead to virObjectRWLockRead
>   util: Introduce and use virObjectRWLockWrite
>   util: Only have virObjectLock handle virObjectLockable
>   util: Introduce virObjectGetRWLockableObj
>   util: Introduce and use virObjectRWUnlock
>   util: Create common error path for invalid object
>   util: Add magic number check for object validity
>   util: Add object checking for virObject{Ref|Unref}
> 
>  src/conf/virdomainobjlist.c |  62 ++++++++--------
>  src/libvirt_private.syms    |   4 +-
>  src/util/virobject.c        | 169 +++++++++++++++++++++++++++++++++-----------
>  src/util/virobject.h        |  10 ++-
>  4 files changed, 169 insertions(+), 76 deletions(-)
> 

Okay, I've ran some local tests and indeed no tool showed any
misbehaviour when my test binary was mutex-locking an RW lock or
rwlocking a mutex. While I still believe that us, reviewers have to be
careful around locks anyway, rename to virObjectRWLock* can help us
remember if we forget.

ACK series.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux