qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establis


From: Lluís Vilanova
Subject: Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
Date: Tue, 20 Mar 2012 15:00:19 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.94 (gnu/linux)

Stefan Hajnoczi writes:

> 2012/3/13 Lluís Vilanova <address@hidden>:
>> Adds decorators to establish which backend and/or format each routine is 
>> meant
>> to process.
>> 
>> With this, tables enumerating format and backend routines can be eliminated 
>> and
>> part of the usage message can be computed in a more generic way.
>> 
>> Signed-off-by: Lluís Vilanova <address@hidden>
>> Signed-off-by: Harsh Prateek Bora <address@hidden>
>> ---
>>  Makefile.objs        |    6 -
>>  Makefile.target      |    3
>>  scripts/tracetool.py |  320 
>> ++++++++++++++++++++++++++++++++------------------
>>  3 files changed, 211 insertions(+), 118 deletions(-)

> I find the decorators are overkill and we miss the chance to use more
> straightforward approaches that Python supports.  The decorators build
> structures behind the scenes instead of using classes in an open coded
> way.  I think this makes it more difficult for people to modify the
> code - they will need to dig in to what exactly the decorators do -
> what they do is pretty similar to what you get from a class.

> I've tried out an alternative approach which works very nicely for
> formats.  For backends it's not a perfect fit because it creates
> instances when we don't really need them, but I think it's still an
> overall cleaner approach:

> https://github.com/stefanha/qemu/commit/3500eb43f80f3c9200107aa0ca19a1b57300ef8a

> What do you think?

I don't like it:

1) Format and backend tables must be manually filled.

2) The base Backend class has empty methods for each possible format.

3) There is no control on format/backend compatibility.

But I do like the idea of having a single per-backend class having methods for
each possible format.

The main reason for automatically establishing formats, backends and their
compatibility is that the instrumentation patches add some extra formats and
backends, which makes the process much more tedious if it's not automated.

Whether you use decorators or classes is not that important.

Auto-registration can be accomplished using metaclasses and special
per-format/backend "special" attributes the metaclasses will look for (e.g. NAME
to set the commandline-visible name of a format/backend.).

Format/backend compatibility can be established by introspecting into the
methods on each backend child class, matched against the NAMEs of the registered
formats.

But going this way does not sound to me like it will be much clearer than
decorators.

Do you agree? (I'll wait on solving this before fixing the rest of your concerns
in this series). Do you still prefer classes over decorators?


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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]