Re: [libvirt] [PATCH v2 4/4] Push nwfilter update locking up to top level

The NWFilter code has as a deadlock race condition between the virNWFilter{Define,Undefine} APIs and starting of guest VMs due to mis-matched lock ordering. In the virNWFilter{Define,Undefine} codepaths the lock ordering is 1. nwfilter driver lock 2. virt driver lock 3. nwfilter update lock 4. domain object lock In the VM guest startup paths the lock ordering is 1. virt driver lock 2. domain object lock 3. nwfilter update lock As can be seen the domain object and nwfilter update locks are not acquired in a consistent order. The fix used is to push the nwfilter update lock upto the top level resulting in a lock ordering for virNWFilter{Define,Undefine} of 1. nwfilter driver lock 2. nwfilter update lock

3. virt driver lock 4. domain object lock and VM start using 1. nwfilter update lock 2. virt driver lock 3. domain object lock This has the effect of serializing VM startup once again, even if no nwfilters are applied to the guest. There is also the possibility of deadlock due to a call graph loop via virNWFilterInstantiate and virNWFilterInstantiateFilterLate. These two problems mean the lock must be turned into a read/write lock instead of a plain mutex at the same time. The lock is used to serialize changes to the "driver->nwfilters" hash, so the write lock only needs to be held by the define/undefine methods. All other methods can rely on a read lock which allows good concurrency. Signed-off-by: Daniel P. Berrange<berrange(a)redhat.com> --- src/conf/nwfilter_conf.c | 23 +++++++++++------------ src/conf/nwfilter_conf.h | 3 ++- src/libvirt_private.syms | 3 ++- src/lxc/lxc_driver.c | 6 ++++++ src/nwfilter/nwfilter_driver.c | 11 +++++++---- src/nwfilter/nwfilter_gentech_driver.c | 4 +--- src/qemu/qemu_driver.c | 6 ++++++ src/uml/uml_driver.c | 4 ++++ 8 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 112e8cb..80030c8 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -285,12 +285,15 @@ nwfilterStateReload(void) virNWFilterLearnThreadsTerminate(true); nwfilterDriverLock(driverState); + virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); +

virNWFilterLoadAllConfigs(&driverState->nwfilters, driverState->configDir); virNWFilterCallbackDriversUnlock(); + virNWFilterUnlockFilterUpdates(); nwfilterDriverUnlock(driverState); virNWFilterInstFiltersOnAllVMs();