Re: [libvirt] [PATCH 01/10] util: Introduce thread queues
On Fri, May 22, 2015 at 13:09:17 +0100, Daniel P. Berrange wrote:
On Fri, May 22, 2015 at 12:42:34AM +0200, Jiri Denemark wrote:
> Our usage of pthread conditions does not allow a single thread to wait
> for several events from different sources. This is because the condition
> is bound to the source of the event. We can invert the usage by giving
> each thread its own condition and providing APIs for registering this
> thread condition with several sources. Each of the sources can then
> signal the thread condition.
>
> Thread queues also support several threads to be registered with a
> single event source, which can either wakeup all waiting threads or just
> the first one.
>
> Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
I'm not really convinced this abstraction is neccessary / appropriate.
I can see what you mean about the problems with migration having access
to several different virCond's that it needs to wait on, but AFAICT,
all the condition variables are ultimately associated with a single
guest domain. So it seems the problem could have been solved much more
simply by just having a single virCond in the qemuDomainObjPrivate
struct.
Moving to a centralized single condition var per thread feels like it
is really breaking encapsulation of the APIs across the codebase.
Because we may want to wake up a thread which we know nothing about,
that is, we have no idea what job (if any) or even on which domain it is
doing. Currently, you can't gracefully kill libvirtd when it is, e.g.,
migrating a domain to another host. Apart from a bug which stops the
main event loop first (which I already solved in another branch of
mine), libvirtd would only stop once migration completes or is aborted
manually. You have to force kill it if you don't want to wait. Using a
thread local condition allows us to wake up any thread and ask it to
finish the job asap, possibly canceling it.
Jirka