Re: [libvirt] [PATCH 2/6] virObject: Introduce virObjectRecursiveLockableNew

On Fri, Feb 16, 2018 at 09:52:53 +0100, Michal Privoznik wrote: > On 02/16/2018 09:34 AM, Pavel Hrdina wrote: >> On Mon, Feb 12, 2018 at 01:16:28PM +0100, Michal Privoznik wrote: >>> On 02/12/2018 01:10 PM, Peter Krempa wrote: >>>> On Mon, Feb 12, 2018 at 11:52:49 +0100, Michal Privoznik wrote: >>>>> Sometimes we need the lock in virObjectLockable to be recursive. >>>>> Because of the nature of pthreads we don't need a special class >>>>> for that - the pthread_* APIs don't distinguish between normal >>>>> and recursive locks. >>>>> >>>>> Based-on-work-of: John Ferlan <jferlan(a)redhat.com&gt; >>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>>>> --- >>>>> src/libvirt_private.syms | 1 + >>>>> src/util/virobject.c | 22 +++++++++++++++++++--- >>>>> src/util/virobject.h | 4 ++++ >>>>> 3 files changed, 24 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >>>>> index 3b14d7d15..fcf378105 100644 >>>>> --- a/src/libvirt_private.syms >>>>> +++ b/src/libvirt_private.syms [...] > This can be viewed as rewrite of existing code, not completely new code. > >> >> I know that NWFilter code is complex and removing recursive locks is >> not an easy task, but for the long run I think it's worth it, it will >> make the code cleaner and easier to follow. > > Right, that the ideal goal. But as I said it's far from happening. I > think it was you who when trying to fix some issue in NWFilter drew call > graph in NWFilter driver and realized how complicated it is. That's why > I don't see it happening anywhere in near future. Also, if we really > have multiple entry points as Dan mentioned earlier can we really fix > this? I mean there are multiple locks that need to be acquired when > touching a virNWFilterObj. The advantage of reentrant mutex is that we > will not get a dead lock scenario if two functions fight over lock. > > Anyway, it's a pity that we are stuck on this patch while reworking the > vir*ObjList code. So and why can't we keep the NWfilter code as-is until the locking is sanitized first? It is working so I don't see a reason to try to rewrite it to objects if it is not trivially possible.