Re: [libvirt] [PATCH v2 2/4] virDomainObjSignal: drop this function
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)