Re: [libvirt] [PATCH 5/9] util: Introduce virObjectLockableRecursiveNew
On 05/30/2017 09:19 AM, Daniel P. Berrange wrote:
On Tue, May 30, 2017 at 03:03:14PM +0200, Pavel Hrdina wrote:
> On Tue, May 30, 2017 at 07:38:17AM -0400, John Ferlan wrote:
>> Introduce a recursive option for the lockable object which calls
>> virMutexInitRecursive instead of virMutexInit.
>
> We should avoid using recursive lock because they are dangerous. I
> would rather cleanup the nwfilter code than introducing more stuff for
> recursive locks.
Last time I looked I came to the conclusion that it wasn't possible to
remove the recursive locking from nwfilter.
I agree about /not/ introducing new usage of recursive locking though.
So this is a chicken/egg problem.... Cannot create a new API to use
recursive locks and cannot remove the recursive lock without having
adjustments to nwfilter code that would seemingly require self locking
list objects to get hash table elements.
If the comments in src/nwfilter/nwfilter_gentech_driver.c (search on
XXX) could prove to be true, then it would be possible to not have a
recursive lock. Perhaps that loop could be rewritten in virnwfilterobj
to grab a reference to a hash table object while releasing the object
lock that's causing the deadlock. I haven't yet circled back to
determine whether this is possible yet, but since code in my branches is
at least this far, I can look again.
John