[libvirt] [PATCH] Fix misc thread locking bugs / bogus warnings

[Daniel P. Berrange]

On Tue, Sep 08, 2009 at 04:33:24PM +0200, Daniel Veillard wrote:
> On Wed, Sep 02, 2009 at 02:11:50PM +0100, Daniel P. Berrange wrote:
> > Fix all thread locking bugs reported by object-locking test
> > case.
> > 
> > NB, some of the driver locking is getting too coarse. Driver
> > mutexes really need to be turned into RW locks instead to
> > significantly increase concurrency.
> > 
> > * src/lxc_driver.c: Fix useof driver when unlocked in the methods
> >   lxcDomainGetInfo, lxcSetSchedulerParameters, and
> >   lxcGetSchedulerParameters
> > * src/opennebula/one_driver.c: Fix missing unlock in oneDomainUndefine.
> >   Fix use of driver when unlocked in oneDomainGetInfo,
> >   oneGetOSType, oneDomainShutdown
> > * src/qemu_driver.c: Fix use of driver when unlocked in
> >   qemudDomainSavem, qemuGetSchedulerType, qemuSetSchedulerParameters
> >   and qemuGetSchedulerParameters
> > * src/storage_driver.c: Re-work storagePoolCreate to avoid bogus
> >   lock checking warning. Re-work storageVolumeCreateXMLFrom to
> >   remove a potential NULL de-reference & avoid bogus lock check
> >   warnings
> > * src/test.c: Remove testDomainAssignDef since it break lock chekc
> >   warnings.
> > * tests/object-locking.ml: Add oneDriverLock, oneDriverUnlock
> >   and one_driver_t methods/types to allow lock checking on the
> >    OpenNebula drivers



> > diff --git a/src/lxc_driver.c b/src/lxc_driver.c
> > index bd0cf0e..0ec1e92 100644
> > --- a/src/lxc_driver.c
> > +++ b/src/lxc_driver.c
> > @@ -439,7 +439,6 @@ static int lxcDomainGetInfo(virDomainPtr dom,
> >  
> >      lxcDriverLock(driver);
> >      vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> > -    lxcDriverUnlock(driver);
> >  
> >      if (!vm) {
> >          lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
> > @@ -470,6 +469,7 @@ static int lxcDomainGetInfo(virDomainPtr dom,
> >      ret = 0;
> >  
> >  cleanup:
> > +    lxcDriverUnlock(driver);
> >      if (cgroup)
> >          virCgroupFree(&cgroup);
> >      if (vm)
> 
>   I'm just surprized by this series of fixes, I though we needed to drop
> locks when calling into errors routines like lxcError ?


No, we only need to drop the virConnect mutex - the driver/domain mutexes
can be held without trouble.  I really need to write some docs for this
mutex stuff ....


Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|