----- Original Message ----- > From: "Eric Blake" <eblake@xxxxxxxxxx> > To: "Federico Simoncelli" <fsimonce@xxxxxxxxxx> > Cc: libvir-list@xxxxxxxxxx > Sent: Friday, May 13, 2011 1:38:45 AM > Subject: Re: [PATCH] qemu: allow blkstat/blkinfo calls during migration > On 05/11/2011 07:26 AM, Federico Simoncelli wrote: > > > > + if ((priv->jobActive == QEMU_JOB_MIGRATION_OUT) > > + || (priv->jobActive == QEMU_JOB_SAVE)) { > > + virDomainObjRef(vm); > > + while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT) > > + ignore_value(virCondWait(&priv->signalCond, &vm->lock)); > > Hmm, should we mark priv->jobSignals as volatile in the header, to > ensure the compiler won't optimize this into an infinite loop? Not sure. http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/ > > + if (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT) { > > + qemuDomainObjEnterMonitorWithDriver(driver, vm); > > + rc = qemuMonitorGetBlockStatsInfo(priv->mon, > > I'm still wondering if we need to hold the signalLock condition during > the duration where we drop driver lock to call out to the monitor. I > can't convince myself that we need to, but I also can't convince > myself > that your code is safe without it (I guess it goes to show that I > haven't done much programming on condition variables in any prior job > - > they're cool tools, but hard to wrap your head around when first > learning them). There is no signalLock, on the other hand the signalCond is used to wake up a thread (using virCondSignal/virCondBroadcast) that is currently sleeping on virCondWait (and not holding vm->lock). -- Federico -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list