Re: [Qemu-devel] [PATCH 2/3] Tracepoint, buffer & monitor framework

[Stefan Hajnoczi]

On Tue, May 25, 2010 at 7:20 PM, Prerna Saxena
<address@hidden> wrote:
>> Some added lines of code use tabs for indentation, 4 space indentation
>> should
>> be used.
>>
>> +struct tracepoint {
>> +       char *name;                     /* Tracepoint name */
>> +       uint8_t  trace_id;              /* numerical ID */
>>
>> Maximum 256 tracepoints in QEMU?  A limit of 65536 is less likely to
>> be an issue in the future.
>>
>
> No, this field describes the maximum tracepoints for a given hash queue.

I see now, thanks.

> I'll work on merging this circular buffer + monitor-based reader as a
> backend for your proposed tracer. Would it be a good idea to have two trace
> buffers -- when one is full, it gets written to disk ; while the second is
> used to log traces.

In a double-buffering approach there are finite resources.  There
needs to be a case for when the write-out buffer hasn't been written
yet and the active trace buffer becomes full.  I think in that case
the active buffer should start overwriting the oldest entry.

> I think the monitor interface for reading traces can be retained as is.
> Also, I'd implemented the monitor interface for enabling/disabling data
> logging for a given tracepoint (for a running guest) Not sure if this is
> supported in the set of patches you've posted ? It might be a good to have
> feature.

The "disable" trace event feature in my tracing backends patchset
allows statically disabling a trace event.  It doesn't support
enabling/disabling trace events at runtime, which is left up to the
backend.

The motivation for the "disable" attribute in the trace-events file is
to allow completely disabling a trace event without having to remove
it from trace-events *and* removing trace_*() calls in QEMU source
code.  It's a handy way of completely knocking out a trace event.

Thanks for your patches,
Stefan