Re: [Qemu-devel] [PATCH v4 14/17] 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 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>

Other than the comment below:

Reviewed-by: Lluís Vilanova <address@hidden>


> ---
>  Makefile                             |  3 --
>  include/trace-tcg.h                  |  1 -
>  include/trace.h                      |  1 -
>  scripts/tracetool/format/c.py        | 50 ++++++++++++++++++++++++++---
>  scripts/tracetool/format/events_c.py | 62 
> ------------------------------------
>  scripts/tracetool/format/events_h.py | 49 ----------------------------
>  scripts/tracetool/format/h.py        | 20 ++++++++++++
>  trace/Makefile.objs                  | 28 +++-------------
>  trace/control.h                      |  2 +-
>  trace/simple.h                       |  4 ---
>  10 files changed, 70 insertions(+), 150 deletions(-)
>  delete mode 100644 scripts/tracetool/format/events_c.py
>  delete mode 100644 scripts/tracetool/format/events_h.py

> diff --git a/Makefile b/Makefile
> index 444ae37..ac52e81 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..f054d34 100644
> --- a/scripts/tracetool/format/c.py
> +++ b/scripts/tracetool/format/c.py
> @@ -17,12 +17,52 @@ 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 = e.api(e.QEMU_EVENT),
> +            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 = e.api(e.QEMU_EVENT))
> +
> +    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/events_c.py 
> b/scripts/tracetool/format/events_c.py
> deleted file mode 100644
> index cdfcfbc..0000000
> --- a/scripts/tracetool/format/events_c.py
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -#!/usr/bin/env python
> -# -*- coding: utf-8 -*-
> -
> -"""
> -trace/generated-events.c
> -"""
> -
> -__author__     = "Lluís Vilanova <address@hidden>"
> -__copyright__  = "Copyright 2012-2016, Lluís Vilanova <address@hidden>"
> -__license__    = "GPL version 2 or (at your option) any later version"
> -
> -__maintainer__ = "Stefan Hajnoczi"
> -__email__      = "address@hidden"
> -
> -
> -from tracetool import out
> -
> -
> -def generate(events, backend):
> -    out('/* This file is autogenerated by tracetool, do not edit. */',
> -        '',
> -        '#include "qemu/osdep.h"',
> -        '#include "trace.h"',
> -        '#include "trace/generated-events.h"',
> -        '#include "trace/control.h"',
> -        '')
> -
> -    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 = e.api(e.QEMU_EVENT),
> -            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 = e.api(e.QEMU_EVENT))
> -
> -    out('  NULL,',
> -        '};',
> -        '')
> -
> -    out('static void trace_register_events(void)',
> -        '{',
> -        '    trace_event_register_group(trace_events);',
> -        '}',
> -        'trace_init(trace_register_events)')
> diff --git a/scripts/tracetool/format/events_h.py 
> b/scripts/tracetool/format/events_h.py
> deleted file mode 100644
> index 1cb332b..0000000
> --- a/scripts/tracetool/format/events_h.py
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -#!/usr/bin/env python
> -# -*- coding: utf-8 -*-
> -
> -"""
> -trace/generated-events.h
> -"""
> -
> -__author__     = "Lluís Vilanova <address@hidden>"
> -__copyright__  = "Copyright 2012-2016, Lluís Vilanova <address@hidden>"
> -__license__    = "GPL version 2 or (at your option) any later version"
> -
> -__maintainer__ = "Stefan Hajnoczi"
> -__email__      = "address@hidden"
> -
> -
> -from tracetool import out
> -
> -
> -def generate(events, backend):
> -    out('/* This file is autogenerated by tracetool, do not edit. */',
> -        '',
> -        '#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))
> -
> -    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))
> -
> -    out('',
> -        '#endif  /* TRACE__GENERATED_EVENTS_H */')
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index 64a6680..f4748c3 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -26,6 +26,26 @@ def generate(events, backend):
>          '#include "trace/control.h"',
>          '')
 
> +    for e in events:
> +        out('extern TraceEvent %(event)s;',
> +            event = e.api(e.QEMU_EVENT))
> +
> +    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
> -
 
>  ######################################################################
>  # 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 feb5397..98bc8ec 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"

Please include "trace/event-internal.h" to make the full path explicit.

 
>  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