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

Michal Privoznik mprivozn at redhat.com
Fri Feb 16 09:47:18 UTC 2018


On 02/16/2018 10:08 AM, Peter Krempa wrote:
> 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 at redhat.com>
>>>>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>>>>> ---
>>>>>>  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.
> 

Well, I find it somewhat disappointing. The patches that John and I
proposed make things better. But because they don't make it 100% better
they are NACKed. But I can live with having two different
implementations for vir*ObjList if that's what we want. Or if it's
better than having either John's or mines patches merged. I think
otherwise though.

Michal




More information about the libvir-list mailing list