[...]
>>> }
>>>
>>>
>>> +static void
>>> +virNWFilterObjPromoteToWrite(virNWFilterObjPtr obj)
>>> +{
>>> + virObjectRWUnlock(obj);
>>> + virObjectRWLockWrite(obj);
>>> +}
>>
>> How can this not deadlock? This will work only if @obj is locked exactly
>> once. But since we allow recursive locks any lock() that happens in the
>> 2nd level must deadlock with this. On the other hand, there's no such
>> case in the code currently. But that doesn't stop anybody from calling
>> PromoteWrite() without understanding its limitations.
>>
>> Maybe the fact that we need recursive lock for NWFilterObj means it's
>> not a good candidate for virObjectRWLocable? It is still a good
>> candidate for virObject though.
>>
>> Or if you want to spend extra time on this, maybe introducing
>> virObjectRecursiveLockable may be worth it (terrible name, I know).
>
> I can't remember exactly call trace scenarios that meant we required
> recursive locking. I vaguely recall is was related to fact that we
> have an alternative entry point into the nwfilter code that's triggered
> by the virt drivers. I'm thus vaguely hoping that when we split nwfilter
> off into a separate daemon from virt driver, the need for recursive
> lockingn would magically disappear. I could be completely wrong though :-)
>
>
There's multiple recursive locks. I tried to deal only with the
NWFilterObj locks.
There some "magical" entry points with domain start/stop via
nwfilterInstantiateFilter and nwfilterTeardownFilter.... There's also
interactions via libvirtd start/stop and of course whatever magical
entry points for firewalld. Lot's of chances for issues when the
virNWFilter{ReadLock|Unlock}FilterUpdates calls are made. Finally
there's an @updateMutex in nwfilter_gentech_driver.c which truly brings
great joy and happiness to debugging lock issues.
I have this piece of paper which I tried to keep track of various locks
and paths - suffice to say it got messy very quickly.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
