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

[libvirt] [PATCH] Document that ff callbacks need to be invoked from a clean stack



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 sigxcpu org>
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
-- 
1.7.6.3


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