Re: [libvirt] [PATCHv4] qemu: allow blkstat/blkinfo calls during migration

On 05/16/2011 07:30 PM, Eric Blake wrote: > + while (priv->jobSignals& QEMU_JOB_SIGNAL_BLKSTAT) > + ignore_value(virCondWait(&priv->signalCond,&vm->lock)); > + > + priv->jobSignalsData.statDevName = disk->info.alias; > + priv->jobSignalsData.blockStat = stats; > + priv->jobSignalsData.statRetCode = -1; > + priv->jobSignals |= QEMU_JOB_SIGNAL_BLKSTAT; > + > + while (priv->jobSignals& QEMU_JOB_SIGNAL_BLKSTAT) > + ignore_value(virCondWait(&priv->signalCond,&vm->lock)); I'm not sure that the original problem with jobSignals is fixed (the one that you solved by splitting the retcode field). A similar race can happen with two threads, both requesting blkstat. Thread 1 starts a migration. Thread 2 queries BlockStats, waits for the condition variable and priv->jobSignals to be 0. Thread 2 sets priv->jobSignals to non-zero, then waits for condition variable and priv->jobSignals to clear BLKSTATS. Thread 1 processes thread 2's query, and clears priv->jobSignals Thread 3 sees priv->jobSignals == 0 and queries BlockStats. Thread 1 processes thread 3's query, and clears priv->jobSignals Thread 2 gets condition lock, but sees the answer intended for thread 3.

I think the code should use two condition variables. signalCond is broadcast when you _can start_ a job, resultCond is broadcast when a job has finished.