[Qemu-devel] [PATCH v3 0/8] Switch all subdirs over to modular trace.h file

[Qemu-devel] [PATCH v3 0/8] Switch all subdirs over to modular trace.h file

[Daniel P. Berrange]

This is the final part of my trace events refactoring.

Previously we merged code that split trace-events up
into one file per sub-directory. We also merged code
to the code-generator that enables us to generate
and use multiple independant sets of trace events.

This is the final glue that makes use of these two
earlier changes, so that we actually generate a
separate trace.h & trace.c file per sub-directory.

The result is that when adding a new trace event to
a file, only files in that sub-directory get rebuilt.
Theoretically the build is faster too, since each
source file includes a much smaller trace.h that
only contains probes relevant to it. In reality this
speed benefit is probably marginal & lost in the noise.

This patch series has massively changed since v2.

 - It no longer modifies the Makefile.objs rules in each
   sub-directory. A bit of magic in the top level
   Makefile.objs file lets us generate all the needed
   rules from one place

 - It sticks with bare "trace.h" includes instead of adding
   sub-dir prefix "hw/block/trace.h". To achieve this we
   had to rename the trace.h to trace-root.h in the top
   level directory to avoid ambiguity in places where a
   file needs to include the top-level instead of local
   trace.h

Daniel P. Berrange (8):
  make: move top level dir to end of include search path
  trace: move hw/block/dataplane events to correct subdir
  trace: move hw/xen events to correct subdir
  trace: move hw/i386/xen events to correct subdir
  trace: move setting of group name into Makefiles
  trace: switch to modular code generation for sub-directories
  trace: update docs to reflect new code generation approach
  trace: improve error reporting when parsing simpletrace header

 .gitignore                                |  22 +++--
 Makefile                                  | 156 ++++++++++++++++++++++++++----
 Makefile.objs                             |  99 ++++++++++---------
 Makefile.target                           |  10 +-
 aio-posix.c                               |   2 +-
 balloon.c                                 |   2 +-
 block.c                                   |   2 +-
 blockdev-nbd.c                            |   1 -
 blockdev.c                                |   2 +-
 blockjob.c                                |   1 -
 cpu-exec.c                                |   2 +-
 dma-helpers.c                             |   2 +-
 docs/tracing.txt                          |  55 ++++++++---
 exec.c                                    |   2 +-
 hw/block/dataplane/trace-events           |   6 ++
 hw/block/trace-events                     |   5 -
 hw/i386/trace-events                      |   7 --
 hw/i386/xen/trace-events                  |   6 ++
 hw/net/fsl_etsec/etsec.c                  |   1 -
 hw/xen/trace-events                       |  13 +++
 include/exec/cpu_ldst_template.h          |   2 +-
 include/exec/cpu_ldst_useronly_template.h |   2 +-
 include/hw/xen/xen_common.h               |   2 +-
 include/trace.h                           |   6 --
 ioport.c                                  |   2 +-
 kvm-all.c                                 |   2 +-
 memory.c                                  |   2 +-
 monitor.c                                 |   2 +-
 qom/cpu.c                                 |   2 +-
 rules.mak                                 |  17 ++--
 scripts/simpletrace.py                    |  10 +-
 scripts/tracetool.py                      |  31 +++---
 scripts/tracetool/backend/dtrace.py       |   7 +-
 scripts/tracetool/backend/simple.py       |   1 -
 scripts/tracetool/backend/ust.py          |   7 +-
 scripts/tracetool/format/c.py             |   7 +-
 scripts/tracetool/format/tcg_h.py         |   6 +-
 scripts/tracetool/format/tcg_helper_c.py  |   6 +-
 scripts/tracetool/format/ust_events_c.py  |   2 +-
 scripts/tracetool/format/ust_events_h.py  |   7 +-
 spice-qemu-char.c                         |   2 +-
 tests/Makefile.include                    |   2 +-
 thread-pool.c                             |   2 +-
 trace-events                              |  10 --
 trace/Makefile.objs                       |  93 ++----------------
 trace/control-target.c                    |   2 +-
 trace/control.c                           |   2 +-
 trace/ftrace.c                            |   2 +-
 trace/simple.c                            |   1 -
 translate-all.c                           |   2 +-
 vl.c                                      |   2 +-
 xen-hvm.c                                 |   2 +-
 xen-mapcache.c                            |   2 +-
 53 files changed, 385 insertions(+), 258 deletions(-)
 create mode 100644 hw/block/dataplane/trace-events
 create mode 100644 hw/i386/xen/trace-events
 create mode 100644 hw/xen/trace-events
 delete mode 100644 include/trace.h

-- 
2.9.3

[Qemu-devel] [PATCH v3 1/8] make: move top level dir to end of include search path

[Daniel P. Berrange]

Currently the search path is

  1. source dir corresponding to input file (implicit by compiler)
  2. top level build dir
  3. top level source dir
  4. top level source include/ dir
  5. source dir corresponding to input file
  6. build dir corresponding to output file

This causes a semantic difference in behaviour for builds
where srcdir == builddir vs srcdir != builddir, because
item 5 moves from end to start, when srcdir == builddir.

As a general rule we also want to move to have all shared
headers in the include/ dir, so move that ahead of the
top level dirs in the search order.

Thus we now have:

  1. source dir corresponding to input file
  2. build dir corresponding to output file
  3. top level build dir
  4. top level source dir
  5. top level source include/ dir

and items 1+2 and 4+5 collapse into a single dir when srcdir==builddir
so overall order remains the same.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 rules.mak | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/rules.mak b/rules.mak
index d5c516c..e09aabe 100644
--- a/rules.mak
+++ b/rules.mak
@@ -26,8 +26,10 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
 # Flags for dependency generation
 QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 
-# Same as -I$(SRC_PATH) -I., but for the nested source/object directories
-QEMU_INCLUDES += -I$(<D) -I$(@D)
+# Compiler searches the source file dir first, but in vpath builds
+# we need to make it search the build dir too, before any other
+# explicit search paths.
+QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D)
 
 WL_U := -Wl,-u,
 find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
@@ -61,7 +63,7 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \
                   $(filter-out %.o %.mo,$1))
 
 %.o: %.c
-	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
+	$(call quiet-command,$(CC) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
 %.o: %.rc
 	$(call quiet-command,$(WINDRES) -I. -o $@ $<,"RC","$(TARGET_DIR)$@")
 
@@ -74,16 +76,16 @@ LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o
        $(version-obj-y) $(call extract-libs,$1) $(LIBS),"LINK","$(TARGET_DIR)$@")
 
 %.o: %.S
-	$(call quiet-command,$(CCAS) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
+	$(call quiet-command,$(CCAS) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
 
 %.o: %.cc
-	$(call quiet-command,$(CXX) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CXX","$(TARGET_DIR)$@")
+	$(call quiet-command,$(CXX) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CXX","$(TARGET_DIR)$@")
 
 %.o: %.cpp
-	$(call quiet-command,$(CXX) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CXX","$(TARGET_DIR)$@")
+	$(call quiet-command,$(CXX) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CXX","$(TARGET_DIR)$@")
 
 %.o: %.m
-	$(call quiet-command,$(OBJCC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"OBJC","$(TARGET_DIR)$@")
+	$(call quiet-command,$(OBJCC) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"OBJC","$(TARGET_DIR)$@")
 
 %.o: %.dtrace
 	$(call quiet-command,dtrace -o $@ -G -s $<,"GEN","$(TARGET_DIR)$@")
@@ -359,6 +361,7 @@ define unnest-vars
                 $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)),
                 $(error $o added in $v but $o-objs is not set)))
         $(shell mkdir -p ./ $(sort $(dir $($v))))
+        $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v))))
         # Include all the .d files
         $(eval -include $(patsubst %.o,%.d,$(patsubst %.mo,%.d,$($v))))
         $(eval $v := $(filter-out %/,$($v))))
-- 
2.9.3

[Qemu-devel] [PATCH v3 2/8] trace: move hw/block/dataplane events to correct subdir

[Daniel P. Berrange]

The trace-events for a given source file should generally
always live in the same directory as the source file.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 Makefile.objs                   | 1 +
 hw/block/dataplane/trace-events | 6 ++++++
 hw/block/trace-events           | 5 -----
 3 files changed, 7 insertions(+), 5 deletions(-)
 create mode 100644 hw/block/dataplane/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index 01cef86..babbc4e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -125,6 +125,7 @@ trace-events-y += io/trace-events
 trace-events-y += migration/trace-events
 trace-events-y += block/trace-events
 trace-events-y += hw/block/trace-events
+trace-events-y += hw/block/dataplane/trace-events
 trace-events-y += hw/char/trace-events
 trace-events-y += hw/intc/trace-events
 trace-events-y += hw/net/trace-events
diff --git a/hw/block/dataplane/trace-events b/hw/block/dataplane/trace-events
new file mode 100644
index 0000000..13f5dbb
--- /dev/null
+++ b/hw/block/dataplane/trace-events
@@ -0,0 +1,6 @@
+# See docs/tracing.txt for syntax documentation.
+
+# hw/block/dataplane/virtio-blk.c
+virtio_blk_data_plane_start(void *s) "dataplane %p"
+virtio_blk_data_plane_stop(void *s) "dataplane %p"
+virtio_blk_data_plane_process_request(void *s, unsigned int out_num, unsigned int in_num, unsigned int head) "dataplane %p out_num %u in_num %u head %u"
diff --git a/hw/block/trace-events b/hw/block/trace-events
index d0dd94f..65e83dc 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -7,11 +7,6 @@ virtio_blk_handle_write(void *req, uint64_t sector, size_t nsectors) "req %p sec
 virtio_blk_handle_read(void *req, uint64_t sector, size_t nsectors) "req %p sector %"PRIu64" nsectors %zu"
 virtio_blk_submit_multireq(void *mrb, int start, int num_reqs, uint64_t offset, size_t size, bool is_write) "mrb %p start %d num_reqs %d offset %"PRIu64" size %zu is_write %d"
 
-# hw/block/dataplane/virtio-blk.c
-virtio_blk_data_plane_start(void *s) "dataplane %p"
-virtio_blk_data_plane_stop(void *s) "dataplane %p"
-virtio_blk_data_plane_process_request(void *s, unsigned int out_num, unsigned int in_num, unsigned int head) "dataplane %p out_num %u in_num %u head %u"
-
 # hw/block/hd-geometry.c
 hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p LCHS %d %d %d"
 hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "blk %p CHS %u %u %u trans %d"
