[...]
>> }
>>
>>
>> +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