On Fri, Apr 08, 2011 at 09:57:16AM +0100, Daniel P. Berrange wrote:
> On Fri, Apr 08, 2011 at 11:07:44AM +0800, Hu Tao wrote:
> > On Thu, Apr 07, 2011 at 10:38:34AM +0100, Daniel P. Berrange wrote:
> > > On Wed, Apr 06, 2011 at 03:19:55PM +0800, Hu Tao wrote:
> > > > ---
> > > > src/qemu/qemu_domain.c | 30 ++-----------
> > > > src/qemu/qemu_migration.c | 2 -
> > > > src/qemu/qemu_monitor.c | 109 ++++++++++++++++++++++++--------------------
> > > > src/qemu/qemu_monitor.h | 4 +-
> > > > src/qemu/qemu_process.c | 32 +++++++-------
> > > > 5 files changed, 81 insertions(+), 96 deletions(-)
> > > >
> > > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > > > index 3a3c953..d11dc5f 100644
> > > > --- a/src/qemu/qemu_domain.c
> > > > +++ b/src/qemu/qemu_domain.c
> > > > @@ -560,9 +560,8 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
> > > > {
> > > > qemuDomainObjPrivatePtr priv = obj->privateData;
> > > >
> > > > - qemuMonitorLock(priv->mon);
> > > > - qemuMonitorRef(priv->mon);
> > > > virDomainObjUnlock(obj);
> > > > + qemuMonitorLock(priv->mon);
> > > > }
> > > >
> > > >
> > > > @@ -573,18 +572,9 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
> > > > void qemuDomainObjExitMonitor(virDomainObjPtr obj)
> > > > {
> > > > qemuDomainObjPrivatePtr priv = obj->privateData;
> > > > - int refs;
> > > > -
> > > > - refs = qemuMonitorUnref(priv->mon);
> > > > -
> > > > - if (refs > 0)
> > > > - qemuMonitorUnlock(priv->mon);
> > > >
> > > > + qemuMonitorUnlock(priv->mon);
> > > > virDomainObjLock(obj);
> > > > -
> > > > - if (refs == 0) {
> > > > - priv->mon = NULL;
> > > > - }
> > > > }
> > > >
> > > >
> > > > @@ -601,10 +591,8 @@ void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver,
> > > > {
> > > > qemuDomainObjPrivatePtr priv = obj->privateData;
> > > >
> > > > - qemuMonitorLock(priv->mon);
> > > > - qemuMonitorRef(priv->mon);
> > > > virDomainObjUnlock(obj);
> > > > - qemuDriverUnlock(driver);
> > > > + qemuMonitorLock(priv->mon);
> > > > }
> > > >
> > > >
> > > > @@ -617,19 +605,9 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver,
> > > > virDomainObjPtr obj)
> > > > {
> > > > qemuDomainObjPrivatePtr priv = obj->privateData;
> > > > - int refs;
> > > > -
> > > > - refs = qemuMonitorUnref(priv->mon);
> > > > -
> > > > - if (refs > 0)
> > > > - qemuMonitorUnlock(priv->mon);
> > > >
> > > > - qemuDriverLock(driver);
> > > > + qemuMonitorUnlock(priv->mon);
> > > > virDomainObjLock(obj);
> > > > -
> > > > - if (refs == 0) {
> > > > - priv->mon = NULL;
> > > > - }
> > > > }
> > >
> > > This means that the 'driver' lock is now whenever any QEMU
> > > monitor command is runing, which blocks the entire driver.
> >
> > qemuDomainObjEnterMonitorWithDriver is now called without holding qemu
> > driver lock.
>
> Well, this and the other changes in this series are completely
> altering all the locking rules used throughout the QEMU
> driver, with no clear explanation of what you are actually
> doing. Please read src/qemu/THREADS.txt and then provide an
> equivalent document explaining what you think the new rules
> should be, otherwise it is impossible to tell if these patches
> are at all threadsafe.
Sorry I didn't make this clear, will do in next version.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list