[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 20/20] trace: introduce a formal group name f
From: |
Lluís Vilanova |
Subject: |
Re: [Qemu-devel] [PATCH v5 20/20] trace: introduce a formal group name for trace events |
Date: |
Fri, 30 Sep 2016 16:36:25 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Daniel P Berrange writes:
> The declarations in the generated-tracers.h file are
> assuming there's only ever going to be one instance
> of this header, as they are not namespaced. When we
> have one header per event group, if a single source
> file needs to include multiple sets of trace events,
> the symbols will all clash.
> This change thus introduces a '--group NAME' arg to the
> 'tracetool' program. This will cause all the symbols in
> the generated header files to be given a unique namespace.
> If no group is given, the group name 'common' is used,
> which is suitable for the current usage where there is
> only one global trace-events file used for code generation.
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> scripts/tracetool.py | 15 ++++++++++++++-
> scripts/tracetool/__init__.py | 6 ++++--
> scripts/tracetool/backend/__init__.py | 12 ++++++------
> scripts/tracetool/backend/dtrace.py | 4 ++--
> scripts/tracetool/backend/ftrace.py | 5 +++--
> scripts/tracetool/backend/log.py | 4 ++--
> scripts/tracetool/backend/simple.py | 8 ++++----
> scripts/tracetool/backend/syslog.py | 4 ++--
> scripts/tracetool/backend/ust.py | 4 ++--
> scripts/tracetool/format/__init__.py | 4 ++--
> scripts/tracetool/format/c.py | 18 ++++++++++--------
> scripts/tracetool/format/d.py | 2 +-
> scripts/tracetool/format/h.py | 14 +++++++-------
> scripts/tracetool/format/simpletrace_stap.py | 2 +-
> scripts/tracetool/format/stap.py | 2 +-
> scripts/tracetool/format/tcg_h.py | 2 +-
> scripts/tracetool/format/tcg_helper_c.py | 2 +-
> scripts/tracetool/format/tcg_helper_h.py | 2 +-
> scripts/tracetool/format/tcg_helper_wrapper_h.py | 2 +-
> scripts/tracetool/format/ust_events_c.py | 2 +-
> scripts/tracetool/format/ust_events_h.py | 9 +++++----
> 21 files changed, 71 insertions(+), 52 deletions(-)
> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index f66e767..629b259 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py
> @@ -15,6 +15,8 @@ __email__ = "address@hidden"
> import sys
> import getopt
> +import os.path
> +import re
> from tracetool import error_write, out
> import tracetool.backend
> @@ -60,6 +62,15 @@ Options:
> else:
> sys.exit(1)
> +def make_group_name(filename):
> + dirname = os.path.realpath(os.path.dirname(filename))
> + basedir = os.path.join(os.path.dirname(__file__), os.pardir)
> + basedir = os.path.realpath(os.path.abspath(basedir))
> + dirname = dirname[len(basedir) + 1:]
> +
> + if dirname == "":
> + return "common"
> + return re.sub(r"/|-", "_", dirname)
> def main(args):
> global _SCRIPT
> @@ -134,8 +145,10 @@ def main(args):
> with open(args[0], "r") as fh:
> events = tracetool.read_events(fh)
> + group = make_group_name(args[0])
> +
> try:
> - tracetool.generate(events, arg_format, arg_backends,
> + tracetool.generate(events, group, arg_format, arg_backends,
> binary=binary, probe_prefix=probe_prefix)
> except tracetool.TracetoolError as e:
> error_opt(str(e))
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index ac2cb8f..365446f 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -364,7 +364,7 @@ def try_import(mod_name, attr_name=None,
> attr_default=None):
> return False, None
> -def generate(events, format, backends,
> +def generate(events, group, format, backends,
> binary=None, probe_prefix=None):
> """Generate the output for the given (format, backends) pair.
> @@ -372,6 +372,8 @@ def generate(events, format, backends,
> ----------
> events : list
> list of Event objects to generate for
> + group: str
> + Name of the tracing group
> format : str
> Output format name.
> backends : list
> @@ -401,4 +403,4 @@ def generate(events, format, backends,
> tracetool.backend.dtrace.BINARY = binary
> tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
> - tracetool.format.generate(events, format, backend)
> + tracetool.format.generate(events, format, backend, group)
> diff --git a/scripts/tracetool/backend/__init__.py
> b/scripts/tracetool/backend/__init__.py
> index d4b6dab..f735a25 100644
> --- a/scripts/tracetool/backend/__init__.py
> +++ b/scripts/tracetool/backend/__init__.py
> @@ -113,11 +113,11 @@ class Wrapper:
> if func is not None:
> func(*args, **kwargs)
> - def generate_begin(self, events):
> - self._run_function("generate_%s_begin", events)
> + def generate_begin(self, events, group):
> + self._run_function("generate_%s_begin", events, group)
> - def generate(self, event):
> - self._run_function("generate_%s", event)
> + def generate(self, event, group):
> + self._run_function("generate_%s", event, group)
> - def generate_end(self, events):
> - self._run_function("generate_%s_end", events)
> + def generate_end(self, events, group):
> + self._run_function("generate_%s_end", events, group)
> diff --git a/scripts/tracetool/backend/dtrace.py
> b/scripts/tracetool/backend/dtrace.py
> index ab9ecfa..79505c6 100644
> --- a/scripts/tracetool/backend/dtrace.py
> +++ b/scripts/tracetool/backend/dtrace.py
> @@ -35,12 +35,12 @@ def binary():
> return BINARY
> -def generate_h_begin(events):
> +def generate_h_begin(events, group):
> out('#include "trace/generated-tracers-dtrace.h"',
> '')
> -def generate_h(event):
> +def generate_h(event, group):
> out(' QEMU_%(uppername)s(%(argnames)s);',
> uppername=event.name.upper(),
> argnames=", ".join(event.args.names()))
> diff --git a/scripts/tracetool/backend/ftrace.py
> b/scripts/tracetool/backend/ftrace.py
> index 4b83b44..8ee1a20 100644
> --- a/scripts/tracetool/backend/ftrace.py
> +++ b/scripts/tracetool/backend/ftrace.py
> @@ -19,12 +19,12 @@ from tracetool import out
> PUBLIC = True
> -def generate_h_begin(events):
> +def generate_h_begin(events, group):
> out('#include "trace/ftrace.h"',
> '')
> -def generate_h(event):
> +def generate_h(event, group):
> argnames = ", ".join(event.args.names())
> if len(event.args) > 0:
> argnames = ", " + argnames
> @@ -40,6 +40,7 @@ def generate_h(event):
> ' unused = write(trace_marker_fd, ftrace_buf,
> trlen);',
> ' }',
> ' }',
> + group=group.lower(),
This seems to be added for no reason.
> name=event.name,
> args=event.args,
> event_id="TRACE_" + event.name.upper(),
> diff --git a/scripts/tracetool/backend/log.py
> b/scripts/tracetool/backend/log.py
> index 426bb6e..4f4a4d3 100644
> --- a/scripts/tracetool/backend/log.py
> +++ b/scripts/tracetool/backend/log.py
> @@ -19,12 +19,12 @@ from tracetool import out
> PUBLIC = True
> -def generate_h_begin(events):
> +def generate_h_begin(events, group):
> out('#include "qemu/log.h"',
> '')
> -def generate_h(event):
> +def generate_h(event, group):
> argnames = ", ".join(event.args.names())
> if len(event.args) > 0:
> argnames = ", " + argnames
> diff --git a/scripts/tracetool/backend/simple.py
> b/scripts/tracetool/backend/simple.py
> index 1114e35..9885e83 100644
> --- a/scripts/tracetool/backend/simple.py
> +++ b/scripts/tracetool/backend/simple.py
> @@ -27,7 +27,7 @@ def is_string(arg):
> return False
> -def generate_h_begin(events):
> +def generate_h_begin(events, group):
> for event in events:
> out('void _simple_%(api)s(%(args)s);',
> api=event.api(),
> @@ -35,13 +35,13 @@ def generate_h_begin(events):
> out('')
> -def generate_h(event):
> +def generate_h(event, group):
> out(' _simple_%(api)s(%(args)s);',
> api=event.api(),
> args=", ".join(event.args.names()))
> -def generate_c_begin(events):
> +def generate_c_begin(events, group):
> out('#include "qemu/osdep.h"',
> '#include "trace.h"',
> '#include "trace/control.h"',
> @@ -49,7 +49,7 @@ def generate_c_begin(events):
> '')
> -def generate_c(event):
> +def generate_c(event, group):
> out('void _simple_%(api)s(%(args)s)',
> '{',
> ' TraceBufferRecord rec;',
> diff --git a/scripts/tracetool/backend/syslog.py
> b/scripts/tracetool/backend/syslog.py
> index 5f693bd..b8ff279 100644
> --- a/scripts/tracetool/backend/syslog.py
> +++ b/scripts/tracetool/backend/syslog.py
> @@ -19,12 +19,12 @@ from tracetool import out
> PUBLIC = True
> -def generate_h_begin(events):
> +def generate_h_begin(events, group):
> out('#include <syslog.h>',
> '')
> -def generate_h(event):
> +def generate_h(event, group):
> argnames = ", ".join(event.args.names())
> if len(event.args) > 0:
> argnames = ", " + argnames
> diff --git a/scripts/tracetool/backend/ust.py
> b/scripts/tracetool/backend/ust.py
> index ed4c227..4594db6 100644
> --- a/scripts/tracetool/backend/ust.py
> +++ b/scripts/tracetool/backend/ust.py
> @@ -19,13 +19,13 @@ from tracetool import out
> PUBLIC = True
> -def generate_h_begin(events):
> +def generate_h_begin(events, group):
> out('#include <lttng/tracepoint.h>',
> '#include "trace/generated-ust-provider.h"',
> '')
> -def generate_h(event):
> +def generate_h(event, group):
> argnames = ", ".join(event.args.names())
> if len(event.args) > 0:
> argnames = ", " + argnames
> diff --git a/scripts/tracetool/format/__init__.py
> b/scripts/tracetool/format/__init__.py
> index 812570f..cf6e0e2 100644
> --- a/scripts/tracetool/format/__init__.py
> +++ b/scripts/tracetool/format/__init__.py
> @@ -74,7 +74,7 @@ def exists(name):
> return tracetool.try_import("tracetool.format." + name)[1]
> -def generate(events, format, backend):
> +def generate(events, format, backend, group):
> if not exists(format):
> raise ValueError("unknown format: %s" % format)
> format = format.replace("-", "_")
> @@ -82,4 +82,4 @@ def generate(events, format, backend):
> "generate")[1]
> if func is None:
> raise AttributeError("format has no 'generate': %s" % format)
> - func(events, backend)
> + func(events, backend, group)
> diff --git a/scripts/tracetool/format/c.py b/scripts/tracetool/format/c.py
> index b79114b..7addde6 100644
> --- a/scripts/tracetool/format/c.py
> +++ b/scripts/tracetool/format/c.py
> @@ -16,7 +16,7 @@ __email__ = "address@hidden"
> from tracetool import out
> -def generate(events, backend):
> +def generate(events, backend, group):
> active_events = [e for e in events
> if "disable" not in e.properties]
> @@ -47,7 +47,8 @@ def generate(events, backend):
> sstate = "TRACE_%s_ENABLED" % e.name.upper(),
> dstate = e.api(e.QEMU_DSTATE))
> - out('TraceEvent *trace_events[] = {')
> + out('TraceEvent *%(group)s_trace_events[] = {',
> + group = group.lower())
> for e in events:
> out('&%(event)s,', event = e.api(e.QEMU_EVENT))
> @@ -56,13 +57,14 @@ def generate(events, backend):
> '};',
> '')
> - out('static void trace_register_events(void)',
> + out('static void trace_%(group)s_register_events(void)',
> '{',
> - ' trace_event_register_group(trace_events);',
> + ' trace_event_register_group(%(group)s_trace_events);',
> '}',
> - 'trace_init(trace_register_events)')
> + 'trace_init(trace_%(group)s_register_events)',
> + group = group.lower())
> - backend.generate_begin(active_events)
> + backend.generate_begin(active_events, group)
> for event in active_events:
> - backend.generate(event)
> - backend.generate_end(active_events)
> + backend.generate(event, group)
> + backend.generate_end(active_events, group)
> diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
> index c77d5b7..78397c2 100644
> --- a/scripts/tracetool/format/d.py
> +++ b/scripts/tracetool/format/d.py
> @@ -29,7 +29,7 @@ RESERVED_WORDS = (
> )
> -def generate(events, backend):
> +def generate(events, backend, group):
> events = [e for e in events
> if "disable" not in e.properties]
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index f4748c3..3682f4e 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -16,11 +16,11 @@ __email__ = "address@hidden"
> from tracetool import out
> -def generate(events, backend):
> +def generate(events, backend, group):
> out('/* This file is autogenerated by tracetool, do not edit. */',
> '',
> - '#ifndef TRACE__GENERATED_TRACERS_H',
> - '#define TRACE__GENERATED_TRACERS_H',
> + '#ifndef TRACE_%s_GENERATED_TRACERS_H' % group.upper(),
> + '#define TRACE_%s_GENERATED_TRACERS_H' % group.upper(),
> '',
> '#include "qemu-common.h"',
> '#include "trace/control.h"',
> @@ -46,7 +46,7 @@ def generate(events, backend):
> enabled=enabled)
> out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
> - backend.generate_begin(events)
> + backend.generate_begin(events, group)
> for e in events:
> if "vcpu" in e.properties:
> @@ -68,11 +68,11 @@ def generate(events, backend):
> cond=cond)
> if "disable" not in e.properties:
> - backend.generate(e)
> + backend.generate(e, group)
> out(' }',
> '}')
> - backend.generate_end(events)
> + backend.generate_end(events, group)
> - out('#endif /* TRACE__GENERATED_TRACERS_H */')
> + out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
> diff --git a/scripts/tracetool/format/simpletrace_stap.py
> b/scripts/tracetool/format/simpletrace_stap.py
> index ac3580f..c35e662 100644
> --- a/scripts/tracetool/format/simpletrace_stap.py
> +++ b/scripts/tracetool/format/simpletrace_stap.py
> @@ -19,7 +19,7 @@ from tracetool.backend.simple import is_string
> from tracetool.format.stap import stap_escape
> -def generate(events, backend):
> +def generate(events, backend, group):
> out('/* This file is autogenerated by tracetool, do not edit. */',
> '',
> 'global event_name_to_id_map',
> diff --git a/scripts/tracetool/format/stap.py
> b/scripts/tracetool/format/stap.py
> index 9e780f1..e8ef3e7 100644
> --- a/scripts/tracetool/format/stap.py
> +++ b/scripts/tracetool/format/stap.py
> @@ -34,7 +34,7 @@ def stap_escape(identifier):
> return identifier
> -def generate(events, backend):
> +def generate(events, backend, group):
> events = [e for e in events
> if "disable" not in e.properties]
> diff --git a/scripts/tracetool/format/tcg_h.py
> b/scripts/tracetool/format/tcg_h.py
> index e2331f2..628388a 100644
> --- a/scripts/tracetool/format/tcg_h.py
> +++ b/scripts/tracetool/format/tcg_h.py
> @@ -27,7 +27,7 @@ def vcpu_transform_args(args):
> ])
> -def generate(events, backend):
> +def generate(events, backend, group):
> out('/* This file is autogenerated by tracetool, do not edit. */',
> '/* You must include this file after the inclusion of helper.h */',
> '',
The header guard here should use the module name too.
> diff --git a/scripts/tracetool/format/tcg_helper_c.py
> b/scripts/tracetool/format/tcg_helper_c.py
> index e3485b7..cc26e03 100644
> --- a/scripts/tracetool/format/tcg_helper_c.py
> +++ b/scripts/tracetool/format/tcg_helper_c.py
> @@ -40,7 +40,7 @@ def vcpu_transform_args(args, mode):
> assert False
> -def generate(events, backend):
> +def generate(events, backend, group):
> events = [e for e in events
> if "disable" not in e.properties]
> diff --git a/scripts/tracetool/format/tcg_helper_h.py
> b/scripts/tracetool/format/tcg_helper_h.py
> index dc76c15..6b184b6 100644
> --- a/scripts/tracetool/format/tcg_helper_h.py
> +++ b/scripts/tracetool/format/tcg_helper_h.py
> @@ -18,7 +18,7 @@ from tracetool.transform import *
> import tracetool.vcpu
> -def generate(events, backend):
> +def generate(events, backend, group):
> events = [e for e in events
> if "disable" not in e.properties]
> diff --git a/scripts/tracetool/format/tcg_helper_wrapper_h.py
> b/scripts/tracetool/format/tcg_helper_wrapper_h.py
> index 020f442..ff53447 100644
> --- a/scripts/tracetool/format/tcg_helper_wrapper_h.py
> +++ b/scripts/tracetool/format/tcg_helper_wrapper_h.py
> @@ -18,7 +18,7 @@ from tracetool.transform import *
> import tracetool.vcpu
> -def generate(events, backend):
> +def generate(events, backend, group):
> events = [e for e in events
> if "disable" not in e.properties]
> diff --git a/scripts/tracetool/format/ust_events_c.py
> b/scripts/tracetool/format/ust_events_c.py
> index 9967c7a..cd87d8a 100644
> --- a/scripts/tracetool/format/ust_events_c.py
> +++ b/scripts/tracetool/format/ust_events_c.py
> @@ -16,7 +16,7 @@ __email__ = "address@hidden"
> from tracetool import out
> -def generate(events, backend):
> +def generate(events, backend, group):
> events = [e for e in events
> if "disabled" not in e.properties]
> diff --git a/scripts/tracetool/format/ust_events_h.py
> b/scripts/tracetool/format/ust_events_h.py
> index 3e8a7cd..d853155 100644
> --- a/scripts/tracetool/format/ust_events_h.py
> +++ b/scripts/tracetool/format/ust_events_h.py
> @@ -16,7 +16,7 @@ __email__ = "address@hidden"
> from tracetool import out
> -def generate(events, backend):
> +def generate(events, backend, group):
> events = [e for e in events
> if "disabled" not in e.properties]
> @@ -28,8 +28,9 @@ def generate(events, backend):
> '#undef TRACEPOINT_INCLUDE_FILE',
> '#define TRACEPOINT_INCLUDE_FILE ./generated-ust-provider.h',
> '',
> - '#if !defined (TRACE__GENERATED_UST_H) ||
> defined(TRACEPOINT_HEADER_MULTI_READ)',
> - '#define TRACE__GENERATED_UST_H',
> + '#if !defined (TRACE_%s_GENERATED_UST_H) || \\' % group.upper(),
> + ' defined(TRACEPOINT_HEADER_MULTI_READ)',
> + '#define TRACE_%s_GENERATED_UST_H' % group.upper(),
> '',
> '#include "qemu-common.h"',
> '#include <lttng/tracepoint.h>',
> @@ -94,7 +95,7 @@ def generate(events, backend):
> '',
> name=e.name)
> - out('#endif /* TRACE__GENERATED_UST_H */',
> + out('#endif /* TRACE_%s_GENERATED_UST_H */' % group.upper(),
> '',
> '/* This part must be outside ifdef protection */',
> '#include <lttng/tracepoint-event.h>')
> --
> 2.7.4
- [Qemu-devel] [PATCH v5 14/20] trace: dynamically allocate trace_dstate in CPUState, (continued)
- [Qemu-devel] [PATCH v5 14/20] trace: dynamically allocate trace_dstate in CPUState, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 10/20] trace: remove the TraceEventID and TraceEventVCPUID enums, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 13/20] trace: provide mechanism for registering trace events, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 15/20] trace: dynamically allocate event IDs at runtime, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 16/20] trace: get rid of generated-events.h/generated-events.c, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 18/20] trace: push reading of events up a level to tracetool main, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 17/20] trace: rename _read_events to read_events, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 19/20] trace: pass trace-events to tracetool as a positional param, Daniel P. Berrange, 2016/09/28
- [Qemu-devel] [PATCH v5 20/20] trace: introduce a formal group name for trace events, Daniel P. Berrange, 2016/09/28
- Re: [Qemu-devel] [PATCH v5 20/20] trace: introduce a formal group name for trace events,
Lluís Vilanova <=