[libvirt] [PATCH v2 4/4] Push nwfilter update locking up to top level
Daniel P. Berrange
berrange at redhat.com
Wed Jan 29 13:50:28 UTC 2014
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 :|
More information about the libvir-list
mailing list