| Message ID | 20120313200332.24179.78152.stgit@ginnungagap.bsc.es |
|---|---|
| State | New |
| Headers | show |
2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>: > 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 <vilanova@ac.upc.edu> > Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> > --- > 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? Stefan
Stefan Hajnoczi writes: > 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>: >> 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 <vilanova@ac.upc.edu> >> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> >> --- >> 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
2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>: > Stefan Hajnoczi writes: > >> 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>: >>> 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 <vilanova@ac.upc.edu> >>> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> >>> --- >>> 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? For formats the Format class plus a dict approach is much nicer than decorators. The code is short and easy to understand. For backends it becomes a little tougher and I was wondering whether splitting the code into modules would buy us something. The fact that you've added '####...' section delimeters shows that tracetool.py is growing to long and we're putting too many concepts into one file. If each backend is a module then we have a natural way of containing backend-specific code. Perhaps the module can register itself when tracetool.py wildcard imports them all. I think this will approach the level of magic that decorators involve but with the bonus that we begin to split the code instead of growing a blob. What do you think of putting each backend in its own module? Do you have a link to your latest code that adds formats/backends? I'd like to take a quick look to make sure I understand where you're going with this - I've only been thinking of the current set of formats/backends. 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. Stefan
Stefan Hajnoczi writes: > 2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>: >> Stefan Hajnoczi writes: >> >>> 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>: >>>> 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 <vilanova@ac.upc.edu> >>>> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> >>>> --- >>>> 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? > For formats the Format class plus a dict approach is much nicer than > decorators. The code is short and easy to understand. Well, I prefer to automate this kind of things so that new features get automatically registered and the changes are localized; but it really doesn't matter that much :) > For backends it becomes a little tougher and I was wondering whether > splitting the code into modules would buy us something. The fact that > you've added '####...' section delimeters shows that tracetool.py is > growing to long and we're putting too many concepts into one file. If > each backend is a module then we have a natural way of containing > backend-specific code. Perhaps the module can register itself when > tracetool.py wildcard imports them all. I think this will approach > the level of magic that decorators involve but with the bonus that we > begin to split the code instead of growing a blob. What do you think > of putting each backend in its own module? Sure, the script is getting too large. I just tried to get what I needed with minimal changes on top of the existing code. > Do you have a link to your latest code that adds formats/backends? > I'd like to take a quick look to make sure I understand where you're > going with this - I've only been thinking of the current set of > formats/backends. There's no public repo, sorry. Still, some of "my backends" need registration of intermediate backends *and* formats (e.g., not available through --list-backends) that are specific to instrumentation. 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] 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. > 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. Lluis
2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>: > Stefan Hajnoczi writes: > >> 2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>: >>> Stefan Hajnoczi writes: >>> >>>> 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>: >>>>> 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 <vilanova@ac.upc.edu> >>>>> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> >>>>> --- >>>>> 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? > >> For formats the Format class plus a dict approach is much nicer than >> decorators. The code is short and easy to understand. > > Well, I prefer to automate this kind of things so that new features get > automatically registered and the changes are localized; but it really doesn't > matter that much :) Formats seem so simple to me that the cost of any infrastructure is higher than throwing a Format() instance in a dict. > 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. > > 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(). >> 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. Stefan
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
Stefan Hajnoczi writes: [...] > 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. Does this mean removing from this series the patch adding the decorators? Or just re-send all the series after addressing the other concerns? Thanks, Lluis
Lluís Vilanova writes: > Stefan Hajnoczi writes: > [...] >> 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. > Does this mean removing from this series the patch adding the decorators? Or > just re-send all the series after addressing the other concerns? My bad, never mind.
On Wed, Mar 21, 2012 at 03:10:49PM +0100, Lluís Vilanova wrote: > 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. Ah, yes. That makes sense, I misunderstood. Stefan
diff --git a/Makefile.objs b/Makefile.objs index a718963..228a756 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -357,12 +357,12 @@ else trace.h: trace.h-timestamp endif trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -h < $< > $@," GEN trace.h") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=h --backend=$(TRACE_BACKEND) < $< > $@," GEN trace.h") @cmp -s $@ trace.h || cp $@ trace.h trace.c: trace.c-timestamp trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -c < $< > $@," GEN trace.c") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=c --backend=$(TRACE_BACKEND) < $< > $@," GEN trace.c") @cmp -s $@ trace.c || cp $@ trace.c trace.o: trace.c $(GENERATED_HEADERS) @@ -375,7 +375,7 @@ trace-dtrace.h: trace-dtrace.dtrace # rule file. So we use '.dtrace' instead trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -d < $< > $@," GEN trace-dtrace.dtrace") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=d --backend=$(TRACE_BACKEND) < $< > $@," GEN trace-dtrace.dtrace") @cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS) diff --git a/Makefile.target b/Makefile.target index 3e42e5a..c1e58fb 100644 --- a/Makefile.target +++ b/Makefile.target @@ -60,11 +60,12 @@ endif $(QEMU_PROG).stp: $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py \ + --format=stap \ --backend=$(TRACE_BACKEND) \ --binary=$(bindir)/$(QEMU_PROG) \ --target-arch=$(TARGET_ARCH) \ --target-type=$(TARGET_TYPE) \ - --stap < $(SRC_PATH)/trace-events > $(QEMU_PROG).stp," GEN $(QEMU_PROG).stp") + < $(SRC_PATH)/trace-events > $(QEMU_PROG).stp," GEN $(QEMU_PROG).stp") else stap: endif diff --git a/scripts/tracetool.py b/scripts/tracetool.py index 377c683..6ef4374 100755 --- a/scripts/tracetool.py +++ b/scripts/tracetool.py @@ -12,33 +12,107 @@ import sys import getopt import re -def usage(): - print "Tracetool: Generate tracing code for trace events file on stdin" - print "Usage:" - print sys.argv[0], "--backend=[nop|simple|stderr|dtrace|ust] [-h|-c|-d|--stap]" - print ''' -Backends: - --nop Tracing disabled - --simple Simple built-in backend - --stderr Stderr built-in backend - --dtrace DTrace/SystemTAP backend - --ust LTTng User Space Tracing backend - -Output formats: - -h Generate .h file - -c Generate .c file - -d Generate .d file (DTrace only) - --stap Generate .stp file (DTrace with SystemTAP only) +###################################################################### +# format auto-registration -Options: - --binary [path] Full path to QEMU binary - --target-arch [arch] QEMU emulator target arch - --target-type [type] QEMU emulator target type ('system' or 'user') - --probe-prefix [prefix] Prefix for dtrace probe names - (default: qemu-targettype-targetarch) -''' - sys.exit(1) +class _Tag: + pass + +_formats = {} + +BEGIN = _Tag() +END = _Tag() +_DESCR = _Tag() + +def for_format(format_, when, descr = None): + """Decorator for format generator functions.""" + + if when is not BEGIN and when is not END: + raise ValueError("Invalid 'when' tag") + if format_ in _formats and when in _formats[format_]: + raise ValueError("Format '%s' already set for given 'when' tag" % format_) + + if format_ not in _formats: + _formats[format_] = {} + if descr is not None: + if _DESCR in _formats[format_]: + raise ValueError("Description already set") + _formats[format_][_DESCR] = descr + + def func(f): + _formats[format_][when] = f + return f + return func + +def get_format(format_, when): + """Get a format generator function.""" + + def nop(*args, **kwargs): + pass + if format_ in _formats and when in _formats[format_]: + return _formats[format_][when] + else: + return nop + +def get_format_descr(format_): + """Get the description of a format generator.""" + + if format_ in _formats and _DESCR in _formats[format_]: + return _formats[format_][_DESCR] + else: + return "" + + +###################################################################### +# backend auto-registration and format compatibility + +_backends = {} + +def for_backend(backend, format_, descr = None): + if backend not in _backends: + _backends[backend] = {} + if format_ in _backends[backend]: + raise ValueError("Backend '%s' already set for backend '%s'" % (backend, format_)) + if format_ not in _formats: + raise ValueError("Unknown format '%s'" % format_) + + if descr is not None: + if _DESCR in _backends[backend]: + raise ValueError("Description already set") + _backends[backend][_DESCR] = descr + + def func(f): + _backends[backend][format_] = f + return f + return func + +def get_backend(format_, backend): + if backend not in _backends: + raise ValueError("Unknown backend '%s'" % backend) + if format_ not in _formats: + raise ValueError("Unknown format '%s'" % format_) + if format_ not in _backends[backend]: + raise ValueError("Format '%s' not supported with backend '%s'" % (format_, backend)) + return _backends[backend][format_] + +def get_backend_descr(backend): + """Get the description of a backend.""" + + if backend in _backends and _DESCR in _backends[backend]: + return _backends[backend][_DESCR] + else: + return "" + + + +###################################################################### +# formats + +################################################## +# format: h + +@for_format("h", BEGIN, "Generate .h file") def trace_h_begin(events): print '''#ifndef TRACE_H #define TRACE_H @@ -47,12 +121,27 @@ def trace_h_begin(events): #include "qemu-common.h"''' +@for_format("h", END) def trace_h_end(events): print '#endif /* TRACE_H */' + +################################################## +# format: c + +@for_format("c", BEGIN, "Generate .c file") def trace_c_begin(events): print '/* This file is autogenerated by tracetool, do not edit. */' + + +###################################################################### +# backends + +################################################## +# backend: nop + +@for_backend("nop", "h", "Tracing disabled") def nop_h(events): print for event in events: @@ -63,11 +152,15 @@ def nop_h(events): 'name': event.name, 'args': event.args } - return +@for_backend("nop", "c") def nop_c(events): pass # nop, reqd for converters +################################################## +# backend: simple + +@for_backend("simple", "h", "Simple built-in backend") def simple_h(events): print '#include "trace/simple.h"' print @@ -95,6 +188,7 @@ def simple_h(events): print 'extern TraceEvent trace_list[NR_TRACE_EVENTS];' +@for_backend("simple", "c") def simple_c(events): print '#include "trace.h"' print @@ -108,6 +202,10 @@ def simple_c(events): print print '};' +################################################## +# backend: stderr + +@for_backend("stderr", "h", "Stderr built-in backend") def stderr_h(events): print '''#include <stdio.h> #include "trace/stderr.h" @@ -133,6 +231,7 @@ static inline void trace_%(name)s(%(args)s) print print '#define NR_TRACE_EVENTS %d' % len(events) +@for_backend("stderr", "c") def stderr_c(events): print '''#include "trace.h" @@ -145,6 +244,11 @@ TraceEvent trace_list[] = { print print '};' + +################################################## +# backend: ust + +@for_backend("ust", "h", "LTTng User Space Tracing backend") def ust_h(events): print '''#include <ust/tracepoint.h> #undef mutex_lock @@ -169,6 +273,7 @@ _DECLARE_TRACEPOINT_NOARGS(ust_%(name)s); } print +@for_backend("ust", "c") def ust_c(events): print '''#include <ust/marker.h> #undef mutex_lock @@ -214,6 +319,10 @@ static void __attribute__((constructor)) trace_init(void) } print '}' +################################################## +# backend: dtrace + +@for_backend("dtrace", "h", "DTrace/SystemTAP backend") def dtrace_h(events): print '#include "trace-dtrace.h"' print @@ -230,9 +339,15 @@ def dtrace_h(events): 'argnames': ", ".join(event.args.names()), } +@for_backend("dtrace", "c") def dtrace_c(events): pass # No need for function definitions in dtrace backend +@for_format("d", BEGIN, "Generate .d file (DTrace probes)") +def trace_d_begin(events): + print '/* This file is autogenerated by tracetool, do not edit. */' + +@for_backend("dtrace", "d") def dtrace_d(events): print 'provider qemu {' for event in events: @@ -252,9 +367,15 @@ def dtrace_d(events): print print '};' +@for_backend("nop", "d") def dtrace_nop_d(events): pass + +@for_format("stap", BEGIN, "Generate .stp file (SystemTAP tapsets)") +def trace_stap_begin(events): + print '/* This file is autogenerated by tracetool, do not edit. */' + def dtrace_stp(events): for event in events: # Define prototype for probe arguments @@ -279,64 +400,10 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s") def dtrace_nop_stp(events): pass -def trace_stap_begin(events): - print '/* This file is autogenerated by tracetool, do not edit. */' - -def trace_d_begin(events): - print '/* This file is autogenerated by tracetool, do not edit. */' - - -# Registry of backends and their converter functions -converters = { - 'simple': { - 'h': simple_h, - 'c': simple_c, - }, - - 'nop': { - 'h': nop_h, - 'c': nop_c, - 'd': dtrace_nop_d, - 'stap': dtrace_nop_stp, - }, - - 'stderr': { - 'h': stderr_h, - 'c': stderr_c, - }, - - 'dtrace': { - 'h': dtrace_h, - 'c': dtrace_c, - 'd': dtrace_d, - 'stap': dtrace_stp - }, - - 'ust': { - 'h': ust_h, - 'c': ust_c, - }, - -} - -# Trace file header and footer code generators -formats = { - 'h': { - 'begin': trace_h_begin, - 'end': trace_h_end, - }, - 'c': { - 'begin': trace_c_begin, - }, - 'd': { - 'begin': trace_d_begin, - }, - 'stap': { - 'begin': trace_stap_begin, - }, -} +###################################################################### # Event arguments + class Arguments: def __init__ (self, arg_str): self._args = [] @@ -369,7 +436,10 @@ class Arguments: def types(self): return [ type_ for type_, _ in self._args ] + +###################################################################### # A trace event + cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?") VALID_PROPS = set(["disable"]) @@ -399,32 +469,51 @@ def read_events(fobj): res.append(Event(line)) return res +###################################################################### +# Main + +format_ = "" binary = "" probeprefix = "" +def usage(): + print "Tracetool: Generate tracing code for trace events file on stdin" + print "Usage:" + print sys.argv[0], " --format=<format> --backend=<backend>" + print + print "Output formats:" + for f in _formats: + print " %-10s %s" % (f, get_format_descr(f)) + print + print "Backends:" + for b in _backends: + print " %-10s %s" % (b, get_backend_descr(b)) + print """ +Options: + --binary [path] Full path to QEMU binary + --target-arch [arch] QEMU emulator target arch + --target-type [type] QEMU emulator target type ('system' or 'user') + --probe-prefix [prefix] Prefix for dtrace probe names + (default: qemu-targettype-targetarch) +""" + + sys.exit(1) + def main(): - global binary, probeprefix + global format_, binary, probeprefix targettype = "" targetarch = "" - 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"] + long_options = ["format=", "backend=", "binary=", "target-arch=", "target-type=", "probe-prefix=", "list-backends", "check-backend"] try: - opts, args = getopt.getopt(sys.argv[1:], short_options, long_options) + opts, args = getopt.getopt(sys.argv[1:], "", 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' + if opt == '--format': + format_ = arg elif opt == '--backend': backend = arg elif opt == '--binary': @@ -436,30 +525,27 @@ def main(): elif opt == '--probe-prefix': probeprefix = arg elif opt == '--list-backends': - print 'simple, nop, stderr, dtrace, ust' + print ', '.join(_backends) sys.exit(0) elif opt == "--check-backend": - if any(backend in s for s in supported_backends): + if backend in _backends: sys.exit(0) else: sys.exit(1) else: - #assert False, "unhandled option" print "unhandled option: ", opt usage() - if backend == "" or output == "": + if format_ not in _formats: + print "Unknown format: %s" % format_ + print + usage() + if backend not in _backends: + print "Unknown backend: %s" % backend + print usage() - sys.exit(0) - - if backend != 'dtrace' and output == 'd': - print 'DTrace probe generator not applicable to %s backend' % backend - sys.exit(1) - if output == 'stap': - if backend != "dtrace": - print 'SystemTAP tapset generator not applicable to %s backend' % backend - sys.exit(1) + if format_ == 'stap': if binary == "": print '--binary is required for SystemTAP tapset generator' sys.exit(1) @@ -474,12 +560,18 @@ def main(): events = read_events(sys.stdin) - if 'begin' in formats[output]: - formats[output]['begin'](events) - converters[backend][output]([ e for e in events if 'disable' not in e.properties ]) - converters['nop'][output]([ e for e in events if 'disable' in e.properties ]) - if 'end' in formats[output]: - formats[output]['end'](events) + try: + # just force format/backend compatibility check + bfun = get_backend(format_, backend) + bnop = get_backend(format_, "nop") + except Exception as e: + sys.stderr.write(str(e) + "\n\n") + usage() + + get_format(format_, BEGIN)(events) + bfun([ e for e in events if "disable" not in e.properties ]) + bnop([ e for e in events if "disable" in e.properties ]) + get_format(format_, END)(events) if __name__ == "__main__": main()