[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/4] Tracetool cleanup
From: |
Lluís Vilanova |
Subject: |
Re: [Qemu-devel] [PATCH 0/4] Tracetool cleanup |
Date: |
Thu, 20 Feb 2014 14:39:27 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Stefan Hajnoczi writes:
> On Wed, Feb 19, 2014 at 04:19:10PM +0100, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>>
>> > On Mon, Feb 17, 2014 at 08:36:19PM +0100, Lluís Vilanova wrote:
>> >> Minimizes the amount of backend code, making it simpler to add
>> >> new/different
>> >> backends.
>> >>
>> >> Also performs other cleanups all around.
>> >>
>> >> Signed-off-by: Lluís Vilanova <address@hidden>
>> >> ---
>> >>
>> >> Lluís Vilanova (4):
>> >> trace: [tracetool] Add method 'Event.api' to build event names
>> >> trace: [tracetool,trivial] Style changes
>> >> trace: [tracetool] Identify formats directly used by QEMU
>> >> trace: [tracetool] Minimize the amount of per-backend code
>>
>> > I think we stretched the concepts of backends and formats too far.
>> > There are formats that only work with one backend (like 'stap'). And
>> > there are backends that behave differently from all other backends.
>>
>> > As a result we're trying to abstract and make common a bunch of stuff
>> > that isn't really common. This problem existed before this patch
>> > series, but I feel we're going further down a direction that
>> > increasingly seems to be wrong.
>>
>> > It's simpler if we turn the design inside-out. Instead of making
>> > backends export all sorts of interfaces and flags, tracetool should just
>> > parse trace-events and hand the list over to the backend.
>>
>> > Let the backend do whatever it wants. The format option simply becomes
>> > an option telling the backend which type of output to generate
>> > (events.h, events.c, .stp, etc).
>>
>> > Common behavior should live in plain old Python modules/functions.
>>
>> > TL;DR moving to a library design would simplify and clean up more than
>> > trying to improve the current framework design
>>
>> > What do you think?
>>
>> This series moves into that direction; some of the formats are simply not
>> handled by backends. For example, the "stap", "events_c" and "events_h"
>> formats
>> have no backend-specific contents.
>>
>> Also, having common code for the "format", and then relying on backends for a
>> small piece of the contents simplifies later patches like the multi-backend
>> tracing.
>>
>> The thing here is that maybe we should change format to "file", since it
>> actually refers to a specific output file.
> You have a point. tracetool needs to output a particular file (e.g.
> generated-events.c, generated-tracers.h), which is kind of what a
> "format" has become.
> So the "format" is the primary piece of code that emits output. But if
> it needs to do something backend-specific (like generated-tracers.h)
> then it should call into backend modules.
Exactly.
> I am still concerned about the weird and wonderful interfaces that we're
> creating (like the "API" field in this patch series). They make it
> harder to understand the code and add new backends. Will think about
> this more when reviewing the next revision of this series.
Right. To be honest, the "API" sounded like a nice idea when I wrote it, but the
only thing it buys you is that non-API formats will only receive non-disabled
events. A really small benefit for adding more inter-module semantics. I will
probably remove it after rebasing the series.
Thanks,
Lluis
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
- [Qemu-devel] [PATCH 0/4] Tracetool cleanup, Lluís Vilanova, 2014/02/17
- [Qemu-devel] [PATCH 1/4] trace: [tracetool] Add method 'Event.api' to build event names, Lluís Vilanova, 2014/02/17
- [Qemu-devel] [PATCH 2/4] trace: [tracetool,trivial] Style changes, Lluís Vilanova, 2014/02/17
- [Qemu-devel] [PATCH 3/4] trace: [tracetool] Identify formats directly used by QEMU, Lluís Vilanova, 2014/02/17
- [Qemu-devel] [PATCH 4/4] trace: [tracetool] Minimize the amount of per-backend code, Lluís Vilanova, 2014/02/17
- Re: [Qemu-devel] [PATCH 0/4] Tracetool cleanup, Stefan Hajnoczi, 2014/02/19