qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v4 07/17] trace: give each trace event a named T


From: Lluís Vilanova
Subject: Re: [Qemu-devel] [PATCH v4 07/17] trace: give each trace event a named TraceEvent struct
Date: Thu, 22 Sep 2016 14:26:37 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Daniel P Berrange writes:

> Currently we only expose a TraceEvent array, which must
> be indexed via the TraceEventID enum constants. This
> changes the generator to expose a named TraceEvent
> instance for each event, with an _EV suffix.

> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  scripts/tracetool/__init__.py        |  1 +
>  scripts/tracetool/format/events_c.py | 19 +++++++++++++------
>  scripts/tracetool/format/events_h.py | 11 ++++++++---
>  trace/control-internal.h             |  2 +-
>  trace/control.c                      |  2 +-
>  5 files changed, 24 insertions(+), 11 deletions(-)

> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 2a8d7d5..2d0290d 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -266,6 +266,7 @@ class Event(object):
>      QEMU_TRACE               = "trace_%(name)s"
>      QEMU_TRACE_TCG           = QEMU_TRACE + "_tcg"
>      QEMU_DSTATE              = "___TRACE_%(NAME)s_DSTATE"
> +    QEMU_EVENT               = "TRACE_%(NAME)s_EVENT"

Please add a "___" prefix like in QEMU_DSTATE, to make it clear this is not
meant to be directly used.


>      def api(self, fmt=None):
>          if fmt is None:
> diff --git a/scripts/tracetool/format/events_c.py 
> b/scripts/tracetool/format/events_c.py
> index ef873fa..0dd4a33 100644
> --- a/scripts/tracetool/format/events_c.py
> +++ b/scripts/tracetool/format/events_c.py
> @@ -28,22 +28,29 @@ def generate(events, backend):
>      for e in events:
>          out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
 
> -    out('TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
> -
>      for e in events:
>          if "vcpu" in e.properties:
>              vcpu_id = "TRACE_VCPU_" + e.name.upper()
>          else:
>              vcpu_id = "TRACE_VCPU_EVENT_COUNT"
> -        out('    { .id = %(id)s, .vcpu_id = %(vcpu_id)s,'
> -            ' .name = \"%(name)s\",'
> -            ' .sstate = %(sstate)s,',
> -            ' .dstate = &%(dstate)s, }, ',
> +        out('TraceEvent %(event)s = {',
> +            '  .id = %(id)s,',
> +            '  .vcpu_id = %(vcpu_id)s,',
> +            '  .name = \"%(name)s\",',
> +            '  .sstate = %(sstate)s,',
> +            '  .dstate = &%(dstate)s ',

The event attributes should have 4 indentation spaces for the generated code to
be consistent with the coding style (picky complain, feel free to ignore).


> +            '};',
> +            event = e.api(e.QEMU_EVENT),
>              id = "TRACE_" + e.name.upper(),
>              vcpu_id = vcpu_id,
>              name = e.name,
>              sstate = "TRACE_%s_ENABLED" % e.name.upper(),
>              dstate = e.api(e.QEMU_DSTATE))
 
> +    out('TraceEvent *trace_events[TRACE_EVENT_COUNT] = {')
> +
> +    for e in events:
> +        out('&%(event)s,', event = e.api(e.QEMU_EVENT))
> +
>      out('};',
>          '')
> diff --git a/scripts/tracetool/format/events_h.py 
> b/scripts/tracetool/format/events_h.py
> index 03417de..80a66c5 100644
> --- a/scripts/tracetool/format/events_h.py
> +++ b/scripts/tracetool/format/events_h.py
> @@ -21,7 +21,13 @@ def generate(events, backend):
>          '',
>          '#ifndef TRACE__GENERATED_EVENTS_H',
>          '#define TRACE__GENERATED_EVENTS_H',
> -        '')
> +        '',
> +        '#include "trace/event-internal.h"',
> +        )
> +
> +    for e in events:
> +        out('extern TraceEvent %(event)s;',
> +            event = e.api(e.QEMU_EVENT))
 
>      # event identifiers
>      out('typedef enum {')
> @@ -58,6 +64,5 @@ def generate(events, backend):
>                  enabled=enabled)
>          out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
 
> -    out('#include "trace/event-internal.h"',
> -        '',
> +    out('',
>          '#endif  /* TRACE__GENERATED_EVENTS_H */')
> diff --git a/trace/control-internal.h b/trace/control-internal.h
> index 828c1fc..52b6b72 100644
> --- a/trace/control-internal.h
> +++ b/trace/control-internal.h
> @@ -15,7 +15,7 @@
>  #include "qom/cpu.h"
 
 
> -extern TraceEvent trace_events[];
> +extern TraceEvent *trace_events[];
>  extern int trace_events_enabled_count;
 
 
> diff --git a/trace/control.c b/trace/control.c
> index a103560..9107919 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -106,7 +106,7 @@ void trace_event_iter_init(TraceEventIter *iter, const 
> char *pattern)
>  TraceEvent *trace_event_iter_next(TraceEventIter *iter)
>  {
>      while (iter->event < TRACE_EVENT_COUNT) {
> -        TraceEvent *ev = &(trace_events[iter->event]);
> +        TraceEvent *ev = trace_events[iter->event];
iter-> event++;
>          if (!iter->pattern ||
>              pattern_glob(iter->pattern,
> -- 
> 2.7.4

Thanks,
  Lluis



reply via email to

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