[libvirt] [PATCH 3/6] use virObject to manage reference-count of virDomainObj

[Daniel P. Berrange]

On Tue, Apr 12, 2011 at 06:33:21PM +0800, Hu Tao wrote:
> On Tue, Apr 12, 2011 at 06:22:12PM +0800, Wen Congyang wrote:
> > At 04/12/2011 01:51 PM, Hu Tao Write:
> > > Sorry, I unexpectedly deleted text body.
> > > 
> > > I changed the code like this:
> > > 
> > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > > index c2a1f9a..8aad0b3 100644
> > > --- a/src/qemu/qemu_domain.c
> > > +++ b/src/qemu/qemu_domain.c
> > > @@ -514,7 +514,10 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
> > >              else
> > >                  virReportSystemError(errno,
> > >                                       "%s", _("cannot acquire job mutex"));
> > > +            virDomainObjUnlock(obj);
> > >              qemuDriverLock(driver);
> > > +            virDomainObjLock(obj);
> > 
> > We lock qemu_driver and vm as the folling steps:
> > 1. lock qemu_driver
> > 2. lock vm
> > 
> > We try to lock qemu_driver while holding the vm's lock.
> > OOps, it will cause the libvirtd deadlock.(I can reproduce this bug by your script)
> > 
> > > +            virDomainObjUnref(obj);
> > 
> > We have unref it, so do not unref it again.
> 
> Didn't see there is already a call to virDomainObjUnref. the patch
> should be:
> 
> >From 712883d0151222a276f777f473d96aa23ad5d9d6 Mon Sep 17 00:00:00 2001
> From: Hu Tao <hutao at cn.fujitsu.com>
> Date: Tue, 12 Apr 2011 18:29:27 +0800
> Subject: [PATCH] qemu: fix a dead-lock problem
> 
> In qemuDomainObjBeginJobWithDriver, when virCondWaitUntil timeouts,
> the function tries to call qemuDriverLock with virDomainObj locked,
> this causes the dead-lock problem. This patch fixes this.
> ---
>  src/qemu/qemu_domain.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c2a1f9a..a947b4e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -506,15 +506,17 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
>  
>      while (priv->jobActive) {
>          if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) {
> -            /* Safe to ignore value since ref count was incremented above */
> -            ignore_value(virDomainObjUnref(obj));
>              if (errno == ETIMEDOUT)
>                  qemuReportError(VIR_ERR_OPERATION_TIMEOUT,
>                                  "%s", _("cannot acquire state change lock"));
>              else
>                  virReportSystemError(errno,
>                                       "%s", _("cannot acquire job mutex"));
> +            virDomainObjUnlock(obj);
>              qemuDriverLock(driver);
> +            virDomainObjLock(obj);
> +            /* Safe to ignore value since ref count was incremented above */
> +            ignore_value(virDomainObjUnref(obj));
>              return -1;
>          }
>      }
> -- 

ACK

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