Re: [libvirt] [PATCH 01/10] util: Introduce thread queues

On Fri, May 22, 2015 at 14:28:05 +0200, Jiri Denemark wrote: > 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&gt; > > > > 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. Actually, thinking about this a bit more, I could implement it with per-domain condition. Any thread working on a domain would just register the domain's condition with the thread's pool in case the pool wants to wake up its threads. I think this would even be a bit nicer than the approach I implemented. Although, I'd go with a condition stored directly in virDomainObj rather than inside qemuDomainObjPrivate since this is something all driver could (and should) use if they want to use conditions.