Re: [Qemu-devel] [PATCH v3 18/18] trace: introduce a formal group name for trace events

[Lluís Vilanova]

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                             | 24 
> +++++++++++++++++++++++-
>  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         |  8 ++++----
>  21 files changed, 79 insertions(+), 52 deletions(-)

> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index f66e767..0ee66f3 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,24 @@ Options:
>      else:
>          sys.exit(1)
 
> +def find_git_root(dirname):
> +    while dirname != "":
> +        git = os.path.join(dirname, ".git")
> +        if os.path.exists(git):
> +            return dirname
> +        dirname = os.path.dirname(dirname)
> +
> +    raise ValueError("Cannot find .git directory for %s",
> +                     filename)
> +

Some people (distributions?) distribute source tarballs without the VCS
files/directories, so this might break.

Even if ugly, it might be easier to just hardcode it based on the script's
relative location:

  os.path.abspath(os.path.join(os.dirname(___file___), os.pardir))


> +def make_group_name(filename):
> +    dirname = os.path.dirname(filename)
> +    gitdir = find_git_root(dirname)
> +    dirname = dirname[len(gitdir):]
> +
> +    if dirname == "":
> +        return "common"
> +    return re.sub(r"/|-", "_", dirname)
 
>  def main(args):
>      global _SCRIPT
> @@ -134,8 +154,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 ed668a1..be8a360 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -363,7 +363,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.
 
> @@ -371,6 +371,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
> @@ -400,4 +402,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 group seems unused.


>          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 971bd97..b0db50c 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 24f8d2e..31438fa 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,',
> @@ -57,13 +58,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 f1dc493..3f1d5e9 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"',
> @@ -45,7 +45,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:
> @@ -67,11 +67,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 0573f43..3d700ca 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 */',
>          '',
> 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..00b69d9 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,8 @@ 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) || 
> defined(TRACEPOINT_HEADER_MULTI_READ)' % group.upper(),
> +        '#define TRACE_%s_GENERATED_UST_H' % group.upper(),
>          '',
>          '#include "qemu-common.h"',
>          '#include <lttng/tracepoint.h>',
> @@ -94,7 +94,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