Re: [Qemu-devel] [PATCH v3 14/18] trace: get rid of generated-events.h/generated-events.c

[Lluís Vilanova]

Daniel P Berrange writes:

> Currently the generated-events.[ch] files contain the
> event dstates, constants and TraceEvent structs, while the
> generated-tracers.[ch] files contain the actual trace
> probe logic. With the removal of usage of the event enums
> from the API there is no longer any compelling reason for
> the separation between these files. The generated-events.h
> content is only ever be needed from the generated-tracers.[ch]
> files.

> The enums/constants/structs from generated-events.[ch] are
> thus moved into the generated-tracers.[ch], so that there
> is one less file to be generated.

> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  Makefile                      |  3 ---
>  include/trace-tcg.h           |  1 -
>  include/trace.h               |  1 -
>  scripts/tracetool/format/c.py | 51 
> ++++++++++++++++++++++++++++++++++++++-----
>  scripts/tracetool/format/h.py | 19 ++++++++++++++++
>  trace/Makefile.objs           | 28 ++++--------------------
>  trace/control.h               |  2 +-
>  trace/simple.h                |  4 ----
>  8 files changed, 70 insertions(+), 39 deletions(-)

> diff --git a/Makefile b/Makefile
> index 50b4b3a..16a35b0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -56,9 +56,6 @@ GENERATED_SOURCES += qmp-marshal.c qapi-types.c 
> qapi-visit.c qapi-event.c
>  GENERATED_HEADERS += qmp-introspect.h
>  GENERATED_SOURCES += qmp-introspect.c
 
> -GENERATED_HEADERS += trace/generated-events.h
> -GENERATED_SOURCES += trace/generated-events.c
> -
>  GENERATED_HEADERS += trace/generated-tracers.h
>  ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
>  GENERATED_HEADERS += trace/generated-tracers-dtrace.h
> diff --git a/include/trace-tcg.h b/include/trace-tcg.h
> index edab4b1..da68608 100644
> --- a/include/trace-tcg.h
> +++ b/include/trace-tcg.h
> @@ -2,6 +2,5 @@
>  #define TRACE_TCG_H
 
>  #include "trace/generated-tcg-tracers.h"
> -#include "trace/generated-events.h"
 
>  #endif /* TRACE_TCG_H */
> diff --git a/include/trace.h b/include/trace.h
> index 9a01e44..ac9ff3d 100644
> --- a/include/trace.h
> +++ b/include/trace.h
> @@ -2,6 +2,5 @@
>  #define TRACE_H
 
>  #include "trace/generated-tracers.h"
> -#include "trace/generated-events.h"
 
>  #endif /* TRACE_H */
> diff --git a/scripts/tracetool/format/c.py b/scripts/tracetool/format/c.py
> index 699598f..24f8d2e 100644
> --- a/scripts/tracetool/format/c.py
> +++ b/scripts/tracetool/format/c.py
> @@ -17,12 +17,53 @@ from tracetool import out
 
 
>  def generate(events, backend):
> -    events = [e for e in events
> -              if "disable" not in e.properties]
> +    active_events = [e for e in events
> +                     if "disable" not in e.properties]
 
>      out('/* This file is autogenerated by tracetool, do not edit. */',
> +        '',
> +        '#include "qemu/osdep.h"',
> +        '#include "trace.h"',
>          '')
> -    backend.generate_begin(events)
> -    for event in events:
> +
> +    for e in events:
> +        out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
> +
> +    for e in events:
> +        if "vcpu" in e.properties:
> +            vcpu_id = 0
> +        else:
> +            vcpu_id = "TRACE_VCPU_EVENT_NONE"
> +        out('TraceEvent %(event)s = {',
> +            '  .id = 0,',
> +            '  .vcpu_id = %(vcpu_id)s,',
> +            '  .name = \"%(name)s\",',
> +            '  .sstate = %(sstate)s,',
> +            '  .dstate = &%(dstate)s ',
> +            '};',
> +            event = "TRACE_" + e.name.upper() + "_EV",

Event.api() again.


> +            vcpu_id = vcpu_id,
> +            name = e.name,
> +            sstate = "TRACE_%s_ENABLED" % e.name.upper(),
> +            dstate = e.api(e.QEMU_DSTATE))
> +
> +    out('TraceEvent *trace_events[] = {')
> +
> +    for e in events:
> +        out('&%(event)s,',
> +            event = "TRACE_" + e.name.upper() + "_EV")
> +
> +    out('  NULL,',
> +        '};',
> +        '')
> +
> +    out('static void trace_register_events(void)',
> +        '{',
> +        '    trace_event_register_group(trace_events);',
> +        '}',
> +        'trace_init(trace_register_events)')
> +
> +    backend.generate_begin(active_events)
> +    for event in active_events:
>          backend.generate(event)
> -    backend.generate_end(events)
> +    backend.generate_end(active_events)
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index 64a6680..f1dc493 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -26,6 +26,25 @@ def generate(events, backend):
>          '#include "trace/control.h"',
>          '')
 
