Re: [libvirt] [PATCH] Add a mutex to serialize updates to firewall

On 03/07/2014 09:15 AM, Daniel P. Berrange wrote: > For > > https://bugzilla.redhat.com/show_bug.cgi?id=1066801 > > The nwfilter conf update mutex previously serialized > updates to the internal data structures for firewall > rules, and updates to the firewall itself. The latter > was recently turned into a read/write lock, and filter > instantiation allowed to proceed in parallel. It was > believed that this was ok, since each filter is created > on a seperate iptables/ebtables chain. s/seperate/separate/ > > It turns out that there is a sutle lock ordering problem s/sutle/subtle/ > on virNWFilterObjPtr instances. __virNWFilterInstantiateFilter > will hold a lock on the virNWFilterObjPtr it is instantiating. > This in turn invokes virNWFilterInstantiate which then invokes > virNWFilterDetermineMissingVarsRec which then invokes > virNWFilterObjFindByName. This iterates over every single > virNWFilterObjPtr in the list, locking them and checking their > name. So if 2 or more threads try to instantiate a filter in > parallel, they'll all hold 1 lock at the top level in the > __virNWFilterInstantiateFilter method which will cause the > other thread to deadlock in virNWFilterObjFindByName. > > The fix is to add an exclusive mutex to serialize the > execution of __virNWFilterInstantiateFilter. > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; > --- > src/nwfilter/nwfilter_driver.c | 6 ++++-- > src/nwfilter/nwfilter_gentech_driver.c | 34 ++++++++++++++++++++++++++++++++-- > src/nwfilter/nwfilter_gentech_driver.h | 2 +- > 3 files changed, 37 insertions(+), 5 deletions(-) ACK with spelling fixes.