On Mon, Oct 03, 2011 at 10:41:17PM +0200, Guido Günther wrote: > On Tue, Aug 16, 2011 at 02:24:53PM +0200, Guido Günther wrote: > > Hi Daniel, > > On Sat, Aug 13, 2011 at 08:57:45PM -0700, Daniel P. Berrange wrote: > > > On Fri, Aug 12, 2011 at 11:54:28PM +0200, Guido Günther wrote: > > [..snip..] > > > In the default libvirt event loop, the 'ff' callback is always invoked > > > from a "clean" stack in the event loop, so you never have this problem > > > with re-entrancy. > > > > > > > Working around this by removing the locks from > > > > virNetSocketRemoveIOCallback leads to another deadlock: > > > > > > Yeah this is not a viable approach. > > Sure. This was only to see what else fails. > > > > > > > > > > I didn't see a simple way to fix this but would welcome any suggestions. > > > > > > IMHO we just have to document that event loop implementations > > > should make sure that the 'ff' callbacks are always invoked > > > from a clean stack. In the case of virt-viewer, this means > > > changing it to register a g_idle callback function to invoke > > > the 'ff' callback. > > > > Patch for virt-viewer attached. I'll come up with a doc patch for > > libvirt once I have a bit more time. > > As promised here's the patch for libvirt. O.k. to apply? > Cheers, > -- Guido > From c2e2be4e377871ace12bb7adfde130e9b8779ab5 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Guido=20G=C3=BCnther?= <agx@xxxxxxxxxxx> > Date: Mon, 3 Oct 2011 22:24:13 +0200 > Subject: [PATCH] Document that ff callbacks need to be invoked from a clean > stack. > > --- > include/libvirt/libvirt.h.in | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in > index a3c581d..bd7a0f7 100644 > --- a/include/libvirt/libvirt.h.in > +++ b/include/libvirt/libvirt.h.in > @@ -2243,13 +2243,15 @@ typedef void (*virEventHandleCallback)(int watch, int fd, int events, void *opaq > * @opaque: user data to pass to the callback > * @ff: the callback invoked to free opaque data blob > * > - * Part of the EventImpl, this callback Adds a file handle callback to > + * Part of the EventImpl, this callback adds a file handle callback to > * listen for specific events. The same file handle can be registered > * multiple times provided the requested event sets are non-overlapping > * > * If the opaque user data requires free'ing when the handle > * is unregistered, then a 2nd callback can be supplied for > - * this purpose. > + * this purpose. This callback needs to be invoked from a clean stack. > + * If 'ff' callbacks are invoked directly from the virEventRemoveHandleFunc > + * they will likely deadlock in libvirt. > * > * Returns a handle watch number to be used for updating > * and unregistering for events ACK 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list