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

On 01/27/2014 12:18 PM, Daniel P. Berrange wrote: >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 Insert: 3. nwfilter callback drivers lock

This is then used in this order also by nwfilterStateReload > 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(-)