On Wed, Jul 01, 2015 at 17:29:09 -0400, John Ferlan wrote:
On 06/29/2015 11:17 AM, Pavel Hrdina wrote:
There are multiple consumers for the domain condition and we should always wake them all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 7 ------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - src/qemu/qemu_process.c | 4 ++-- 4 files changed, 2 insertions(+), 11 deletions(-)
Hmmm... This is perhaps quite a difference as you're now unblocking all threads for HandleBlockJob and HandleSpiceMigrated.
Using virCondBroadcast means all threads waiting on the condition will be unblocked as opposed to unblocking 'at least one of the threads' when using virCondSignal - for some reason that just doesn't feel right.
Well, since a later patch in this series adds a code path that may signal a condition asyncrhonously to any known combination of threads and in the future we might add more than one waiting thread the change is desired.
I can certainly understand unblocking everything waiting for HandleTrayChange since it's a single physical block and perhaps HandleSpiceMigrate since that seems to be a single SPICE server transferring things (such as perhaps ports). Whereas, it just feels like there could be multiple causes for BlockJob wanting synchronized access.
Every single instance of waiting needs to check a extra variable to decide whether it should be unblocked, since pthreads may cause spurious wakeup in some cases so the existing code is robust to that. Peter