Re: [Qemu-devel] [RFC PATCH v3 1/3] Converting tracetool.sh to tracetool.py

[Stefan Hajnoczi]

On Wed, Jan 11, 2012 at 8:46 AM, Harsh Bora <address@hidden> wrote:
> On 01/11/2012 12:08 PM, Harsh Bora wrote:
>>
>> On 01/11/2012 04:21 AM, Lluís Vilanova wrote:
>>>
>>> Harsh Prateek Bora writes:
>>>
>>>> Signed-off-by: Harsh Prateek Bora<address@hidden>
>>>> ---
>>>> Makefile.objs | 6 +-
>>>> Makefile.target | 10 +-
>>>> configure | 7 +-
>>>> scripts/tracetool | 643
>>>> --------------------------------------------------
>>>> scripts/tracetool.py | 585 +++++++++++++++++++++++++++++++++++++++++++++
>>>> 5 files changed, 597 insertions(+), 654 deletions(-)
>>>> delete mode 100755 scripts/tracetool
>>>> create mode 100755 scripts/tracetool.py
>>>
>>>
>
> [...]
>
>>>> +def main():
>>>> + global backend, output, binary, targettype, targetarch, probeprefix
>>>> + supported_backends = ["simple", "nop", "stderr", "dtrace", "ust"]
>>>> + short_options = "hcd"
>>>> + long_options = ["stap", "backend=", "binary=", "target-arch=",
>>>> "target-type=", "probe-prefix=", "list-backends", "check-backend"]
>>>> + try:
>>>> + opts, args = getopt.getopt(sys.argv[1:], short_options, long_options)
>>>> + except getopt.GetoptError, err:
>>>> + # print help information and exit:
>>>> + print str(err) # will print something like "option -a not recognized"
>>>> + usage()
>>>> + sys.exit(2)
>>>> + for opt, arg in opts:
>>>> + if opt == '-h':
>>>> + output = 'h'
>>>> + elif opt == '-c':
>>>> + output = 'c'
>>>> + elif opt == '-d':
>>>> + output = 'd'
>>>> + elif opt == '--stap':
>>>> + output = 'stap'
>>>> + elif opt == '--backend':
>>>> + backend = arg
>>>> + elif opt == '--binary':
>>>> + binary = arg
>>>> + elif opt == '--target-arch':
>>>> + targetarch = arg
>>>> + elif opt == '--target-type':
>>>> + targettype = arg
>>>> + elif opt == '--probe-prefix':
>>>> + probeprefix = arg
>>>> + elif opt == '--list-backends':
>>>> + print 'simple, nop, stderr, dtrace'
>>>> + sys.exit(0)
>>>> + elif opt == "--check-backend":
>>>> + if any(backend in s for s in supported_backends):
>>>> + sys.exit(0)
>>>> + else:
>>>> + sys.exit(1)
>>>> + else:
>>>> + #assert False, "unhandled option"
>>>> + print "unhandled option: ", opt
>>>> + usage()
>>>> +
>>>> + if backend == "" or output == "":
>>>> + usage()
>>>> + sys.exit(0)
>>>> +
>>>> + events = read_events(sys.stdin)
>>>> + trace_gen[output]['begin']()
>>>> + converters[backend][output](events)
>>>
>>>
>>> This should use the "disable" property to establish whether to use
>>> output or
>>> "nop".
>
>
> Will it be better to again break converters to their begin, process_line,
> end counterparts and call respective converters accordingly for
> enabled/disabled events, or let all the backends handle disabled events on
> their own ?
>
> Stefan ?

I suggest adding a Event.properties field.  For most events it will be
[] but for disabled events it will be ['disable'].  In the future we
could support new keywords.

Then we can filter out the disabled events in main():

disabled_events, enabled_events = [], []
for e in read_events(sys.stdin):
    if 'disable' in e.properties:
        disabled_events.append(e)
    else:
        enabled_events.append(e)

trace_gen[output]['begin']()
converters['nop'][output](disabled_events)
converters[backend][output](enabled_events)
trace_gen[output]['end']()