Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats

[Lluís Vilanova]

Stefan Hajnoczi writes:
[...]
>> Maybe this would work nice for everybody:
>> 
>> tracetool.py                  # main program (just parse cmdline opts and 
>> call tracetool module)
>> tracetool/__init__.py         # common boilerplate code (e.g., event parsing 
>> and call dispatching)
>> tracetool/format/__init__.py  # format auto-registration/lookup code
>> tracetool/format/h.py         # code for the 'h' format
>>                              # __doc__           [mandatory, format 
>> description]
>>                              # def begin(events) [optional]
>>                              # def end(events)   [optional]
>> tracetool/backend/__init__.py # backend auto-registration/lookup code
>> tracetool/backend/simple.py   # code for the 'simple' backend
>>                              # __doc__            [mandatory, backend 
>> description]
>>                              # PUBLIC = [True|False] [optional,
>>                              #                        defaults to False,
>>                              #                        indicates it's listed 
>> on --list-backends]
>>                              # def format(events) [optional,
>>                              #                     backend-specific code for 
>> given format,
>>                              #                     implicitly indicates 
>> format compatibility]

> Let's call this function backend.generate(events) instead of "format"
> since we already use that term and it's a Python builtin too.  This is
> a code generation function.

Maybe I wasn't clear. I meant that for tracetool/backend/simple.py to work with
the format 'h', it must have the following routine:

    def h (events):
        ...

That is, there must exist a routine with the same name of the format (with '-'
replaced with '_', when necessary) for the backend to be compatible with that
format.


>> Note that new formats will need to add new format routines in
>> 'tracetool/backend/nop.py' to accomodate whatever action has to be taken on
>> disabled events.

> I think it's more convenient to drop the nop backend and introduce a
> nop() code generation function for each format.

> The .h format would nop() function would emit a static inline empty C
> function that does nothing.

> The other formats could probably do nothing in nop().

Sounds good:

tracetool/format/f.py:

    def begin (events):
        ...

    def nop (events):
        ...

    def end (events):
        ...

tracetool/__init__.py:

    def generate (events, format, backend, force_nop = False):
        if force_nop:
            events = [ e.properties.add("disabled") for e in events ]

        tracetool.format.begin(events)

        tracetool.backend.<backend>.<format>(non-disabled events events)
        tracetool.format.<format>.nop(disabled events events)

        tracetool.format.end(events)


>>> As the next step with this patch series we could drop this patch.  It
>>> doesn't make an external difference.  Then we can continue to discuss
>>> how to best handle backends as a separate patch.
>> 
>> WDYT of the organization above? Given the current code it's pretty simple to
>> split it into different modules. If everybody agrees on the above, I can 
>> make it
>> happen.

> I like the organization you have proposed.

> In order to avoid rebasing and porting too many fixes from tracetool
> to tracetool.py I suggest you resend this series without the
> format/backend consolidation code.  I can merge this series quickly
> and we can handle the format/backend consolidation code in a separate
> patch series.

Sure, I'll try to do it later today or tomorrow otherwise.


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