-- 
2.9.3

[Qemu-devel] [PATCH v3 3/8] trace: move hw/xen events to correct subdir

[Daniel P. Berrange]

The trace-events for a given source file should generally
always live in the same directory as the source file.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 Makefile.objs       |  1 +
 hw/xen/trace-events | 13 +++++++++++++
 trace-events        | 10 ----------
 3 files changed, 14 insertions(+), 10 deletions(-)
 create mode 100644 hw/xen/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index babbc4e..9ac11e7 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -152,6 +152,7 @@ trace-events-y += hw/vfio/trace-events
 trace-events-y += hw/acpi/trace-events
 trace-events-y += hw/arm/trace-events
 trace-events-y += hw/alpha/trace-events
+trace-events-y += hw/xen/trace-events
 trace-events-y += ui/trace-events
 trace-events-y += audio/trace-events
 trace-events-y += net/trace-events
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
new file mode 100644
index 0000000..c4fb6f1
--- /dev/null
+++ b/hw/xen/trace-events
@@ -0,0 +1,13 @@
+# See docs/tracing.txt for syntax documentation.
+
+# include/hw/xen/xen_common.h
+xen_default_ioreq_server(void) ""
+xen_ioreq_server_create(uint32_t id) "id: %u"
+xen_ioreq_server_destroy(uint32_t id) "id: %u"
+xen_ioreq_server_state(uint32_t id, bool enable) "id: %u: enable: %i"
+xen_map_mmio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_unmap_mmio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_map_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_unmap_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_map_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u bdf: %02x.%02x.%02x"
+xen_unmap_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u bdf: %02x.%02x.%02x"
diff --git a/trace-events b/trace-events
index 839a9d0..05ac6ac 100644
--- a/trace-events
+++ b/trace-events
@@ -62,16 +62,6 @@ spice_vmc_event(int event) "spice vmc event %d"
 # xen-hvm.c
 xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: %#lx, size %#lx"
 xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "%#"PRIx64" size %#lx, log_dirty %i"
