As indicated in a couple of recent reviews, the new virObjectRWLockable object and API's I believe needed a few adjustments in order to make them better. So I present those thoughts in patch format to be hopefully dicussed and dissected. Essentially this series, will modify the newly added virObjectLockRead to return a status and to have that status managed. Since there were only 9 new callers, this was straightforward. None of the callers actually needed any sort of "divert logic" in order to jump around code that shouldn't do the Unlock now. Although that could be a problem if ever changing other more deeply nested type locks. That's someone else's headache though. Next, introduce and use virObjectLockWrite to be only for the new style RWLocks. This too will return status. Only the vir*Remove caller has a code path that I'm not happy with, but since these type locks have been processing along anyway without technically ensuring they have the lock before doing so, well no harm, no foul. Next, introduce and use virObjectRWUnlock. Rather than overloading the virObjectUnlock, let's just keep each caller doing it's one thing to be sure that the calling code is doing the "right thing". In this case, returning a failure status is just not going to work. We've gone too far down the path in order for a failure to cause some failure in the caller. We'll just treat it like virObjectUnlock and if the resource remains locked, well we'll find out relatively soon as the next time someone goes to read/write the list they'll be waiting for an unlock that will never happen because of some coding mistake. But that will save us from perhaps other dire consequences involving corruption. Finally, revert the virObjectLock and virObjectUnlock code to just manage virObjectLockable locks. No worse than what happened before the RWLock was introduced. Leaving the consumers to handle things as they would before - which probably ends up in some sort of awful state leading to memory corruption and a quick death as opposed to abort()'g in the void() function which has been deemed a don't do that type operation for libvirt. The last 2 patches actually have appeared before in various common object series postings, so they could be a v2 or 3 type logic, but I see them more related to this than that, so I present them here again. John Ferlan (7): util: Alter virObjectLockRead to return status util: Introduce and use virObjectLockWrite util: Only have virObjectLock handle virObjectLockable util: Introduce virObjectGetRWLockableObj util: Introduce and use virObjectRWUnlock util: Create common error path for invalid object util: Add safety net of checks to ensure valid object src/conf/virdomainobjlist.c | 81 +++++++++++++--------- src/libvirt_private.syms | 2 + src/util/virobject.c | 160 +++++++++++++++++++++++++++++++++----------- src/util/virobject.h | 10 ++- 4 files changed, 181 insertions(+), 72 deletions(-) -- 2.9.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
