| Message ID | 20130305103357.GD1938@stefanha-thinkpad.redhat.com |
|---|---|
| State | New |
| Headers | show |
Stefan Hajnoczi writes: > On Mon, Mar 04, 2013 at 10:01:43PM +0100, Lluís Vilanova wrote: >> Stefan Hajnoczi writes: >> >> > On Fri, Mar 01, 2013 at 04:29:35PM +0100, Lluís Vilanova wrote: >> >> Provides a generic event state description structure (TraceEvent) and a more >> >> detailed event control and query interface. >> >> >> >> This is achieved by creating a new "non-public" tracing backend (i.e., not >> >> selectable by the user at configure time) that will generate the appropriate >> >> event description information. >> >> >> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> >> >> --- >> >> >> >> Changes in v11: >> >> >> >> * Rebase on a4bcea3 from master. >> >> > Hi Lluís, >> > Thanks for rebasing. Unfortunately I still hit the hang on shutdown >> > with the simple trace backend. >> >> > Steps to reproduce: >> > $ ./configure --target-list=x86_64-softmmu --enable-trace-backend=simple >> > $ make >> > $ cat my-events >> > bdrv_open_common >> > $ x86_64-softmmu/qemu-system-x86_64 \ >> > -enable-kvm -m 1024 \ >> > -trace events=my-events \ >> > -drive if=virtio,cache=none,file=test.img >> > Ctrl-Alt-2 >> > (qemu) quit >> > ...hang... >> >> > Please let me know if you are able to reproduce it. qemu.git/master >> > does not behave this way. >> >> I've been unable to reproduce it with v11, but was able to using v10 (although >> only a couple of times). > Found it! > The problem is that trace_record_start() is writing a 4-byte event field > instead of an 8-byte event field. TraceEventID used to be typedef > uint64_t. In your series TraceEventID becomes an enum and breaks > trace_record_start(). > Try this: > diff --git a/trace/simple.c b/trace/simple.c > index 5bb905c..1e3f691 100644 > --- a/trace/simple.c > +++ b/trace/simple.c > @@ -218,6 +218,7 @@ int trace_record_start(TraceBufferRecord *rec, TraceEv > { > unsigned int idx, rec_off, old_idx, new_idx; > uint32_t rec_len = sizeof(TraceRecord) + datasize; > + uint64_t event_u64 = event; > uint64_t timestamp_ns = get_clock(); > do { > @@ -235,7 +236,7 @@ int trace_record_start(TraceBufferRecord *rec, TraceEv > idx = old_idx % TRACE_BUF_LEN; > rec_off = idx; > - rec_off = write_to_buffer(rec_off, &event, sizeof(event)); > + rec_off = write_to_buffer(rec_off, &event_u64, sizeof(event_u64)); > rec_off = write_to_buffer(rec_off, ×tamp_ns, sizeof(timestamp_ns > rec_off = write_to_buffer(rec_off, &rec_len, sizeof(rec_len)); Works for me, thx! I'll send v12 in a couple of minutes. Lluis
diff --git a/trace/simple.c b/trace/simple.c index 5bb905c..1e3f691 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -218,6 +218,7 @@ int trace_record_start(TraceBufferRecord *rec, TraceEv { unsigned int idx, rec_off, old_idx, new_idx; uint32_t rec_len = sizeof(TraceRecord) + datasize; + uint64_t event_u64 = event; uint64_t timestamp_ns = get_clock(); do { @@ -235,7 +236,7 @@ int trace_record_start(TraceBufferRecord *rec, TraceEv idx = old_idx % TRACE_BUF_LEN; rec_off = idx; - rec_off = write_to_buffer(rec_off, &event, sizeof(event)); + rec_off = write_to_buffer(rec_off, &event_u64, sizeof(event_u64)); rec_off = write_to_buffer(rec_off, ×tamp_ns, sizeof(timestamp_ns rec_off = write_to_buffer(rec_off, &rec_len, sizeof(rec_len));