[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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.

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.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]