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

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



On Mon, May 25, 2015 at 04:04:37PM +0200, Jiri Denemark wrote:
> 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.

That sounds a bit more appealing to me.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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