Re: [libvirt] [PATCH] util: Remove logging handlers in virExec

On Tue, Jan 12, 2010 at 03:26:26PM -0500, Cole Robinson wrote: > This allows debug statements and raised errors in hook functions to > actually be logged somewhere (stderr). Users can enable debugging in the > daemon and now see more info in /var/log/libvirt/... > > Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; > --- > src/util/util.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/src/util/util.c b/src/util/util.c > index 44a4b2f..23d781d 100644 > --- a/src/util/util.c > +++ b/src/util/util.c > @@ -334,6 +334,7 @@ __virExec(virConnectPtr conn, > int pipeerr[2] = {-1,-1}; > int childout = -1; > int childerr = -1; > + int logprio; > sigset_t oldmask, newmask; > struct sigaction sig_action; > > @@ -452,6 +453,11 @@ __virExec(virConnectPtr conn, > of being seen / logged */ > virSetErrorFunc(NULL, NULL); > > + /* Make sure any hook logging is sent to stderr */ > + logprio = virLogGetDefaultPriority(); > + virLogReset(); > + virLogSetDefaultPriority(logprio); > + This patch turns out to cause a deadlock in libvirtd. The problem is that fork() preserves the state of any mutexes which are locked. So if some thread in libvirtd is currently executing a logging call, while another thread calls virExec(), that other thread no longer exists in the child, but its lock is never released. So when the child then does virLogReset() it deadlocks. I'm actuall surprised we've not hit this problem before, since any call to virRaiseError in the child will also eventually run a logging function which will in turn acquire a lock. The only way I see to address this, is for the parent process to call virLogLock(), immediately before fork(), and then virLogUnlock() afterwards in both parent & child. This will ensure that no other thread can be holding the lock across fork(). https://bugzilla.redhat.com/show_bug.cgi?id=561066