-xen_default_ioreq_server(void) ""
-xen_ioreq_server_create(uint32_t id) "id: %u"
-xen_ioreq_server_destroy(uint32_t id) "id: %u"
-xen_ioreq_server_state(uint32_t id, bool enable) "id: %u: enable: %i"
-xen_map_mmio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
-xen_unmap_mmio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
-xen_map_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
-xen_unmap_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
-xen_map_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u bdf: %02x.%02x.%02x"
-xen_unmap_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u bdf: %02x.%02x.%02x"
 handle_ioreq(void *req, uint32_t type, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p type=%d dir=%d df=%d ptr=%d port=%#"PRIx64" data=%#"PRIx64" count=%d size=%d"
 handle_ioreq_read(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p read type=%d df=%d ptr=%d port=%#"PRIx64" data=%#"PRIx64" count=%d size=%d"
 handle_ioreq_write(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p write type=%d df=%d ptr=%d port=%#"PRIx64" data=%#"PRIx64" count=%d size=%d"
-- 
2.9.3

[Qemu-devel] [PATCH v3 4/8] trace: move hw/i386/xen events to correct subdir

[Daniel P. Berrange]

The trace-events for a given source file should generally
always live in the same directory as the source file.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 Makefile.objs            | 1 +
 hw/i386/trace-events     | 7 -------
 hw/i386/xen/trace-events | 6 ++++++
 3 files changed, 7 insertions(+), 7 deletions(-)
 create mode 100644 hw/i386/xen/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index 9ac11e7..6abe0b9 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -144,6 +144,7 @@ trace-events-y += hw/sd/trace-events
 trace-events-y += hw/isa/trace-events
 trace-events-y += hw/mem/trace-events
 trace-events-y += hw/i386/trace-events
+trace-events-y += hw/i386/xen/trace-events
 trace-events-y += hw/9pfs/trace-events
 trace-events-y += hw/ppc/trace-events
 trace-events-y += hw/pci/trace-events
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index d2b4973..a3568ee 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -1,12 +1,5 @@
 # See docs/tracing.txt for syntax documentation.
 
-# hw/i386/xen/xen_platform.c
-xen_platform_log(char *s) "xen platform: %s"
-
-# hw/i386/xen/xen_pvdevice.c
-xen_pv_mmio_read(uint64_t addr) "WARNING: read from Xen PV Device MMIO space (address %"PRIx64")"
-xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (address %"PRIx64")"
-
 # hw/i386/x86-iommu.c
 x86_iommu_iec_notify(bool global, uint32_t index, uint32_t mask) "Notify IEC invalidation: global=%d index=%" PRIu32 " mask=%" PRIu32
 
diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events
new file mode 100644
index 0000000..321fe60
--- /dev/null
+++ b/hw/i386/xen/trace-events
@@ -0,0 +1,6 @@
+# hw/i386/xen/xen_platform.c
+xen_platform_log(char *s) "xen platform: %s"
+
+# hw/i386/xen/xen_pvdevice.c
+xen_pv_mmio_read(uint64_t addr) "WARNING: read from Xen PV Device MMIO space (address %"PRIx64")"
+xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (address %"PRIx64")"
-- 
2.9.3

[Qemu-devel] [PATCH v3 5/8] trace: move setting of group name into Makefiles

[Daniel P. Berrange]

Having tracetool.py figure out the right group name from just
the input filename is not practical when considering the
different build vs src path combinations. Instead simply take
the group name as a command line arg from the Makefile, which
can trivially provide the right name.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 Makefile.target      |  3 +++
 scripts/tracetool.py | 23 +++++++++--------------
 trace/Makefile.objs  |  9 +++++++++
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index fa2b151..fa6ae0c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -50,6 +50,7 @@ endif
 
 $(QEMU_PROG).stp-installed: $(BUILD_DIR)/trace-events-all
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=stap \
 		--backends=$(TRACE_BACKENDS) \
 		--binary=$(bindir)/$(QEMU_PROG) \
@@ -59,6 +60,7 @@ $(QEMU_PROG).stp-installed: $(BUILD_DIR)/trace-events-all
 
 $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=stap \
 		--backends=$(TRACE_BACKENDS) \
 		--binary=$(realpath .)/$(QEMU_PROG) \
@@ -68,6 +70,7 @@ $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
 
 $(QEMU_PROG)-simpletrace.stp: $(BUILD_DIR)/trace-events-all
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=simpletrace-stap \
 		--backends=$(TRACE_BACKENDS) \
 		--probe-prefix=qemu.$(TARGET_TYPE).$(TARGET_NAME) \
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index c9e4737..0c9d992 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -49,6 +49,7 @@ Options:
     --binary <path>          Full path to QEMU binary.
     --target-type <type>     QEMU emulator target type ('system' or 'user').
     --target-name <name>     QEMU emulator target name.
+    --group <name>           Name of the event group
     --probe-prefix <prefix>  Prefix for dtrace probe names
                              (default: qemu-<target-type>-<target-name>).\
 """ % {
@@ -62,22 +63,12 @@ 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"[^A-Za-z0-9]", "_", dirname)
-
 def main(args):
     global _SCRIPT
     _SCRIPT = args[0]
 
     long_opts = ["backends=", "format=", "help", "list-backends",
-                 "check-backends"]
+                 "check-backends", "group="]
     long_opts += ["binary=", "target-type=", "target-name=", "probe-prefix="]
 
     try:
@@ -88,6 +79,7 @@ def main(args):
     check_backends = False
     arg_backends = []
     arg_format = ""
+    arg_group = None
     binary = None
     target_type = None
     target_name = None
@@ -98,6 +90,8 @@ def main(args):
 
         elif opt == "--backends":
             arg_backends = arg.split(",")
+        elif opt == "--group":
+            arg_group = arg
         elif opt == "--format":
             arg_format = arg
 
@@ -129,6 +123,9 @@ def main(args):
                 sys.exit(1)
         sys.exit(0)
 
+    if arg_group is None:
+        error_opt("group name is required")
+
     if arg_format == "stap":
         if binary is None:
             error_opt("--binary is required for SystemTAP tapset generator")
@@ -145,10 +142,8 @@ 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, group, arg_format, arg_backends,
+        tracetool.generate(events, arg_group, arg_format, arg_backends,
                            binary=binary, probe_prefix=probe_prefix)
     except tracetool.TracetoolError as e:
         error_opt(str(e))
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 1e1ce74..d3b47da 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -20,6 +20,7 @@ $(obj)/generated-ust-provider.h: $(obj)/generated-ust-provider.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-ust-provider.h-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=ust-events-h \
 		--backends=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -28,6 +29,7 @@ $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp $(BUILD_DIR)/config-hos
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-ust.c-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=ust-events-c \
 		--backends=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -48,6 +50,7 @@ $(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
 	@cmp -s $< $@ || cp $< $@
 $(obj)/generated-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=h \
 		--backends=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -59,6 +62,7 @@ $(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
 	@cmp -s $< $@ || cp $< $@
 $(obj)/generated-tracers.c-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=c \
 		--backends=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -77,6 +81,7 @@ $(obj)/generated-tracers-dtrace.dtrace: $(obj)/generated-tracers-dtrace.dtrace-t
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-tracers-dtrace.dtrace-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=d \
 		--backends=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -96,6 +101,7 @@ $(obj)/generated-helpers-wrappers.h: $(obj)/generated-helpers-wrappers.h-timesta
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers-wrappers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=tcg-helper-wrapper-h \
 		--backend=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -104,6 +110,7 @@ $(obj)/generated-helpers.h: $(obj)/generated-helpers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=tcg-helper-h \
 		--backend=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -112,6 +119,7 @@ $(obj)/generated-helpers.c: $(obj)/generated-helpers.c-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers.c-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=tcg-helper-c \
 		--backend=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
@@ -125,6 +133,7 @@ $(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-tcg-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
 		--format=tcg-h \
 		--backend=$(TRACE_BACKENDS) \
 		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-- 
2.9.3

[Qemu-devel] [PATCH v3 6/8] trace: switch to modular code generation for sub-directories

[Daniel P. Berrange]

Introduce rules in the top level Makefile that are able to generate
trace.[ch] files in every subdirectory which has a trace-events file.

The top level directory is handled specially, so instead of creating
trace.h, it creates trace-root.h. This allows sub-directories to
include the top level trace-root.h file, without ambiguity wrt to
the trace.g file in the current sub-dir.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 .gitignore                                |  22 +++--
 Makefile                                  | 156 ++++++++++++++++++++++++++----
 Makefile.objs                             | 102 ++++++++++---------
 Makefile.target                           |   7 +-
 aio-posix.c                               |   2 +-
 balloon.c                                 |   2 +-
 block.c                                   |   2 +-
 blockdev-nbd.c                            |   1 -
 blockdev.c                                |   2 +-
 blockjob.c                                |   1 -
 cpu-exec.c                                |   2 +-
 dma-helpers.c                             |   2 +-
 exec.c                                    |   2 +-
 hw/net/fsl_etsec/etsec.c                  |   1 -
 include/exec/cpu_ldst_template.h          |   2 +-
 include/exec/cpu_ldst_useronly_template.h |   2 +-
 include/hw/xen/xen_common.h               |   2 +-
 include/trace.h                           |   6 --
 ioport.c                                  |   2 +-
 kvm-all.c                                 |   2 +-
 memory.c                                  |   2 +-
 monitor.c                                 |   2 +-
 qom/cpu.c                                 |   2 +-
 scripts/tracetool.py                      |   8 +-
 scripts/tracetool/backend/dtrace.py       |   7 +-
 scripts/tracetool/backend/simple.py       |   1 -
 scripts/tracetool/backend/ust.py          |   7 +-
 scripts/tracetool/format/c.py             |   7 +-
 scripts/tracetool/format/tcg_h.py         |   6 +-
 scripts/tracetool/format/tcg_helper_c.py  |   6 +-
 scripts/tracetool/format/ust_events_c.py  |   2 +-
 scripts/tracetool/format/ust_events_h.py  |   7 +-
 spice-qemu-char.c                         |   2 +-
 tests/Makefile.include                    |   2 +-
 thread-pool.c                             |   2 +-
 trace/Makefile.objs                       |  94 +-----------------
 trace/control-target.c                    |   2 +-
 trace/control.c                           |   2 +-
 trace/ftrace.c                            |   2 +-
 trace/simple.c                            |   1 -
 translate-all.c                           |   2 +-
 vl.c                                      |   2 +-
 xen-hvm.c                                 |   2 +-
 xen-mapcache.c                            |   2 +-
 44 files changed, 283 insertions(+), 209 deletions(-)
 delete mode 100644 include/trace.h

diff --git a/.gitignore b/.gitignore
index 78f180a..c563dc1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,18 +6,12 @@
 /config.status
 /config-temp
 /trace-events-all
-/trace/generated-tracers.h
-/trace/generated-tracers.c
-/trace/generated-tracers-dtrace.h
-/trace/generated-tracers.dtrace
 /trace/generated-events.h
 /trace/generated-events.c
 /trace/generated-helpers-wrappers.h
 /trace/generated-helpers.h
 /trace/generated-helpers.c
 /trace/generated-tcg-tracers.h
-/trace/generated-ust-provider.h
-/trace/generated-ust.c
 /ui/shader/texture-blit-frag.h
 /ui/shader/texture-blit-vert.h
 *-timestamp
@@ -120,3 +114,19 @@ tags
 TAGS
 docker-src.*
 *~
+trace.h
+trace.c
+trace-ust.h
+trace-ust.h
+trace-dtrace.h
+trace-dtrace.dtrace
+trace-root.h
+trace-root.c
+trace-ust-root.h
+trace-ust-root.h
+trace-ust-all.h
+trace-ust-all.c
+trace-dtrace-root.h
+trace-dtrace-root.dtrace
+trace-ust-all.h
+trace-ust-all.c
diff --git a/Makefile b/Makefile
index 9f8968d..a8fa28a 100644
--- a/Makefile
+++ b/Makefile
@@ -56,25 +56,136 @@ 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-tracers.h
-ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
-GENERATED_HEADERS += trace/generated-tracers-dtrace.h
-endif
-GENERATED_SOURCES += trace/generated-tracers.c
-
 GENERATED_HEADERS += trace/generated-tcg-tracers.h
 
 GENERATED_HEADERS += trace/generated-helpers-wrappers.h
 GENERATED_HEADERS += trace/generated-helpers.h
 GENERATED_SOURCES += trace/generated-helpers.c
 
-ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
-GENERATED_HEADERS += trace/generated-ust-provider.h
-GENERATED_SOURCES += trace/generated-ust.c
+ifdef CONFIG_TRACE_UST
+GENERATED_HEADERS += trace-ust-all.h
+GENERATED_SOURCES += trace-ust-all.c
 endif
 
 GENERATED_HEADERS += module_block.h
 
+TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
+TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
+TRACE_DTRACE =
+ifdef CONFIG_TRACE_DTRACE
+TRACE_HEADERS += trace-dtrace-root.h $(trace-events-subdirs:%=%/trace-dtrace.h)
+TRACE_DTRACE += trace-dtrace-root.dtrace $(trace-events-subdirs:%=%/trace-dtrace.dtrace)
+endif
+ifdef CONFIG_TRACE_UST
+TRACE_HEADERS += trace-ust-root.h $(trace-events-subdirs:%=%/trace-ust.h)
+endif
+
+GENERATED_HEADERS += $(TRACE_HEADERS)
+GENERATED_SOURCES += $(TRACE_SOURCES)
+
+trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
+
+%/trace.h: %/trace.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+%/trace.h-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=$(call trace-group-name,$@) \
+		--format=h \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+%/trace.c: %/trace.c-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+%/trace.c-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=$(call trace-group-name,$@) \
+		--format=c \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+%/trace-ust.h: %/trace-ust.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+%/trace-ust.h-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=$(call trace-group-name,$@) \
+		--format=ust-events-h \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+%/trace-dtrace.dtrace: %/trace-dtrace.dtrace-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+%/trace-dtrace.dtrace-timestamp: $(SRC_PATH)/%/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=$(call trace-group-name,$@) \
+		--format=d \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+%/trace-dtrace.h: %/trace-dtrace.dtrace $(tracetool-y)
+	$(call quiet-command,dtrace -o $@ -h -s $<, "GEN","$@")
+
+%/trace-dtrace.o: %/trace-dtrace.dtrace $(tracetool-y)
+
+
+trace-root.h: trace-root.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+trace-root.h-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=root \
+		--format=h \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+trace-root.c: trace-root.c-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+trace-root.c-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=root \
+		--format=c \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+trace-ust-root.h: trace-ust-root.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+trace-ust-root.h-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=root \
+		--format=ust-events-h \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+trace-ust-all.h: trace-ust-all.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+trace-ust-all.h-timestamp: $(trace-events-files) $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
+		--format=ust-events-h \
+		--backends=$(TRACE_BACKENDS) \
+		$(trace-events-files) > $@,"GEN","$(@:%-timestamp=%)")
+
+trace-ust-all.c: trace-ust-all.c-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+trace-ust-all.c-timestamp: $(trace-events-files) $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=all \
+		--format=ust-events-c \
+		--backends=$(TRACE_BACKENDS) \
+		$(trace-events-files) > $@,"GEN","$(@:%-timestamp=%)")
+
+trace-dtrace-root.dtrace: trace-dtrace-root.dtrace-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+trace-dtrace-root.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+	$(call quiet-command,$(TRACETOOL) \
+		--group=root \
+		--format=d \
+		--backends=$(TRACE_BACKENDS) \
+		$< > $@,"GEN","$(@:%-timestamp=%)")
+
+trace-dtrace-root.h: trace-dtrace-root.dtrace
+	$(call quiet-command,dtrace -o $@ -h -s $<, "GEN","$@")
+
+trace-dtrace-root.o: trace-dtrace-root.dtrace
+
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
 Makefile: ;
@@ -160,7 +271,8 @@ dummy := $(call unnest-vars,, \
                 qom-obj-y \
                 io-obj-y \
                 common-obj-y \
-                common-obj-m)
+                common-obj-m \
+                trace-obj-y)
 
 ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/tests/Makefile.include
@@ -223,7 +335,7 @@ subdir-dtc:dtc/libfdt dtc/tests
 dtc/%:
 	mkdir -p $@
 
-$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
+$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) $(trace-obj-y)
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 # Only keep -O and -g cflags
@@ -247,15 +359,17 @@ libqemuutil.a: $(util-obj-y)
 
 ######################################################################
 
+COMMON_LDADDS = $(trace-obj-y) libqemuutil.a libqemustub.a
+
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a
-qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a
-qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a
+qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
+qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
+qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
 
-qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o libqemuutil.a libqemustub.a
+qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
 
-fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/9p-marshal.o fsdev/9p-iov-marshal.o libqemuutil.a libqemustub.a
+fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS)
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
@@ -320,7 +434,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
+qemu-ga$(EXESUF): $(qga-obj-y) $(COMMON_LDADDS)
 	$(call LINK, $^)
 
 ifdef QEMU_GA_MSI_ENABLED
@@ -345,9 +459,9 @@ ifneq ($(EXESUF),)
 qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
 endif
 
-ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) libqemuutil.a libqemustub.a
+ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) $(COMMON_LDADDS)
 	$(call LINK, $^)
-ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) libqemuutil.a libqemustub.a
+ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) $(COMMON_LDADDS)
 	$(call LINK, $^)
 
 module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
@@ -664,6 +778,10 @@ ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fa
 Makefile: $(GENERATED_HEADERS)
 endif
 
+.SECONDARY: $(TRACE_HEADERS) $(TRACE_HEADERS:%=%-timestamp) \
+	$(TRACE_SOURCES) $(TRACE_SOURCES:%=%-timestamp) \
+	$(TRACE_DTRACE) $(TRACE_DTRACE:%=%-timestamp)
+
 # Include automatically generated dependency files
 # Dependencies in Makefile.objs files come from our recursive subdir rules
 -include $(wildcard *.d tests/*.d)
diff --git a/Makefile.objs b/Makefile.objs
index 6abe0b9..cf2f387 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -118,50 +118,58 @@ ivshmem-server-obj-y = contrib/ivshmem-server/
 libvhost-user-obj-y = contrib/libvhost-user/
 
 ######################################################################
-trace-events-y = trace-events
-trace-events-y += util/trace-events
-trace-events-y += crypto/trace-events
-trace-events-y += io/trace-events
-trace-events-y += migration/trace-events
-trace-events-y += block/trace-events
-trace-events-y += hw/block/trace-events
-trace-events-y += hw/block/dataplane/trace-events
-trace-events-y += hw/char/trace-events
-trace-events-y += hw/intc/trace-events
-trace-events-y += hw/net/trace-events
-trace-events-y += hw/virtio/trace-events
-trace-events-y += hw/audio/trace-events
-trace-events-y += hw/misc/trace-events
-trace-events-y += hw/usb/trace-events
-trace-events-y += hw/scsi/trace-events
-trace-events-y += hw/nvram/trace-events
-trace-events-y += hw/display/trace-events
-trace-events-y += hw/input/trace-events
-trace-events-y += hw/timer/trace-events
-trace-events-y += hw/dma/trace-events
-trace-events-y += hw/sparc/trace-events
-trace-events-y += hw/sd/trace-events
-trace-events-y += hw/isa/trace-events
-trace-events-y += hw/mem/trace-events
-trace-events-y += hw/i386/trace-events
-trace-events-y += hw/i386/xen/trace-events
-trace-events-y += hw/9pfs/trace-events
-trace-events-y += hw/ppc/trace-events
-trace-events-y += hw/pci/trace-events
-trace-events-y += hw/s390x/trace-events
-trace-events-y += hw/vfio/trace-events
-trace-events-y += hw/acpi/trace-events
-trace-events-y += hw/arm/trace-events
-trace-events-y += hw/alpha/trace-events
-trace-events-y += hw/xen/trace-events
-trace-events-y += ui/trace-events
-trace-events-y += audio/trace-events
-trace-events-y += net/trace-events
-trace-events-y += target/arm/trace-events
-trace-events-y += target/i386/trace-events
-trace-events-y += target/sparc/trace-events
-trace-events-y += target/s390x/trace-events
-trace-events-y += target/ppc/trace-events
-trace-events-y += qom/trace-events
-trace-events-y += linux-user/trace-events
-trace-events-y += qapi/trace-events
+trace-events-subdirs =
+trace-events-subdirs += util
+trace-events-subdirs += crypto
+trace-events-subdirs += io
+trace-events-subdirs += migration
+trace-events-subdirs += block
+trace-events-subdirs += hw/block
+trace-events-subdirs += hw/block/dataplane
+trace-events-subdirs += hw/char
+trace-events-subdirs += hw/intc
+trace-events-subdirs += hw/net
+trace-events-subdirs += hw/virtio
+trace-events-subdirs += hw/audio
+trace-events-subdirs += hw/misc
+trace-events-subdirs += hw/usb
+trace-events-subdirs += hw/scsi
+trace-events-subdirs += hw/nvram
+trace-events-subdirs += hw/display
+trace-events-subdirs += hw/input
+trace-events-subdirs += hw/timer
+trace-events-subdirs += hw/dma
+trace-events-subdirs += hw/sparc
+trace-events-subdirs += hw/sd
+trace-events-subdirs += hw/isa
+trace-events-subdirs += hw/mem
+trace-events-subdirs += hw/i386
+trace-events-subdirs += hw/i386/xen
+trace-events-subdirs += hw/9pfs
+trace-events-subdirs += hw/ppc
+trace-events-subdirs += hw/pci
+trace-events-subdirs += hw/s390x
+trace-events-subdirs += hw/vfio
+trace-events-subdirs += hw/acpi
+trace-events-subdirs += hw/arm
+trace-events-subdirs += hw/alpha
+trace-events-subdirs += hw/xen
+trace-events-subdirs += ui
+trace-events-subdirs += audio
+trace-events-subdirs += net
+trace-events-subdirs += target/arm
+trace-events-subdirs += target/i386
+trace-events-subdirs += target/sparc
+trace-events-subdirs += target/s390x
+trace-events-subdirs += target/ppc
+trace-events-subdirs += qom
+trace-events-subdirs += linux-user
+trace-events-subdirs += qapi
+
+trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events)
+
+trace-obj-y = trace-root.o
+trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
+trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
+trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
+trace-obj-$(CONFIG_TRACE_DTRACE) += $(trace-events-subdirs:%=%/trace-dtrace.o)
diff --git a/Makefile.target b/Makefile.target
index fa6ae0c..9f2af5b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -186,7 +186,8 @@ dummy := $(call unnest-vars,.., \
                qom-obj-y \
                io-obj-y \
                common-obj-y \
-               common-obj-m)
+               common-obj-m \
+               trace-obj-y)
 target-obj-y := $(target-obj-y-save)
 all-obj-y += $(common-obj-y)
 all-obj-y += $(target-obj-y)
@@ -198,8 +199,10 @@ all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
 $(QEMU_PROG_BUILD): config-devices.mak
 
+COMMON_LDADDS = $(trace-obj-y) ../libqemuutil.a ../libqemustub.a
+
 # build either PROG or PROGW
-$(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
+$(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS)
 	$(call LINK, $(filter-out %.mak, $^))
 ifdef CONFIG_DARWIN
 	$(call quiet-command,Rez -append $(SRC_PATH)/pc-bios/qemu.rsrc -o $@,"REZ","$(TARGET_DIR)$@")
diff --git a/aio-posix.c b/aio-posix.c
index 9453d83..804e03d 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -19,7 +19,7 @@
 #include "qemu/rcu_queue.h"
 #include "qemu/sockets.h"
 #include "qemu/cutils.h"
-#include "trace.h"
+#include "trace-root.h"
 #ifdef CONFIG_EPOLL_CREATE1
 #include <sys/epoll.h>
 #endif
diff --git a/balloon.c b/balloon.c
index f2ef50c..1d720ff 100644
--- a/balloon.c
+++ b/balloon.c
@@ -29,7 +29,7 @@
 #include "exec/cpu-common.h"
 #include "sysemu/kvm.h"
 #include "sysemu/balloon.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "qmp-commands.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
diff --git a/block.c b/block.c
index 39ddea3..19b68f8 100644
--- a/block.c
+++ b/block.c
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
-#include "trace.h"
+#include "block/trace.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "block/nbd.h"
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 81bca17..7ea836b 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -16,7 +16,6 @@
 #include "qapi/qmp/qerror.h"
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
-#include "trace.h"
 #include "block/nbd.h"
 #include "io/channel-socket.h"
 
diff --git a/blockdev.c b/blockdev.c
index 245e1e1..db82ac9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -48,7 +48,7 @@
 #include "sysemu/sysemu.h"
 #include "block/block_int.h"
 #include "qmp-commands.h"
-#include "trace.h"
+#include "block/trace.h"
 #include "sysemu/arch_init.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
diff --git a/blockjob.c b/blockjob.c
index 513620c..abee11b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -25,7 +25,6 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include "trace.h"
 #include "block/block.h"
 #include "block/blockjob_int.h"
 #include "block/block_int.h"
diff --git a/cpu-exec.c b/cpu-exec.c
index 4188fed..8057432 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -18,7 +18,7 @@
  */
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "disas/disas.h"
 #include "exec/exec-all.h"
 #include "tcg.h"
diff --git a/dma-helpers.c b/dma-helpers.c
index 6f9d47c..97157cc 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -10,7 +10,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/dma.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "qemu/thread.h"
 #include "qemu/main-loop.h"
 
diff --git a/exec.c b/exec.c
index f2bed92..f0d10a6 100644
--- a/exec.c
+++ b/exec.c
@@ -44,7 +44,7 @@
 #include "sysemu/dma.h"
 #include "exec/address-spaces.h"
 #include "sysemu/xen-mapcache.h"
-#include "trace.h"
+#include "trace-root.h"
 #endif
 #include "exec/cpu-all.h"
 #include "qemu/rcu_queue.h"
diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index fadf9c8..aa2b0d5 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -29,7 +29,6 @@
 #include "qemu/osdep.h"
 #include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
-#include "trace.h"
 #include "hw/ptimer.h"
 #include "etsec.h"
 #include "registers.h"
diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h
index eaf69a1..4db2302 100644
--- a/include/exec/cpu_ldst_template.h
+++ b/include/exec/cpu_ldst_template.h
@@ -25,7 +25,7 @@
  */
 
 #if !defined(SOFTMMU_CODE_ACCESS)
-#include "trace.h"
+#include "trace-root.h"
 #endif
 
 #include "trace/mem.h"
diff --git a/include/exec/cpu_ldst_useronly_template.h b/include/exec/cpu_ldst_useronly_template.h
index b1378bf..7b8c7c5 100644
--- a/include/exec/cpu_ldst_useronly_template.h
+++ b/include/exec/cpu_ldst_useronly_template.h
@@ -24,7 +24,7 @@
  */
 
 #if !defined(CODE_ACCESS)
-#include "trace.h"
+#include "trace-root.h"
 #endif
 
 #include "trace/mem.h"
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 8e1580d..dce76ee 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -18,7 +18,7 @@
 #include "hw/xen/xen.h"
 #include "hw/pci/pci.h"
 #include "qemu/queue.h"
-#include "trace.h"
+#include "hw/xen/trace.h"
 
 /*
  * We don't support Xen prior to 4.2.0.
diff --git a/include/trace.h b/include/trace.h
deleted file mode 100644
index ac9ff3d..0000000
--- a/include/trace.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef TRACE_H
-#define TRACE_H
-
-#include "trace/generated-tracers.h"
-
-#endif /* TRACE_H */
diff --git a/ioport.c b/ioport.c
index 94e08ab..1a65add 100644
--- a/ioport.c
+++ b/ioport.c
@@ -29,7 +29,7 @@
 #include "qemu-common.h"
 #include "cpu.h"
 #include "exec/ioport.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 
diff --git a/kvm-all.c b/kvm-all.c
index 330219e..a27c880 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -34,7 +34,7 @@
 #include "exec/ram_addr.h"
 #include "exec/address-spaces.h"
 #include "qemu/event_notifier.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "hw/irq.h"
 
 #include "hw/boards.h"
diff --git a/memory.c b/memory.c
index 2bfc37f..5c27200 100644
--- a/memory.c
+++ b/memory.c
@@ -24,7 +24,7 @@
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "qom/object.h"
-#include "trace.h"
+#include "trace-root.h"
 
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
diff --git a/monitor.c b/monitor.c
index 8b06b63..e372559 100644
--- a/monitor.c
+++ b/monitor.c
@@ -59,7 +59,7 @@
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
 #include "qom/object_interfaces.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "trace/control.h"
 #include "monitor/hmp-target.h"
 #ifdef CONFIG_TRACE_SIMPLE
diff --git a/qom/cpu.c b/qom/cpu.c
index 7f57587..41d0c17 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -29,7 +29,7 @@
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/qdev-properties.h"
-#include "trace.h"
+#include "trace-root.h"
 
 bool cpu_exists(int64_t id)
 {
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 0c9d992..c55a215 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -137,10 +137,12 @@ def main(args):
         if probe_prefix is None:
             probe_prefix = ".".join(["qemu", target_type, target_name])
 
-    if len(args) != 1:
+    if len(args) < 1:
         error_opt("missing trace-events filepath")
-    with open(args[0], "r") as fh:
-        events = tracetool.read_events(fh)
+    events = []
+    for arg in args:
+        with open(arg, "r") as fh:
+            events.extend(tracetool.read_events(fh))
 
     try:
         tracetool.generate(events, arg_group, arg_format, arg_backends,
diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
index 79505c6..c469cbd 100644
--- a/scripts/tracetool/backend/dtrace.py
+++ b/scripts/tracetool/backend/dtrace.py
@@ -36,7 +36,12 @@ def binary():
 
 
 def generate_h_begin(events, group):
-    out('#include "trace/generated-tracers-dtrace.h"',
+    if group == "root":
+        header = "trace-dtrace-root.h"
+    else:
+        header = "trace-dtrace.h"
+
+    out('#include "%s"' % header,
         '')
 
 
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 85f6102..4acc06e 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -44,7 +44,6 @@ def generate_h(event, group):
 
 def generate_c_begin(events, group):
     out('#include "qemu/osdep.h"',
-        '#include "trace.h"',
         '#include "trace/control.h"',
         '#include "trace/simple.h"',
         '')
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index 4594db6..52ce892 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -20,8 +20,13 @@ PUBLIC = True
 
 
 def generate_h_begin(events, group):
+    if group == "root":
+        header = "trace-ust-root.h"
+    else:
+        header = "trace-ust.h"
+
     out('#include <lttng/tracepoint.h>',
-        '#include "trace/generated-ust-provider.h"',
+        '#include "%s"' % header,
         '')
 
 
diff --git a/scripts/tracetool/format/c.py b/scripts/tracetool/format/c.py
index 47115ed..833c05a 100644
--- a/scripts/tracetool/format/c.py
+++ b/scripts/tracetool/format/c.py
@@ -20,10 +20,15 @@ def generate(events, backend, group):
     active_events = [e for e in events
                      if "disable" not in e.properties]
 
+    if group == "root":
+        header = "trace-root.h"
+    else:
+        header = "trace.h"
+
     out('/* This file is autogenerated by tracetool, do not edit. */',
         '',
         '#include "qemu/osdep.h"',
-        '#include "trace.h"',
+        '#include "%s"' % header,
         '')
 
     for e in events:
diff --git a/scripts/tracetool/format/tcg_h.py b/scripts/tracetool/format/tcg_h.py
index 5f213f6..7ddc4a5 100644
--- a/scripts/tracetool/format/tcg_h.py
+++ b/scripts/tracetool/format/tcg_h.py
@@ -28,13 +28,17 @@ def vcpu_transform_args(args):
 
 
 def generate(events, backend, group):
+    if group == "root":
+        header = "trace-root.h"
+    else:
+        header = "trace.h"
+
     out('/* This file is autogenerated by tracetool, do not edit. */',
         '/* You must include this file after the inclusion of helper.h */',
         '',
         '#ifndef TRACE_%s_GENERATED_TCG_TRACERS_H' % group.upper(),
         '#define TRACE_%s_GENERATED_TCG_TRACERS_H' % group.upper(),
         '',
-        '#include "trace.h"',
         '#include "exec/helper-proto.h"',
         '',
         )
diff --git a/scripts/tracetool/format/tcg_helper_c.py b/scripts/tracetool/format/tcg_helper_c.py
index cc26e03..7dccd8c 100644
--- a/scripts/tracetool/format/tcg_helper_c.py
+++ b/scripts/tracetool/format/tcg_helper_c.py
@@ -41,6 +41,11 @@ def vcpu_transform_args(args, mode):
 
 
 def generate(events, backend, group):
+    if group == "root":
+        header = "trace-root.h"
+    else:
+        header = "trace.h"
+
     events = [e for e in events
               if "disable" not in e.properties]
 
@@ -49,7 +54,6 @@ def generate(events, backend, group):
         '#include "qemu/osdep.h"',
         '#include "qemu-common.h"',
         '#include "cpu.h"',
-        '#include "trace.h"',
         '#include "exec/helper-proto.h"',
         '',
         )
diff --git a/scripts/tracetool/format/ust_events_c.py b/scripts/tracetool/format/ust_events_c.py
index cd87d8a..264784c 100644
--- a/scripts/tracetool/format/ust_events_c.py
+++ b/scripts/tracetool/format/ust_events_c.py
@@ -32,4 +32,4 @@ def generate(events, backend, group):
         ' */',
         '#pragma GCC diagnostic ignored "-Wredundant-decls"',
         '',
-        '#include "generated-ust-provider.h"')
+        '#include "trace-ust-all.h"')
diff --git a/scripts/tracetool/format/ust_events_h.py b/scripts/tracetool/format/ust_events_h.py
index d853155..514294c 100644
--- a/scripts/tracetool/format/ust_events_h.py
+++ b/scripts/tracetool/format/ust_events_h.py
@@ -20,13 +20,18 @@ def generate(events, backend, group):
     events = [e for e in events
               if "disabled" not in e.properties]
 
+    if group == "all":
+        include = "trace-ust-all.h"
+    else:
+        include = "trace-ust.h"
+
     out('/* This file is autogenerated by tracetool, do not edit. */',
         '',
         '#undef TRACEPOINT_PROVIDER',
         '#define TRACEPOINT_PROVIDER qemu',
         '',
         '#undef TRACEPOINT_INCLUDE_FILE',
-        '#define TRACEPOINT_INCLUDE_FILE ./generated-ust-provider.h',
+        '#define TRACEPOINT_INCLUDE_FILE ./%s' % include,
         '',
         '#if !defined (TRACE_%s_GENERATED_UST_H) || \\'  % group.upper(),
         '     defined(TRACEPOINT_HEADER_MULTI_READ)',
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 276c4ae..3a09be8 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -1,5 +1,5 @@
 #include "qemu/osdep.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "ui/qemu-spice.h"
 #include "sysemu/char.h"
 #include <spice.h>
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 22ea256..8c229ee 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -491,7 +491,7 @@ QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
 
 # Deps that are common to various different sets of tests below
-test-util-obj-y = libqemuutil.a libqemustub.a
+test-util-obj-y = $(trace-obj-y) libqemuutil.a libqemustub.a
 test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y)
 test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
 	tests/test-qapi-event.o tests/test-qmp-introspect.o \
diff --git a/thread-pool.c b/thread-pool.c
index 6fba913..3847969 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -19,7 +19,7 @@
 #include "qemu/queue.h"
 #include "qemu/thread.h"
 #include "qemu/coroutine.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "block/thread-pool.h"
 #include "qemu/main-loop.h"
 
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index d3b47da..7de840a 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -8,98 +8,16 @@
 tracetool-y = $(SRC_PATH)/scripts/tracetool.py
 tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
 
-$(BUILD_DIR)/trace-events-all: $(trace-events-y:%=$(SRC_PATH)/%)
+$(BUILD_DIR)/trace-events-all: $(trace-events-files)
 	$(call quiet-command,cat $^ > $@)
 
-######################################################################
-# Auto-generated event descriptions for LTTng ust code
-
-ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
-
-$(obj)/generated-ust-provider.h: $(obj)/generated-ust-provider.h-timestamp
-	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-ust-provider.h-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
-	$(call quiet-command,$(TRACETOOL) \
-		--group=all \
-		--format=ust-events-h \
-		--backends=$(TRACE_BACKENDS) \
-		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-
-$(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp $(BUILD_DIR)/config-host.mak
-	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-ust.c-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
-	$(call quiet-command,$(TRACETOOL) \
-		--group=all \
-		--format=ust-events-c \
-		--backends=$(TRACE_BACKENDS) \
-		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-
-$(obj)/generated-tracers.h: $(obj)/generated-ust-provider.h
-$(obj)/generated-tracers.c: $(obj)/generated-ust.c
-
-endif
-
-
-######################################################################
-# Auto-generated tracing routines
-
-##################################################
-# Execution level
-
-$(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
-	@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
-	$(call quiet-command,$(TRACETOOL) \
-		--group=all \
-		--format=h \
-		--backends=$(TRACE_BACKENDS) \
-		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-
-##############################
-# non-DTrace
-
-$(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
-	@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.c-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
-	$(call quiet-command,$(TRACETOOL) \
-		--group=all \
-		--format=c \
-		--backends=$(TRACE_BACKENDS) \
-		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-
-$(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.h
-
-##############################
-# DTrace
-
-# Normal practice is to name DTrace probe file with a '.d' extension
-# but that gets picked up by QEMU's Makefile as an external dependency
-# rule file. So we use '.dtrace' instead
-ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
-
-$(obj)/generated-tracers-dtrace.dtrace: $(obj)/generated-tracers-dtrace.dtrace-timestamp
-	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-tracers-dtrace.dtrace-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
-	$(call quiet-command,$(TRACETOOL) \
-		--group=all \
-		--format=d \
-		--backends=$(TRACE_BACKENDS) \
-		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-
-$(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers-dtrace.dtrace
-	$(call quiet-command,dtrace -o $@ -h -s $<,"GEN","$@")
-
-$(obj)/generated-tracers-dtrace.o: $(obj)/generated-tracers-dtrace.dtrace
-
-util-obj-y += generated-tracers-dtrace.o
-endif
 
 ##################################################
 # Translation level
 
 $(obj)/generated-helpers-wrappers.h: $(obj)/generated-helpers-wrappers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-helpers-wrappers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
+$(obj)/generated-helpers-wrappers.h-timestamp: $(trace-events-files) $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--group=all \
 		--format=tcg-helper-wrapper-h \
@@ -108,7 +26,7 @@ $(obj)/generated-helpers-wrappers.h-timestamp: $(BUILD_DIR)/trace-events-all $(B
 
 $(obj)/generated-helpers.h: $(obj)/generated-helpers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-helpers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
+$(obj)/generated-helpers.h-timestamp: $(trace-events-files) $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--group=all \
 		--format=tcg-helper-h \
@@ -117,7 +35,7 @@ $(obj)/generated-helpers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
 
 $(obj)/generated-helpers.c: $(obj)/generated-helpers.c-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-helpers.c-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
+$(obj)/generated-helpers.c-timestamp: $(trace-events-files) $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--group=all \
 		--format=tcg-helper-c \
@@ -131,7 +49,7 @@ target-obj-y += generated-helpers.o
 
 $(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-tcg-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
+$(obj)/generated-tcg-tracers.h-timestamp: $(trace-events-files) $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--group=all \
 		--format=tcg-h \
@@ -142,10 +60,8 @@ $(obj)/generated-tcg-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_
 ######################################################################
 # Backend code
 
-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
 target-obj-y += control-target.o
 util-obj-y += qmp.o
diff --git a/trace/control-target.c b/trace/control-target.c
index e2e138a..6266e63 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -9,7 +9,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "trace/control.h"
 #include "translate-all.h"
 
diff --git a/trace/control.c b/trace/control.c
index 56a2632..9b157b0 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -26,7 +26,7 @@
 #include "qemu/error-report.h"
 #include "qemu/config-file.h"
 #include "monitor/monitor.h"
-#include "trace.h"
+#include "trace-root.h"
 
 int trace_events_enabled_count;
 
diff --git a/trace/ftrace.c b/trace/ftrace.c
index 3588bb0..7de104d 100644
--- a/trace/ftrace.c
+++ b/trace/ftrace.c
@@ -10,8 +10,8 @@
  */
 
 #include "qemu/osdep.h"
-#include "trace.h"
 #include "trace/control.h"
+#include "trace/ftrace.h"
 
 int trace_marker_fd;
 
diff --git a/trace/simple.c b/trace/simple.c
index b263622..a221a3f 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -13,7 +13,6 @@
 #include <pthread.h>
 #endif
 #include "qemu/timer.h"
-#include "trace.h"
 #include "trace/control.h"
 #include "trace/simple.h"
 
diff --git a/translate-all.c b/translate-all.c
index 2026293..0a93474 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -25,7 +25,7 @@
 #include "qemu-common.h"
 #define NO_CPU_IO_DEFS
 #include "cpu.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "disas/disas.h"
 #include "exec/exec-all.h"
 #include "tcg.h"
diff --git a/vl.c b/vl.c
index a260f30..4bf1e7a 100644
--- a/vl.c
+++ b/vl.c
@@ -110,7 +110,7 @@ int main(int argc, char **argv)
 
 #include "slirp/libslirp.h"
 
-#include "trace.h"
+#include "trace-root.h"
 #include "trace/control.h"
 #include "qemu/queue.h"
 #include "sysemu/arch_init.h"
diff --git a/xen-hvm.c b/xen-hvm.c
index 0892361..5043beb 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -22,7 +22,7 @@
 #include "qemu/error-report.h"
 #include "qemu/range.h"
 #include "sysemu/xen-mapcache.h"
-#include "trace.h"
+#include "trace-root.h"
 #include "exec/address-spaces.h"
 
 #include <xen/hvm/ioreq.h>
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 31debdf..1a96d2e 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -19,7 +19,7 @@
 #include <xen/hvm/params.h>
 
 #include "sysemu/xen-mapcache.h"
-#include "trace.h"
+#include "trace-root.h"
 
 
 //#define MAPCACHE_DEBUG
-- 
2.9.3

[Qemu-devel] [PATCH v3 7/8] trace: update docs to reflect new code generation approach

[Daniel P. Berrange]

Describe use of per-subdir trace events files and how it impacts
code generation.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 docs/tracing.txt | 55 ++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index f351998a..e14bb6d 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -27,18 +27,44 @@ for debugging, profiling, and observing execution.
 
 == Trace events ==
 
-Each directory in the source tree can declare a set of static trace events
-in a "trace-events" file. Each trace event declaration names the event, its
-arguments, and the format string which can be used for pretty-printing:
+=== Sub-directory setup ===
 
-    qemu_vmalloc(size_t size, void *ptr) "size %zu ptr %p"
-    qemu_vfree(void *ptr) "ptr %p"
-
-All "trace-events" files must be listed in the "trace-event-y" make variable
-in the top level Makefile.objs. During build the individual files are combined
-to create a "trace-events-all" file, which is processed by the "tracetool"
-script during build to generate code for the trace events. The
-"trace-events-all" file is also installed into "/usr/share/qemu".
+Each directory in the source tree can declare a set of static trace events
+in a local "trace-events" file. All directories which contain "trace-events"
+files must be listed in the "trace-events-subdirs" make variable in the top
+level Makefile.objs. During build, the "trace-events" file in each listed
+subdirectory will be processed by the "tracetool" script to generate code for
+the trace events.
+
+The individual "trace-events" files are merged into a "trace-events-all" file,
+which is also installed into "/usr/share/qemu" with the name "trace-events".
+This merged file is to be used by the "simpletrace.py" script to later analyse
+traces in the simpletrace data format.
+
+In the sub-directory the following files will be automatically generated
+
+ - trace.c - the trace event state declarations
+ - trace.h - the trace event enums and probe functions
+ - trace-dtrace.h - DTrace event probe specification
+ - trace-dtrace.dtrace - DTrace event probe helper declaration
+ - trace-dtrace.o - binary DTrace provider (generated by dtrace)
+ - trace-ust.h - UST event probe helper declarations
+
+Source files in the sub-directory should #include the local 'trace.h' file,
+without any sub-directory path prefix. eg io/channel-buffer.c would do
+
+  #include "trace.h"
+
+To access the 'io/trace.h' file. While it is possible to include a trace.h
+file from outside a source files' own sub-directory, this is discouraged in
+general. It is strongly preferred that all events be declared directly in
+the sub-directory that uses them. The only exception is where there are some
+shared trace events defined in the top level directory trace-events file.
+The top level directory generates trace files with a filename prefix of
+"trace-root" instead of just "trace". This is to avoid ambiguity between
+a trace.h in the current directory, vs the top level directory.
+
+=== Using trace events ===
 
 Trace events are invoked directly from source code like this:
 
@@ -83,6 +109,13 @@ Format strings should reflect the types defined in the trace event.  Take
 special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
 respectively.  This ensures portability between 32- and 64-bit platforms.
 
+Each event declaration will start with the event name, then its arguments,
+finally a format string for pretty-printing. For example:
+
+    qemu_vmalloc(size_t size, void *ptr) "size %zu ptr %p"
+    qemu_vfree(void *ptr) "ptr %p"
+
+
 === Hints for adding new trace events ===
 
 1. Trace state changes in the code.  Interesting points in the code usually
-- 
2.9.3

[Qemu-devel] [PATCH v3 8/8] trace: improve error reporting when parsing simpletrace header

[Daniel P. Berrange]

When loading a simpletrace binary file we just report
"Not a valid trace file!" which is not very helpful. Report
exactly which field we found to be invalid.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 scripts/simpletrace.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
index 4ca903d..4c99004 100755
--- a/scripts/simpletrace.py
+++ b/scripts/simpletrace.py
@@ -73,10 +73,14 @@ def read_record(edict, idtoname, fobj):
 def read_trace_header(fobj):
     """Read and verify trace file header"""
     header = read_header(fobj, log_header_fmt)
-    if header is None or \
-       header[0] != header_event_id or \
-       header[1] != header_magic:
+    if header is None:
         raise ValueError('Not a valid trace file!')
+    if header[0] != header_event_id:
+        raise ValueError('Not a valid trace file, header id %d != %d' %
+                         (header[0], header_event_id))
+    if header[1] != header_magic:
+        raise ValueError('Not a valid trace file, header magic %d != %d' %
+                         (header[1], header_magic))
 
     log_version = header[2]
     if log_version not in [0, 2, 3, 4]:
-- 
2.9.3

Re: [Qemu-devel] [PATCH v3 6/8] trace: switch to modular code generation for sub-directories

[Lluís Vilanova]

Daniel P Berrange writes:

> Introduce rules in the top level Makefile that are able to generate
> trace.[ch] files in every subdirectory which has a trace-events file.

> The top level directory is handled specially, so instead of creating
> trace.h, it creates trace-root.h. This allows sub-directories to
> include the top level trace-root.h file, without ambiguity wrt to
> the trace.g file in the current sub-dir.

> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
[...]
> diff --git a/Makefile b/Makefile
> index 9f8968d..a8fa28a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -56,25 +56,136 @@ 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-tracers.h
> -ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
> -GENERATED_HEADERS += trace/generated-tracers-dtrace.h
> -endif
> -GENERATED_SOURCES += trace/generated-tracers.c
> -
>  GENERATED_HEADERS += trace/generated-tcg-tracers.h
 
>  GENERATED_HEADERS += trace/generated-helpers-wrappers.h
>  GENERATED_HEADERS += trace/generated-helpers.h
>  GENERATED_SOURCES += trace/generated-helpers.c
 
> -ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
> -GENERATED_HEADERS += trace/generated-ust-provider.h
> -GENERATED_SOURCES += trace/generated-ust.c
> +ifdef CONFIG_TRACE_UST
> +GENERATED_HEADERS += trace-ust-all.h
> +GENERATED_SOURCES += trace-ust-all.c
>  endif
 
>  GENERATED_HEADERS += module_block.h
 
> +TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
> +TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
> +TRACE_DTRACE =
> +ifdef CONFIG_TRACE_DTRACE
> +TRACE_HEADERS += trace-dtrace-root.h $(trace-events-subdirs:%=%/trace-dtrace.h)
> +TRACE_DTRACE += trace-dtrace-root.dtrace $(trace-events-subdirs:%=%/trace-dtrace.dtrace)
> +endif
> +ifdef CONFIG_TRACE_UST
> +TRACE_HEADERS += trace-ust-root.h $(trace-events-subdirs:%=%/trace-ust.h)
> +endif
> +
> +GENERATED_HEADERS += $(TRACE_HEADERS)
> +GENERATED_SOURCES += $(TRACE_SOURCES)
> +
> +trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
> +
> +%/trace.h: %/trace.h-timestamp
> +	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
> +%/trace.h-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
> +	$(call quiet-command,$(TRACETOOL) \
> +		--group=$(call trace-group-name,$@) \

Shouldn't you call trace-group-name with $*? To make sure it only contains the
directories *below* SRC_PATH.


Cheers,
  Lluis

Re: [Qemu-devel] [PATCH v3 1/8] make: move top level dir to end of include search path

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 4082 bytes --]

On 01/24/2017 05:01 AM, Daniel P. Berrange wrote:
> Currently the search path is
> 
>   1. source dir corresponding to input file (implicit by compiler)
>   2. top level build dir
>   3. top level source dir
>   4. top level source include/ dir
>   5. source dir corresponding to input file
>   6. build dir corresponding to output file
> 
> This causes a semantic difference in behaviour for builds
> where srcdir == builddir vs srcdir != builddir, because
> item 5 moves from end to start, when srcdir == builddir.

Rather, item 5 is a no-op (because it duplicated 1), and item 6 moves
from the end to the beginning when srcdir == builddir

> 
> As a general rule we also want to move to have all shared
> headers in the include/ dir, so move that ahead of the
> top level dirs in the search order.

Wait - are you proposing that you swap 4 to occur earlier than 2/3?...

> 
> Thus we now have:
> 
>   1. source dir corresponding to input file
>   2. build dir corresponding to output file
>   3. top level build dir
>   4. top level source dir
>   5. top level source include/ dir

...because this doesn't match that swap, and I don't see it in the patch
body (but I may have missed it; I'm not as strong at reviewing make as I
am at C)

> 
> and items 1+2 and 4+5 collapse into a single dir when srcdir==builddir

Isn't that items 3+4 (not 4+5) that collapse?

> so overall order remains the same.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  rules.mak | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)


> 
> diff --git a/rules.mak b/rules.mak
> index d5c516c..e09aabe 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -26,8 +26,10 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
>  
> -# Same as -I$(SRC_PATH) -I., but for the nested source/object directories
> -QEMU_INCLUDES += -I$(<D) -I$(@D)

In particular, this is the old code for 5 and 6,

> +# Compiler searches the source file dir first, but in vpath builds
> +# we need to make it search the build dir too, before any other
> +# explicit search paths.
> +QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D)

while this is the new code for 2, plus documentation that 1 is implicit.

>  
>  WL_U := -Wl,-u,
>  find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
> @@ -61,7 +63,7 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \
>                    $(filter-out %.o %.mo,$1))
>  
>  %.o: %.c
> -	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
> +	$(call quiet-command,$(CC) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CC","$(TARGET_DIR)$@")

And the pre-pending of QEMU_LOCAL_INCLUDES is what changes the position
of the local directory from last to first, thus delaying the top level
dir to the end, but I don't see top/include/ moving.

These are now some long lines; is it worth taking the time to add \ line
splitting for legibility, either in this patch or as an add-on?

> @@ -359,6 +361,7 @@ define unnest-vars
>                  $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)),
>                  $(error $o added in $v but $o-objs is not set)))
>          $(shell mkdir -p ./ $(sort $(dir $($v))))
> +        $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v))))
>          # Include all the .d files

Okay, this change makes sense (make sure all the build directories exist
in time; no-op for in-tree build, but helpful for VPATH), but seems
unrelated to the commit message.  Rebase snafu?

It looks like you're on the right track, but there's enough
discrepancies between the commit message and actual change that I'd
prefer a v4 before I grant R-b.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v3 1/8] make: move top level dir to end of include search path

[Daniel P. Berrange]

On Tue, Jan 24, 2017 at 02:11:29PM -0600, Eric Blake wrote:
> On 01/24/2017 05:01 AM, Daniel P. Berrange wrote:
> > Currently the search path is
> > 
> >   1. source dir corresponding to input file (implicit by compiler)
> >   2. top level build dir
> >   3. top level source dir
> >   4. top level source include/ dir
> >   5. source dir corresponding to input file
> >   6. build dir corresponding to output file
> > 
> > This causes a semantic difference in behaviour for builds
> > where srcdir == builddir vs srcdir != builddir, because
> > item 5 moves from end to start, when srcdir == builddir.
> 
> Rather, item 5 is a no-op (because it duplicated 1), and item 6 moves
> from the end to the beginning when srcdir == builddir

Yes

> > As a general rule we also want to move to have all shared
> > headers in the include/ dir, so move that ahead of the
> > top level dirs in the search order.
> 
> Wait - are you proposing that you swap 4 to occur earlier than 2/3?...

Sigh, left over from an earlier version of this patch. I was trying
todo a more general cleanup as described here, but decided to cut
back to the bare minimum I needed for trace work.

> > Thus we now have:
> > 
> >   1. source dir corresponding to input file
> >   2. build dir corresponding to output file
> >   3. top level build dir
> >   4. top level source dir
> >   5. top level source include/ dir
> 
> ...because this doesn't match that swap, and I don't see it in the patch
> body (but I may have missed it; I'm not as strong at reviewing make as I
> am at C)

Yeah, you're right.

> > 
> > and items 1+2 and 4+5 collapse into a single dir when srcdir==builddir
> 
> Isn't that items 3+4 (not 4+5) that collapse?

Yes, typo.

> > so overall order remains the same.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  rules.mak | 17 ++++++++++-------
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> 
> > 
> > diff --git a/rules.mak b/rules.mak
> > index d5c516c..e09aabe 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -26,8 +26,10 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
> >  # Flags for dependency generation
> >  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
> >  
> > -# Same as -I$(SRC_PATH) -I., but for the nested source/object directories
> > -QEMU_INCLUDES += -I$(<D) -I$(@D)
> 
> In particular, this is the old code for 5 and 6,
> 
> > +# Compiler searches the source file dir first, but in vpath builds
> > +# we need to make it search the build dir too, before any other
> > +# explicit search paths.
> > +QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D)
> 
> while this is the new code for 2, plus documentation that 1 is implicit.

So there's a subtle difference I didn't explain, and which I'm going to
tweak again.

 -I$(@D)

is a relative include. eg for a file 'migration/ram.o' it will resolve
to '-Imigration' relative to the build dir. Except there's a subtle
difference between target-dependent and target-independent files. For
example, migration/migration.o will be built in $BUILD_DIR/migration
but migration/ram.o will be built in $BUILD_DIR/x86_64-softmmu/migration
so this reslative include points to two different places potentially.

Hence, I changed to -I$(BUILD_DIR)/$(@D), so it is gauranteed to
point to $BUILD_DIR/migration, even for target-dependant files.

In retrospect, I think it is more correct to include both directories
ie  -I$(BUILD_DIR)/$(@D) and -I$(@D). Even if not technically needed
by this patch series I think its clearer.

> >  WL_U := -Wl,-u,
> >  find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
> > @@ -61,7 +63,7 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \
> >                    $(filter-out %.o %.mo,$1))
> >  
> >  %.o: %.c
> > -	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
> > +	$(call quiet-command,$(CC) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
> 
> And the pre-pending of QEMU_LOCAL_INCLUDES is what changes the position
> of the local directory from last to first, thus delaying the top level
> dir to the end, but I don't see top/include/ moving.
> 
> These are now some long lines; is it worth taking the time to add \ line
> splitting for legibility, either in this patch or as an add-on?
> 
> > @@ -359,6 +361,7 @@ define unnest-vars
> >                  $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)),
> >                  $(error $o added in $v but $o-objs is not set)))
> >          $(shell mkdir -p ./ $(sort $(dir $($v))))
> > +        $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v))))
> >          # Include all the .d files
> 
> Okay, this change makes sense (make sure all the build directories exist
> in time; no-op for in-tree build, but helpful for VPATH), but seems
> unrelated to the commit message.  Rebase snafu?

The existing mkdir line there ensures that the -I$(@D) always points to a
directory that exists.

The new mkdir line does the same for -I$(BUILD_DIR)/$(@D).

This is to deal with fact that the compiler warns if you give a -I directory
that does not exist

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH v3 2/8] trace: move hw/block/dataplane events to correct subdir

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

On Tue, Jan 24, 2017 at 11:01:45AM +0000, Daniel P. Berrange wrote:
> The trace-events for a given source file should generally
> always live in the same directory as the source file.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  Makefile.objs                   | 1 +
>  hw/block/dataplane/trace-events | 6 ++++++
>  hw/block/trace-events           | 5 -----
>  3 files changed, 7 insertions(+), 5 deletions(-)
>  create mode 100644 hw/block/dataplane/trace-events

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 3/8] trace: move hw/xen events to correct subdir

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On Tue, Jan 24, 2017 at 11:01:46AM +0000, Daniel P. Berrange wrote:
> The trace-events for a given source file should generally
> always live in the same directory as the source file.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  Makefile.objs       |  1 +
>  hw/xen/trace-events | 13 +++++++++++++
>  trace-events        | 10 ----------
>  3 files changed, 14 insertions(+), 10 deletions(-)
>  create mode 100644 hw/xen/trace-events

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 4/8] trace: move hw/i386/xen events to correct subdir

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

On Tue, Jan 24, 2017 at 11:01:47AM +0000, Daniel P. Berrange wrote:
> The trace-events for a given source file should generally
> always live in the same directory as the source file.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  Makefile.objs            | 1 +
>  hw/i386/trace-events     | 7 -------
>  hw/i386/xen/trace-events | 6 ++++++
>  3 files changed, 7 insertions(+), 7 deletions(-)
>  create mode 100644 hw/i386/xen/trace-events

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 5/8] trace: move setting of group name into Makefiles

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

On Tue, Jan 24, 2017 at 11:01:48AM +0000, Daniel P. Berrange wrote:
> Having tracetool.py figure out the right group name from just
> the input filename is not practical when considering the
> different build vs src path combinations. Instead simply take
> the group name as a command line arg from the Makefile, which
> can trivially provide the right name.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  Makefile.target      |  3 +++
>  scripts/tracetool.py | 23 +++++++++--------------
>  trace/Makefile.objs  |  9 +++++++++
>  3 files changed, 21 insertions(+), 14 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 6/8] trace: switch to modular code generation for sub-directories

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3170 bytes --]

On Tue, Jan 24, 2017 at 11:01:49AM +0000, Daniel P. Berrange wrote:
> Introduce rules in the top level Makefile that are able to generate
> trace.[ch] files in every subdirectory which has a trace-events file.
> 
> The top level directory is handled specially, so instead of creating
> trace.h, it creates trace-root.h. This allows sub-directories to
> include the top level trace-root.h file, without ambiguity wrt to
> the trace.g file in the current sub-dir.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  .gitignore                                |  22 +++--
>  Makefile                                  | 156 ++++++++++++++++++++++++++----
>  Makefile.objs                             | 102 ++++++++++---------
>  Makefile.target                           |   7 +-
>  aio-posix.c                               |   2 +-
>  balloon.c                                 |   2 +-
>  block.c                                   |   2 +-
>  blockdev-nbd.c                            |   1 -
>  blockdev.c                                |   2 +-
>  blockjob.c                                |   1 -
>  cpu-exec.c                                |   2 +-
>  dma-helpers.c                             |   2 +-
>  exec.c                                    |   2 +-
>  hw/net/fsl_etsec/etsec.c                  |   1 -
>  include/exec/cpu_ldst_template.h          |   2 +-
>  include/exec/cpu_ldst_useronly_template.h |   2 +-
>  include/hw/xen/xen_common.h               |   2 +-
>  include/trace.h                           |   6 --
>  ioport.c                                  |   2 +-
>  kvm-all.c                                 |   2 +-
>  memory.c                                  |   2 +-
>  monitor.c                                 |   2 +-
>  qom/cpu.c                                 |   2 +-
>  scripts/tracetool.py                      |   8 +-
>  scripts/tracetool/backend/dtrace.py       |   7 +-
>  scripts/tracetool/backend/simple.py       |   1 -
>  scripts/tracetool/backend/ust.py          |   7 +-
>  scripts/tracetool/format/c.py             |   7 +-
>  scripts/tracetool/format/tcg_h.py         |   6 +-
>  scripts/tracetool/format/tcg_helper_c.py  |   6 +-
>  scripts/tracetool/format/ust_events_c.py  |   2 +-
>  scripts/tracetool/format/ust_events_h.py  |   7 +-
>  spice-qemu-char.c                         |   2 +-
>  tests/Makefile.include                    |   2 +-
>  thread-pool.c                             |   2 +-
>  trace/Makefile.objs                       |  94 +-----------------
>  trace/control-target.c                    |   2 +-
>  trace/control.c                           |   2 +-
>  trace/ftrace.c                            |   2 +-
>  trace/simple.c                            |   1 -
>  translate-all.c                           |   2 +-
>  vl.c                                      |   2 +-
>  xen-hvm.c                                 |   2 +-
>  xen-mapcache.c                            |   2 +-
>  44 files changed, 283 insertions(+), 209 deletions(-)
>  delete mode 100644 include/trace.h

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 7/8] trace: update docs to reflect new code generation approach

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 417 bytes --]

On Tue, Jan 24, 2017 at 11:01:50AM +0000, Daniel P. Berrange wrote:
> Describe use of per-subdir trace events files and how it impacts
> code generation.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  docs/tracing.txt | 55 ++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 44 insertions(+), 11 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 8/8] trace: improve error reporting when parsing simpletrace header

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

On Tue, Jan 24, 2017 at 11:01:51AM +0000, Daniel P. Berrange wrote:
> When loading a simpletrace binary file we just report
> "Not a valid trace file!" which is not very helpful. Report
> exactly which field we found to be invalid.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  scripts/simpletrace.py | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 0/8] Switch all subdirs over to modular trace.h file

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 5288 bytes --]

On Tue, Jan 24, 2017 at 11:01:43AM +0000, Daniel P. Berrange wrote:
> This is the final part of my trace events refactoring.
> 
> Previously we merged code that split trace-events up
> into one file per sub-directory. We also merged code
> to the code-generator that enables us to generate
> and use multiple independant sets of trace events.
> 
> This is the final glue that makes use of these two
> earlier changes, so that we actually generate a
> separate trace.h & trace.c file per sub-directory.
> 
> The result is that when adding a new trace event to
> a file, only files in that sub-directory get rebuilt.
> Theoretically the build is faster too, since each
> source file includes a much smaller trace.h that
> only contains probes relevant to it. In reality this
> speed benefit is probably marginal & lost in the noise.
> 
> This patch series has massively changed since v2.
> 
>  - It no longer modifies the Makefile.objs rules in each
>    sub-directory. A bit of magic in the top level
>    Makefile.objs file lets us generate all the needed
>    rules from one place
> 
>  - It sticks with bare "trace.h" includes instead of adding
>    sub-dir prefix "hw/block/trace.h". To achieve this we
>    had to rename the trace.h to trace-root.h in the top
>    level directory to avoid ambiguity in places where a
>    file needs to include the top-level instead of local
>    trace.h
> 
> Daniel P. Berrange (8):
>   make: move top level dir to end of include search path
>   trace: move hw/block/dataplane events to correct subdir
>   trace: move hw/xen events to correct subdir
>   trace: move hw/i386/xen events to correct subdir
>   trace: move setting of group name into Makefiles
>   trace: switch to modular code generation for sub-directories
>   trace: update docs to reflect new code generation approach
>   trace: improve error reporting when parsing simpletrace header
> 
>  .gitignore                                |  22 +++--
>  Makefile                                  | 156 ++++++++++++++++++++++++++----
>  Makefile.objs                             |  99 ++++++++++---------
>  Makefile.target                           |  10 +-
>  aio-posix.c                               |   2 +-
>  balloon.c                                 |   2 +-
>  block.c                                   |   2 +-
>  blockdev-nbd.c                            |   1 -
>  blockdev.c                                |   2 +-
>  blockjob.c                                |   1 -
>  cpu-exec.c                                |   2 +-
>  dma-helpers.c                             |   2 +-
>  docs/tracing.txt                          |  55 ++++++++---
>  exec.c                                    |   2 +-
>  hw/block/dataplane/trace-events           |   6 ++
>  hw/block/trace-events                     |   5 -
>  hw/i386/trace-events                      |   7 --
>  hw/i386/xen/trace-events                  |   6 ++
>  hw/net/fsl_etsec/etsec.c                  |   1 -
>  hw/xen/trace-events                       |  13 +++
>  include/exec/cpu_ldst_template.h          |   2 +-
>  include/exec/cpu_ldst_useronly_template.h |   2 +-
>  include/hw/xen/xen_common.h               |   2 +-
>  include/trace.h                           |   6 --
>  ioport.c                                  |   2 +-
>  kvm-all.c                                 |   2 +-
>  memory.c                                  |   2 +-
>  monitor.c                                 |   2 +-
>  qom/cpu.c                                 |   2 +-
>  rules.mak                                 |  17 ++--
>  scripts/simpletrace.py                    |  10 +-
>  scripts/tracetool.py                      |  31 +++---
>  scripts/tracetool/backend/dtrace.py       |   7 +-
>  scripts/tracetool/backend/simple.py       |   1 -
>  scripts/tracetool/backend/ust.py          |   7 +-
>  scripts/tracetool/format/c.py             |   7 +-
>  scripts/tracetool/format/tcg_h.py         |   6 +-
>  scripts/tracetool/format/tcg_helper_c.py  |   6 +-
>  scripts/tracetool/format/ust_events_c.py  |   2 +-
>  scripts/tracetool/format/ust_events_h.py  |   7 +-
>  spice-qemu-char.c                         |   2 +-
>  tests/Makefile.include                    |   2 +-
>  thread-pool.c                             |   2 +-
>  trace-events                              |  10 --
>  trace/Makefile.objs                       |  93 ++----------------
>  trace/control-target.c                    |   2 +-
>  trace/control.c                           |   2 +-
>  trace/ftrace.c                            |   2 +-
>  trace/simple.c                            |   1 -
>  translate-all.c                           |   2 +-
>  vl.c                                      |   2 +-
>  xen-hvm.c                                 |   2 +-
>  xen-mapcache.c                            |   2 +-
>  53 files changed, 385 insertions(+), 258 deletions(-)
>  create mode 100644 hw/block/dataplane/trace-events
>  create mode 100644 hw/i386/xen/trace-events
>  create mode 100644 hw/xen/trace-events
>  delete mode 100644 include/trace.h

Looks good, I hope to merge v4.  Thanks for investigating all the
Makefile cruft needed to simplify this.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v3 6/8] trace: switch to modular code generation for sub-directories

[Daniel P. Berrange]

On Tue, Jan 24, 2017 at 08:53:57PM +0200, Lluís Vilanova wrote:
> Daniel P Berrange writes:
> 
> > Introduce rules in the top level Makefile that are able to generate
> > trace.[ch] files in every subdirectory which has a trace-events file.
> 
> > The top level directory is handled specially, so instead of creating
> > trace.h, it creates trace-root.h. This allows sub-directories to
> > include the top level trace-root.h file, without ambiguity wrt to
> > the trace.g file in the current sub-dir.
> 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> [...]
> > diff --git a/Makefile b/Makefile
> > index 9f8968d..a8fa28a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -56,25 +56,136 @@ 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-tracers.h
> > -ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
> > -GENERATED_HEADERS += trace/generated-tracers-dtrace.h
> > -endif
> > -GENERATED_SOURCES += trace/generated-tracers.c
> > -
> >  GENERATED_HEADERS += trace/generated-tcg-tracers.h
>  
> >  GENERATED_HEADERS += trace/generated-helpers-wrappers.h
> >  GENERATED_HEADERS += trace/generated-helpers.h
> >  GENERATED_SOURCES += trace/generated-helpers.c
>  
> > -ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
> > -GENERATED_HEADERS += trace/generated-ust-provider.h
> > -GENERATED_SOURCES += trace/generated-ust.c
> > +ifdef CONFIG_TRACE_UST
> > +GENERATED_HEADERS += trace-ust-all.h
> > +GENERATED_SOURCES += trace-ust-all.c
> >  endif
>  
> >  GENERATED_HEADERS += module_block.h
>  
> > +TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
> > +TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
> > +TRACE_DTRACE =
> > +ifdef CONFIG_TRACE_DTRACE
> > +TRACE_HEADERS += trace-dtrace-root.h $(trace-events-subdirs:%=%/trace-dtrace.h)
> > +TRACE_DTRACE += trace-dtrace-root.dtrace $(trace-events-subdirs:%=%/trace-dtrace.dtrace)
> > +endif
> > +ifdef CONFIG_TRACE_UST
> > +TRACE_HEADERS += trace-ust-root.h $(trace-events-subdirs:%=%/trace-ust.h)
> > +endif
> > +
> > +GENERATED_HEADERS += $(TRACE_HEADERS)
> > +GENERATED_SOURCES += $(TRACE_SOURCES)
> > +
> > +trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
> > +
> > +%/trace.h: %/trace.h-timestamp
> > +	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
> > +%/trace.h-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
> > +	$(call quiet-command,$(TRACETOOL) \
> > +		--group=$(call trace-group-name,$@) \
> 
> Shouldn't you call trace-group-name with $*? To make sure it only contains the
> directories *below* SRC_PATH.

$@ is a relative path already

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|