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

[Daniel P. Berrange]

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 at 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(-)
>

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 :|