Patchwork [02/12] trace+libvirt: start trace processing thread in final child process

login
register
mail settings
Submitter Christian Borntraeger
Date July 15, 2013, 7:41 p.m.
Message ID <1373917279-15360-1-git-send-email-borntraeger@de.ibm.com>
Download mbox | patch
Permalink /patch/259176/
State New
Headers show

Comments

Christian Borntraeger - July 15, 2013, 7:41 p.m.
From: Michael Mueller <mimu@linux.vnet.ibm.com>

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.

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
[Christian Borntraeger: white space fixes]
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 vl.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
Stefan Hajnoczi - July 16, 2013, 3:05 a.m.
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

Patch

diff --git a/vl.c b/vl.c
index 25b8f2f..3c24108 100644
--- a/vl.c
+++ b/vl.c
@@ -3935,8 +3935,10 @@  int main(int argc, char **argv, char **envp)
         qemu_set_log(mask);
     }
 
-    if (!trace_backend_init(trace_events, trace_file)) {
-        exit(1);
+    if (!is_daemonized()) {
+        if (!trace_backend_init(trace_events, trace_file)) {
+            exit(1);
+        }
     }
 
     /* If no data_dir is specified then try to find it relative to the
@@ -4429,6 +4431,12 @@  int main(int argc, char **argv, char **envp)
 
     os_setup_post();
 
+    if (is_daemonized()) {
+        if (!trace_backend_init(trace_events, trace_file)) {
+            exit(1);
+        }
+    }
+
     main_loop();
     bdrv_close_all();
     pause_all_vcpus();