Re: [Qemu-devel] [PATCH v2 3/3] Update simpletrace.py to support new v2 log format

[Stefan Hajnoczi]

On Thu, May 24, 2012 at 10:50 AM, Harsh Prateek Bora
<address@hidden> wrote:
> -            fields = [event[0], '%0.3f' % (delta_ns / 1000.0)]
> -            for i in xrange(1, len(event)):
> -                fields.append('%s=0x%x' % (event[i], rec[i + 1]))
> +            if rec[0] == dropped_event_id:
> +                fields = ['Dropped_Event', '%0.3f' % (delta_ns / 1000.0)]
> +                fields.append('%s=0x%x' % ("dropped_events", rec[2]))
> +            else:

Why the special case?  Can the dropped event be an event so the normal
code path pretty-prints it?

> +                fields = [event.name, '%0.3f' % (delta_ns / 1000.0)]
> +                if log_version == 0:

The global log_version variable splits the code.  A nice solution is
to encapsulate the format-specific behavior in a SimpleTraceV1 and
SimpleTraceV2 class - both provide the same interface and the code
that calls it doesn't care which exact version is used.

That said I'm happy for us to drop simpletrace v1 pretty-printing
entirely.  Users can only generate v2 files with this QEMU binary.  If
they want to view old files they also need the old trace-events file -
they might as well use the old simpletrace.py too!  Feel free to focus
on v2 only and drop the v1 support, you can add a nice error message
explaining that this file uses the v1 format and should be used with
the script that came with that QEMU binary.

> +                    for type, name in event.args:
> +                        fields.append('%s=0x%x' % (name, rec[i + 1]))
> +                        i += 1
> +                elif log_version == 2:
> +                    for type, name in event.args:
> +                        if is_string(type):
> +                             fields.append('%s=%s' % (name, rec[i + 1]))
> +                        else:
> +                             fields.append('%s=0x%x' % (name, rec[i + 1]))
> +                        i += 1
>             print ' '.join(fields)
>
>     run(Formatter())
> --
> 1.7.1.1
>
>