On 08/03/2017 11:34 AM, Michal Privoznik wrote:
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
Thanks - I'll hold off on pushing though... I'm fine with using VIR_ERROR instead of VIR_WARN too... In any case, I'm away next week and wouldn't want to leave that kind of present for anyone to revert! Besides, I'm assuming there may be other opinions and there isn't a "rush" to get this in as nothing is broken yet... John