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 <vilan...@ac.upc.edu> >> >> --- >> >> >> >> 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