On Mon, Jan 27, 2014 at 04:53:53PM -0500, Stefan Berger wrote: > 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 When I say "virt driver lock" here I'm refering to the fact that the nwfilter callback drivers lock hook is basically just calling the virt driver 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@xxxxxxxxxx> > >--- > > 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(-) > Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list