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

[Daniel P. Berrange]

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 at redhat.com>
> ---
>  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

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|