[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 0/7] trace: Generic event state description
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v11 0/7] trace: Generic event state description |
Date: |
Tue, 5 Mar 2013 11:33:57 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> >> ---
> >>
> >> 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));
- [Qemu-devel] [PATCH v11 1/7] trace: [tracetool] Explicitly identify public backends, (continued)
- [Qemu-devel] [PATCH v11 1/7] trace: [tracetool] Explicitly identify public backends, Lluís Vilanova, 2013/03/01
- [Qemu-devel] [PATCH v11 2/7] trace: Provide a generic tracing event descriptor, Lluís Vilanova, 2013/03/01
- [Qemu-devel] [PATCH v11 3/7] trace: Provide a detailed event control interface, Lluís Vilanova, 2013/03/01
- [Qemu-devel] [PATCH v11 4/7] trace: [monitor] Use new event control interface, Lluís Vilanova, 2013/03/01
- [Qemu-devel] [PATCH v11 5/7] trace: [default] Port to generic event information and new control interface, Lluís Vilanova, 2013/03/01
- [Qemu-devel] [PATCH v11 6/7] trace: [simple] Port to generic event information and new control interface, Lluís Vilanova, 2013/03/01
- [Qemu-devel] [PATCH v11 7/7] trace: [stderr] Port to generic event information and new control interface, Lluís Vilanova, 2013/03/01
- Re: [Qemu-devel] [PATCH v11 0/7] trace: Generic event state description, Stefan Hajnoczi, 2013/03/04