> +    for e in events:
> +        out('extern TraceEvent TRACE_%s_EV;' % e.name.upper())
> +
> +    for e in events:
> +        out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE))
> +
> +    # static state
> +    for e in events:
> +        if 'disable' in e.properties:
> +            enabled = 0
> +        else:
> +            enabled = 1
> +        if "tcg-exec" in e.properties:
> +            # a single define for the two "sub-events"
> +            out('#define TRACE_%(name)s_ENABLED %(enabled)d',
> +                name=e.original.name.upper(),
> +                enabled=enabled)
> +        out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
> +
>      backend.generate_begin(events)
 
>      for e in events:
> diff --git a/trace/Makefile.objs b/trace/Makefile.objs
> index 4d91b3b..83f754e 100644
> --- a/trace/Makefile.objs
> +++ b/trace/Makefile.objs
> @@ -32,32 +32,11 @@ $(obj)/generated-ust.c-timestamp: 
> $(BUILD_DIR)/trace-events-all $(tracetool-y)
>               --backends=$(TRACE_BACKENDS) \
>               < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
> -$(obj)/generated-events.h: $(obj)/generated-ust-provider.h
> -$(obj)/generated-events.c: $(obj)/generated-ust.c
> +$(obj)/generated-tracers.h: $(obj)/generated-ust-provider.h
> +$(obj)/generated-tracers.c: $(obj)/generated-ust.c
 
>  endif
 
> -######################################################################
> -# Auto-generated event descriptions
> -
> -$(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
> -     @cmp $< $@ >/dev/null 2>&1 || cp $< $@
> -$(obj)/generated-events.h-timestamp: $(BUILD_DIR)/trace-events-all 
> $(tracetool-y)
> -     $(call quiet-command,$(TRACETOOL) \
> -             --format=events-h \
> -             --backends=$(TRACE_BACKENDS) \
> -             < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
> -
> -$(obj)/generated-events.c: $(obj)/generated-events.c-timestamp 
> $(BUILD_DIR)/config-host.mak
> -     @cmp $< $@ >/dev/null 2>&1 || cp $< $@
> -$(obj)/generated-events.c-timestamp: $(BUILD_DIR)/trace-events-all 
> $(tracetool-y)
> -     $(call quiet-command,$(TRACETOOL) \
> -             --format=events-c \
> -             --backends=$(TRACE_BACKENDS) \
> -             < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
> -
> -util-obj-y += generated-events.o
> -
 
You should remove files events_[ch].py too.


>  ######################################################################
>  # Auto-generated tracing routines
> @@ -154,7 +133,8 @@ $(obj)/generated-tcg-tracers.h-timestamp: 
> $(BUILD_DIR)/trace-events-all $(BUILD_
>  ######################################################################
>  # Backend code
 
> -util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o generated-tracers.o
> +util-obj-y += generated-tracers.o
> +util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
>  util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
>  util-obj-$(CONFIG_TRACE_UST) += generated-ust.o
>  util-obj-y += control.o
> diff --git a/trace/control.h b/trace/control.h
> index 60905f7..93efcbf 100644
> --- a/trace/control.h
> +++ b/trace/control.h
> @@ -11,7 +11,7 @@
>  #define TRACE__CONTROL_H
 
>  #include "qemu-common.h"
> -#include "trace/generated-events.h"
> +#include "event-internal.h"
 
>  typedef struct TraceEventIter {
>      size_t event;
> diff --git a/trace/simple.h b/trace/simple.h
> index 17ce472..9931808 100644
> --- a/trace/simple.h
> +++ b/trace/simple.h
> @@ -11,10 +11,6 @@
>  #ifndef TRACE_SIMPLE_H
>  #define TRACE_SIMPLE_H
 
> -
> -#include "trace/generated-events.h"
> -
> -
>  void st_print_trace_file_status(FILE *stream, fprintf_function 
> stream_printf);
>  void st_set_trace_file_enabled(bool enable);
>  void st_set_trace_file(const char *file);
> -- 
> 2.7.4