Re: [Qemu-devel] [PATCH 4/4] trace: [tracetool] Minimize the amount of per-backend code

[Stefan Hajnoczi]

On Mon, Feb 17, 2014 at 08:36:41PM +0100, Lluís Vilanova wrote:
> Backends now only contain the essential backend-specific code, and most of 
> the work is moved to frontend code.
> 
> Signed-off-by: Lluís Vilanova <address@hidden>
> ---
>  scripts/simpletrace.py                |    6 --
>  scripts/tracetool/__init__.py         |   24 ++----
>  scripts/tracetool/backend/__init__.py |   72 ++++++++----------
>  scripts/tracetool/backend/dtrace.py   |   79 ++------------------
>  scripts/tracetool/backend/events.py   |   23 ------
>  scripts/tracetool/backend/ftrace.py   |   56 ++++++--------
>  scripts/tracetool/backend/simple.py   |  130 
> ++++++++++++++++-----------------
>  scripts/tracetool/backend/stderr.py   |   42 ++++-------
>  scripts/tracetool/backend/ust.py      |   44 +++++------
>  scripts/tracetool/format/__init__.py  |   46 ++++--------
>  scripts/tracetool/format/c.py         |    9 ++
>  scripts/tracetool/format/d.py         |   23 +++++-
>  scripts/tracetool/format/events_c.py  |   11 +--
>  scripts/tracetool/format/events_h.py  |   11 +--
>  scripts/tracetool/format/h.py         |   24 ++++--
>  scripts/tracetool/format/stap.py      |   37 +++++++++
>  trace/Makefile.objs                   |    4 +
>  17 files changed, 274 insertions(+), 367 deletions(-)
>  delete mode 100644 scripts/tracetool/backend/events.py

Please split the coding style changes into a separate commit (e.g.
removing trailing commas).

> diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
> index 8bbcb42..03d032e 100755
> --- a/scripts/simpletrace.py
> +++ b/scripts/simpletrace.py
> @@ -109,14 +109,10 @@ def process(events, log, analyzer):
>      if isinstance(log, str):
>          log = open(log, 'rb')
>  
> -    enabled_events = []
>      dropped_event = Event.build("Dropped_Event(uint64_t num_events_dropped)")
>      edict = {dropped_event_id: dropped_event}
>  
> -    for e in events:
> -        if 'disable' not in e.properties:
> -            enabled_events.append(e)
> -    for num, event in enumerate(enabled_events):
> +    for num, event in enumerate(events):
>          edict[num] = event
>  
>      def build_fn(analyzer, event):

This patch breaks existing simpletrace files.  Imagine you are
developing code and use simpletrace.  Then you decide to rebase onto the
latest qemu.git/master.  This patch changes the semantics - now
simpletrace.py outputs junk when analyzing the existing file.

We can't break the file format for convenience.  Either bump the file
format version number or don't change the semantics.