Re: [Qemu-devel] [PATCH 02/12] trace+libvirt: start trace processing thread in final child process

[Stefan Hajnoczi]

On Tue, Jul 16, 2013 at 02:17:28PM +0200, Michael Mueller wrote:
> On Tue, 16 Jul 2013 11:05:11 +0800
> Stefan Hajnoczi <address@hidden> wrote:
> 
> > On Mon, Jul 15, 2013 at 09:41:19PM +0200, Christian Borntraeger wrote:
> > > When running with trace backend e.g. "simple" the writer thread
> > > needs to be implemented in the same process context as the trace
> > > points that will be processed. Under libvirtd control, qemu gets
> > > first started in daemonized mode to privide its capabilities.
> > > Creating the writer thread in the initial process context then
> > > leads to a dead lock because the thread gets termined together with
> > > the initial parent. (-daemonize) This results in stale qemu
> > > processes. Fix this by deferring trace initialization.
> > 
> > I don't think this works since trace events will fill up trace_buf[]
> > and eventually invoke flush_trace_file().
> > 
> > At that point we use trace_available_cond and trace_empty_cond, which
> > may be NULL in Glib <2.31.0.
> > 
> > Perhaps this can be made safe by checking trace_writeout_enabled.  It
> > will be false before the backend has been initialized.
> > 
> > Stefan
> > 
> 
> You mean something like this. I'll give it a try:
> 
> ---
>  trace/simple.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> --- a/trace/simple.c
> +++ b/trace/simple.c
> @@ -53,7 +53,7 @@ static GCond *trace_empty_cond;
>  #endif
>  
>  static bool trace_available;
> -static bool trace_writeout_enabled;
> +static bool trace_writeout_enabled = false;

static bool is automatically initialized to false.

>  enum {
>      TRACE_BUF_LEN = 4096 * 64,
> @@ -427,5 +427,6 @@ bool trace_backend_init(const char *even
>      atexit(st_flush_trace_buffer);
>      trace_backend_init_events(events);
>      st_set_trace_file(file);
> +    trace_writeout_enabled = false;

I was thinking along the lines of trace_record_finish() not calling
flush_trace_file() if trace_writeout_enabled is false.

Stefan