[PATCH v2 0/4] [RfC] fix tracing for modules

[PATCH v2 0/4] [RfC] fix tracing for modules

[Gerd Hoffmann]

First version that actually works.  Only qxl covered for this RfC, other
modules will follow once the basics are hashed out.

More context:
  https://bugzilla.redhat.com/show_bug.cgi?id=1898700
  https://bugzilla.redhat.com/show_bug.cgi?id=1869642

take care,
  Gerd

Gerd Hoffmann (4):
  meson: add trace_events_config[]
  meson: move up hw subdir (specifically before trace subdir)
  meson: add module_trace & module_trace_src
  meson: move qxl trace events to separate file

 hw/display/qxl-render.c     |  1 +
 hw/display/qxl.c            |  1 +
 hw/display/meson.build      |  5 +++
 hw/display/trace-events     | 67 -------------------------------------
 hw/display/trace-events-qxl | 66 ++++++++++++++++++++++++++++++++++++
 meson.build                 |  7 ++--
 trace/meson.build           | 37 +++++++++++++++-----
 7 files changed, 106 insertions(+), 78 deletions(-)
 create mode 100644 hw/display/trace-events-qxl

-- 
2.27.0

[PATCH v2 1/4] meson: add trace_events_config[]

[Gerd Hoffmann]

It's an array of dicts, where each dict holds the configuration for one
trace-events file.  For now just fill it from trace_events_subdirs.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 meson.build       |  1 +
 trace/meson.build | 21 ++++++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 5062407c7052..b62f71c6b4f1 100644
--- a/meson.build
+++ b/meson.build
@@ -1386,6 +1386,7 @@ target_softmmu_arch = {}
 
 # TODO: add each directory to the subdirs from its own meson.build, once
 # we have those
+trace_events_config = []
 trace_events_subdirs = [
   'accel/kvm',
   'accel/tcg',
diff --git a/trace/meson.build b/trace/meson.build
index 843ea1449540..552371b84241 100644
--- a/trace/meson.build
+++ b/trace/meson.build
@@ -1,12 +1,23 @@
 specific_ss.add(files('control-target.c'))
 
 trace_events_files = []
-foreach dir : [ '.' ] + trace_events_subdirs
-  trace_events_file = meson.source_root() / dir / 'trace-events'
+
+trace_events_config += {
+  'file'  : meson.source_root() / 'trace-events',
+  'group' : 'root',
+}
+foreach dir : trace_events_subdirs
+  trace_events_config += {
+    'file'  : meson.source_root() / dir / 'trace-events',
+    'group' : dir.underscorify(),
+  }
+endforeach
+
+foreach c : trace_events_config
+  trace_events_file = c.get('file')
   trace_events_files += [ trace_events_file ]
-  group_name = dir == '.' ? 'root' : dir.underscorify()
-  group = '--group=' + group_name
-  fmt = '@0@-' + group_name + '.@1@'
+  group = '--group=' + c.get('group')
+  fmt = '@0@-' + c.get('group') + '.@1@'
 
   trace_h = custom_target(fmt.format('trace', 'h'),
                           output: fmt.format('trace', 'h'),
-- 
2.27.0

[PATCH v2 2/4] meson: move up hw subdir (specifically before trace subdir)

[Gerd Hoffmann]

Needed so trace/meson.build can see
stuff done in hw/*/meson.build.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index b62f71c6b4f1..a85d331668fa 100644
--- a/meson.build
+++ b/meson.build
@@ -1477,6 +1477,8 @@ trace_events_subdirs += [
   'util',
 ]
 
+subdir('hw')
+
 subdir('contrib/libvhost-user')
 subdir('qapi')
 subdir('qobject')
@@ -1564,7 +1566,6 @@ subdir('migration')
 subdir('monitor')
 subdir('net')
 subdir('replay')
-subdir('hw')
 subdir('accel')
 subdir('plugins')
 subdir('bsd-user')
-- 
2.27.0

[PATCH v2 3/4] meson: add module_trace & module_trace_src

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 meson.build       |  3 ++-
 trace/meson.build | 16 ++++++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index a85d331668fa..0000cc286120 100644
--- a/meson.build
+++ b/meson.build
@@ -1591,7 +1591,8 @@ foreach d, list : modules
   foreach m, module_ss : list
     if enable_modules and targetos != 'windows'
       module_ss = module_ss.apply(config_all, strict: false)
-      sl = static_library(d + '-' + m, [genh, module_ss.sources()],
+      module_trace_src = module_trace.get(d + '-' + m, [])
+      sl = static_library(d + '-' + m, [genh, module_ss.sources(), module_trace_src],
                           dependencies: [modulecommon, module_ss.dependencies()], pic: true)
       if d == 'block'
         block_mods += sl
diff --git a/trace/meson.build b/trace/meson.build
index 552371b84241..b4e59ce9668b 100644
--- a/trace/meson.build
+++ b/trace/meson.build
@@ -1,6 +1,7 @@
 specific_ss.add(files('control-target.c'))
 
 trace_events_files = []
+module_trace = {}
 
 trace_events_config += {
   'file'  : meson.source_root() / 'trace-events',
@@ -18,6 +19,8 @@ foreach c : trace_events_config
   trace_events_files += [ trace_events_file ]
   group = '--group=' + c.get('group')
   fmt = '@0@-' + c.get('group') + '.@1@'
+  mod = c.get('module', '')
+  module_trace_src = []
 
   trace_h = custom_target(fmt.format('trace', 'h'),
                           output: fmt.format('trace', 'h'),
@@ -36,10 +39,10 @@ foreach c : trace_events_config
                                 input: trace_events_file,
                                 command: [ tracetool, group, '--format=ust-events-h', '@INPUT@' ],
                                 capture: true)
-    trace_ss.add(trace_ust_h, lttng, urcubp)
+    module_trace_src += [ trace_ust_h, lttng, urcubp ]
     genh += trace_ust_h
   endif
-  trace_ss.add(trace_h, trace_c)
+  module_trace_src += [ trace_h, trace_c ]
   if 'CONFIG_TRACE_DTRACE' in config_host
     trace_dtrace = custom_target(fmt.format('trace-dtrace', 'dtrace'),
                                  output: fmt.format('trace-dtrace', 'dtrace'),
@@ -50,17 +53,22 @@ foreach c : trace_events_config
                                    output: fmt.format('trace-dtrace', 'h'),
                                    input: trace_dtrace,
                                    command: [ 'dtrace', '-DSTAP_SDT_V2', '-o', '@OUTPUT@', '-h', '-s', '@INPUT@' ])
-    trace_ss.add(trace_dtrace_h)
+    module_trace_src += trace_dtrace_h
     if host_machine.system() != 'darwin'
       trace_dtrace_o = custom_target(fmt.format('trace-dtrace', 'o'),
                                      output: fmt.format('trace-dtrace', 'o'),
                                      input: trace_dtrace,
                                      command: [ 'dtrace', '-DSTAP_SDT_V2', '-o', '@OUTPUT@', '-G', '-s', '@INPUT@' ])
-      trace_ss.add(trace_dtrace_o)
+      module_trace_src += trace_dtrace_o
     endif
 
     genh += trace_dtrace_h
   endif
+  if enable_modules and mod != ''
+    module_trace += { mod : module_trace_src }
+  else
+    trace_ss.add(module_trace_src)
+  endif
 endforeach
 
 trace_events_all = custom_target('trace-events-all',
-- 
2.27.0

[PATCH v2 4/4] meson: move qxl trace events to separate file

[Gerd Hoffmann]

Move qxl trace events to separate trace-events-qxl file.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/qxl-render.c     |  1 +
 hw/display/qxl.c            |  1 +
 hw/display/meson.build      |  5 +++
 hw/display/trace-events     | 67 -------------------------------------
 hw/display/trace-events-qxl | 66 ++++++++++++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 67 deletions(-)
 create mode 100644 hw/display/trace-events-qxl

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 3ce2e57b8feb..cc4862e26eb6 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -23,6 +23,7 @@
 #include "qxl.h"
 #include "sysemu/runstate.h"
 #include "trace.h"
+#include "trace/trace-hw_display_qxl.h"
 
 static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
 {
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 431c1070967a..4e8d1bb8d77b 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -33,6 +33,7 @@
 #include "migration/blocker.h"
 #include "migration/vmstate.h"
 #include "trace.h"
+#include "trace/trace-hw_display_qxl.h"
 
 #include "qxl.h"
 
diff --git a/hw/display/meson.build b/hw/display/meson.build
index dad3bd2b414b..d4a0e0204ee2 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -43,6 +43,11 @@ if config_all_devices.has_key('CONFIG_QXL')
   qxl_ss = ss.source_set()
   qxl_ss.add(when: 'CONFIG_QXL', if_true: [files('qxl.c', 'qxl-logger.c', 'qxl-render.c'),
                                            pixman, spice])
+  trace_events_config += {
+    'file'   : meson.source_root() / 'hw' / 'display' / 'trace-events-qxl',
+    'group'  : 'hw_display_qxl',
+    'module' : 'hw-display-qxl',
+  }
   hw_display_modules += {'qxl': qxl_ss}
 endif
 
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 957b8ba99436..48636149e4b2 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -58,73 +58,6 @@ virtio_gpu_update_cursor(uint32_t scanout, uint32_t x, uint32_t y, const char *t
 virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence 0x%" PRIx64 ", type 0x%x"
 virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64
 
-# qxl.c
-disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d"
-disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u"
-qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, uint64_t mem, uint32_t format, uint32_t position) "%d %ux%u mem=0x%" PRIx64 " %u,%u"
-qxl_create_guest_primary_rest(int qid, int32_t stride, uint32_t type, uint32_t flags) "%d %d,%d,%d"
-qxl_destroy_primary(int qid) "%d"
-qxl_enter_vga_mode(int qid) "%d"
-qxl_exit_vga_mode(int qid) "%d"
-qxl_hard_reset(int qid, int64_t loadvm) "%d loadvm=%"PRId64
-qxl_interface_async_complete_io(int qid, uint32_t current_async, void *cookie) "%d current=%d cookie=%p"
-qxl_interface_attach_worker(int qid) "%d"
-qxl_interface_get_init_info(int qid) "%d"
-qxl_interface_set_compression_level(int qid, int64_t level) "%d %"PRId64
-qxl_interface_update_area_complete(int qid, uint32_t surface_id, uint32_t dirty_left, uint32_t dirty_right, uint32_t dirty_top, uint32_t dirty_bottom) "%d surface=%d [%d,%d,%d,%d]"
-qxl_interface_update_area_complete_rest(int qid, uint32_t num_updated_rects) "%d #=%d"
-qxl_interface_update_area_complete_overflow(int qid, int max) "%d max=%d"
-qxl_interface_update_area_complete_schedule_bh(int qid, uint32_t num_dirty) "%d #dirty=%d"
-qxl_io_destroy_primary_ignored(int qid, const char *mode) "%d %s"
-qxl_io_log(int qid, const char *log_buf) "%d %s"
-qxl_io_read_unexpected(int qid) "%d"
-qxl_io_unexpected_vga_mode(int qid, uint64_t addr, uint64_t val, const char *desc) "%d 0x%"PRIx64"=%"PRIu64" (%s)"
-qxl_io_write(int qid, const char *mode, uint64_t addr, const char *aname, uint64_t val, unsigned size, int async) "%d %s addr=%"PRIu64 " (%s) val=%"PRIu64" size=%u async=%d"
-qxl_memslot_add_guest(int qid, uint32_t slot_id, uint64_t guest_start, uint64_t guest_end) "%d %u: guest phys 0x%"PRIx64 " - 0x%" PRIx64
-qxl_post_load(int qid, const char *mode) "%d %s"
-qxl_pre_load(int qid) "%d"
-qxl_pre_save(int qid) "%d"
-qxl_reset_surfaces(int qid) "%d"
-qxl_ring_command_check(int qid, const char *mode) "%d %s"
-qxl_ring_command_get(int qid, const char *mode) "%d %s"
-qxl_ring_command_req_notification(int qid) "%d"
-qxl_ring_cursor_check(int qid, const char *mode) "%d %s"
-qxl_ring_cursor_get(int qid, const char *mode) "%d %s"
-qxl_ring_cursor_req_notification(int qid) "%d"
-qxl_ring_res_push(int qid, const char *mode, uint32_t surface_count, uint32_t free_res, void *last_release, const char *notify) "%d %s s#=%d res#=%d last=%p notify=%s"
-qxl_ring_res_push_rest(int qid, uint32_t ring_has, uint32_t ring_size, uint32_t prod, uint32_t cons) "%d ring %d/%d [%d,%d]"
-qxl_ring_res_put(int qid, uint32_t free_res) "%d #res=%d"
-qxl_set_mode(int qid, int modenr, uint32_t x_res, uint32_t y_res, uint32_t bits, uint64_t devmem) "%d mode=%d [ x=%d y=%d @ bpp=%d devmem=0x%" PRIx64 " ]"
-qxl_soft_reset(int qid) "%d"
-qxl_spice_destroy_surfaces_complete(int qid) "%d"
-qxl_spice_destroy_surfaces(int qid, int async) "%d async=%d"
-qxl_spice_destroy_surface_wait_complete(int qid, uint32_t id) "%d sid=%d"
-qxl_spice_destroy_surface_wait(int qid, uint32_t id, int async) "%d sid=%d async=%d"
-qxl_spice_flush_surfaces_async(int qid, uint32_t surface_count, uint32_t num_free_res) "%d s#=%d, res#=%d"
-qxl_spice_monitors_config(int qid) "%d"
-qxl_spice_loadvm_commands(int qid, void *ext, uint32_t count) "%d ext=%p count=%d"
-qxl_spice_oom(int qid) "%d"
-qxl_spice_reset_cursor(int qid) "%d"
-qxl_spice_reset_image_cache(int qid) "%d"
-qxl_spice_reset_memslots(int qid) "%d"
-qxl_spice_update_area(int qid, uint32_t surface_id, uint32_t left, uint32_t right, uint32_t top, uint32_t bottom) "%d sid=%d [%d,%d,%d,%d]"
-qxl_spice_update_area_rest(int qid, uint32_t num_dirty_rects, uint32_t clear_dirty_region) "%d #d=%d clear=%d"
-qxl_surfaces_dirty(int qid, uint64_t offset, uint64_t size) "%d offset=0x%"PRIx64" size=0x%"PRIx64
-qxl_send_events(int qid, uint32_t events) "%d %d"
-qxl_send_events_vm_stopped(int qid, uint32_t events) "%d %d"
-qxl_set_guest_bug(int qid) "%d"
-qxl_interrupt_client_monitors_config(int qid, int num_heads, void *heads) "%d %d %p"
-qxl_client_monitors_config_unsupported_by_guest(int qid, uint32_t int_mask, void *client_monitors_config) "%d 0x%X %p"
-qxl_client_monitors_config_unsupported_by_device(int qid, int revision) "%d revision=%d"
-qxl_client_monitors_config_capped(int qid, int requested, int limit) "%d %d %d"
-qxl_client_monitors_config_crc(int qid, unsigned size, uint32_t crc32) "%d %u %u"
-qxl_set_client_capabilities_unsupported_by_revision(int qid, int revision) "%d revision=%d"
-
-# qxl-render.c
-qxl_render_blit(int32_t stride, int32_t left, int32_t right, int32_t top, int32_t bottom) "stride=%d [%d, %d, %d, %d]"
-qxl_render_guest_primary_resized(int32_t width, int32_t height, int32_t stride, int32_t bytes_pp, int32_t bits_pp) "%dx%d, stride %d, bpp %d, depth %d"
-qxl_render_update_area_done(void *cookie) "%p"
-
 # vga.c
 vga_std_read_io(uint32_t addr, uint32_t val) "addr 0x%x, val 0x%x"
 vga_std_write_io(uint32_t addr, uint32_t val) "addr 0x%x, val 0x%x"
diff --git a/hw/display/trace-events-qxl b/hw/display/trace-events-qxl
new file mode 100644
index 000000000000..1146bd1640d2
--- /dev/null
+++ b/hw/display/trace-events-qxl
@@ -0,0 +1,66 @@
+# qxl.c
+disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d"
+disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u"
+qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, uint64_t mem, uint32_t format, uint32_t position) "%d %ux%u mem=0x%" PRIx64 " %u,%u"
+qxl_create_guest_primary_rest(int qid, int32_t stride, uint32_t type, uint32_t flags) "%d %d,%d,%d"
+qxl_destroy_primary(int qid) "%d"
+qxl_enter_vga_mode(int qid) "%d"
+qxl_exit_vga_mode(int qid) "%d"
+qxl_hard_reset(int qid, int64_t loadvm) "%d loadvm=%"PRId64
+qxl_interface_async_complete_io(int qid, uint32_t current_async, void *cookie) "%d current=%d cookie=%p"
+qxl_interface_attach_worker(int qid) "%d"
+qxl_interface_get_init_info(int qid) "%d"
+qxl_interface_set_compression_level(int qid, int64_t level) "%d %"PRId64
+qxl_interface_update_area_complete(int qid, uint32_t surface_id, uint32_t dirty_left, uint32_t dirty_right, uint32_t dirty_top, uint32_t dirty_bottom) "%d surface=%d [%d,%d,%d,%d]"
+qxl_interface_update_area_complete_rest(int qid, uint32_t num_updated_rects) "%d #=%d"
+qxl_interface_update_area_complete_overflow(int qid, int max) "%d max=%d"
+qxl_interface_update_area_complete_schedule_bh(int qid, uint32_t num_dirty) "%d #dirty=%d"
+qxl_io_destroy_primary_ignored(int qid, const char *mode) "%d %s"
+qxl_io_log(int qid, const char *log_buf) "%d %s"
+qxl_io_read_unexpected(int qid) "%d"
+qxl_io_unexpected_vga_mode(int qid, uint64_t addr, uint64_t val, const char *desc) "%d 0x%"PRIx64"=%"PRIu64" (%s)"
+qxl_io_write(int qid, const char *mode, uint64_t addr, const char *aname, uint64_t val, unsigned size, int async) "%d %s addr=%"PRIu64 " (%s) val=%"PRIu64" size=%u async=%d"
+qxl_memslot_add_guest(int qid, uint32_t slot_id, uint64_t guest_start, uint64_t guest_end) "%d %u: guest phys 0x%"PRIx64 " - 0x%" PRIx64
+qxl_post_load(int qid, const char *mode) "%d %s"
+qxl_pre_load(int qid) "%d"
+qxl_pre_save(int qid) "%d"
+qxl_reset_surfaces(int qid) "%d"
+qxl_ring_command_check(int qid, const char *mode) "%d %s"
+qxl_ring_command_get(int qid, const char *mode) "%d %s"
+qxl_ring_command_req_notification(int qid) "%d"
+qxl_ring_cursor_check(int qid, const char *mode) "%d %s"
+qxl_ring_cursor_get(int qid, const char *mode) "%d %s"
+qxl_ring_cursor_req_notification(int qid) "%d"
+qxl_ring_res_push(int qid, const char *mode, uint32_t surface_count, uint32_t free_res, void *last_release, const char *notify) "%d %s s#=%d res#=%d last=%p notify=%s"
+qxl_ring_res_push_rest(int qid, uint32_t ring_has, uint32_t ring_size, uint32_t prod, uint32_t cons) "%d ring %d/%d [%d,%d]"
+qxl_ring_res_put(int qid, uint32_t free_res) "%d #res=%d"
+qxl_set_mode(int qid, int modenr, uint32_t x_res, uint32_t y_res, uint32_t bits, uint64_t devmem) "%d mode=%d [ x=%d y=%d @ bpp=%d devmem=0x%" PRIx64 " ]"
+qxl_soft_reset(int qid) "%d"
+qxl_spice_destroy_surfaces_complete(int qid) "%d"
+qxl_spice_destroy_surfaces(int qid, int async) "%d async=%d"
+qxl_spice_destroy_surface_wait_complete(int qid, uint32_t id) "%d sid=%d"
+qxl_spice_destroy_surface_wait(int qid, uint32_t id, int async) "%d sid=%d async=%d"
+qxl_spice_flush_surfaces_async(int qid, uint32_t surface_count, uint32_t num_free_res) "%d s#=%d, res#=%d"
+qxl_spice_monitors_config(int qid) "%d"
+qxl_spice_loadvm_commands(int qid, void *ext, uint32_t count) "%d ext=%p count=%d"
+qxl_spice_oom(int qid) "%d"
+qxl_spice_reset_cursor(int qid) "%d"
+qxl_spice_reset_image_cache(int qid) "%d"
+qxl_spice_reset_memslots(int qid) "%d"
+qxl_spice_update_area(int qid, uint32_t surface_id, uint32_t left, uint32_t right, uint32_t top, uint32_t bottom) "%d sid=%d [%d,%d,%d,%d]"
+qxl_spice_update_area_rest(int qid, uint32_t num_dirty_rects, uint32_t clear_dirty_region) "%d #d=%d clear=%d"
+qxl_surfaces_dirty(int qid, uint64_t offset, uint64_t size) "%d offset=0x%"PRIx64" size=0x%"PRIx64
+qxl_send_events(int qid, uint32_t events) "%d %d"
+qxl_send_events_vm_stopped(int qid, uint32_t events) "%d %d"
+qxl_set_guest_bug(int qid) "%d"
+qxl_interrupt_client_monitors_config(int qid, int num_heads, void *heads) "%d %d %p"
+qxl_client_monitors_config_unsupported_by_guest(int qid, uint32_t int_mask, void *client_monitors_config) "%d 0x%X %p"
+qxl_client_monitors_config_unsupported_by_device(int qid, int revision) "%d revision=%d"
+qxl_client_monitors_config_capped(int qid, int requested, int limit) "%d %d %d"
+qxl_client_monitors_config_crc(int qid, unsigned size, uint32_t crc32) "%d %u %u"
+qxl_set_client_capabilities_unsupported_by_revision(int qid, int revision) "%d revision=%d"
+
+# qxl-render.c
+qxl_render_blit(int32_t stride, int32_t left, int32_t right, int32_t top, int32_t bottom) "stride=%d [%d, %d, %d, %d]"
+qxl_render_guest_primary_resized(int32_t width, int32_t height, int32_t stride, int32_t bytes_pp, int32_t bits_pp) "%dx%d, stride %d, bpp %d, depth %d"
+qxl_render_update_area_done(void *cookie) "%p"
-- 
2.27.0

Re: [PATCH v2 0/4] [RfC] fix tracing for modules

[Stefan Hajnoczi]

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

On Tue, Nov 24, 2020 at 05:02:51PM +0100, Gerd Hoffmann wrote:
> First version that actually works.  Only qxl covered for this RfC, other
> modules will follow once the basics are hashed out.
> 
> More context:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1898700
>   https://bugzilla.redhat.com/show_bug.cgi?id=1869642
> 
> take care,
>   Gerd
> 
> Gerd Hoffmann (4):
>   meson: add trace_events_config[]
>   meson: move up hw subdir (specifically before trace subdir)
>   meson: add module_trace & module_trace_src
>   meson: move qxl trace events to separate file

Awesome, thank you for working on this!

I noticed an issue with simpletrace: the trace file does not contain
qxl_* TRACE_RECORD_TYPE_MAPPING records when ./configure
--enable-modules is used. This happens because st_write_event_mapping()
is called before the qxl module calls trace_event_register_group().

(The mapping records describe the integer ID to string name mapping used
in a simpletrace file.)

You can check this using "grep -a qxl_ trace-$LAST_QEMU_PID" after
running qemu --device qxl built with ./configure --enable-modules
--enable-trace-backend=simple.

Remove --enable-modules and the file will contain the qxl_ trace events.

This means the trace file is broken because the simpletrace records
cannot be mapped to a trace event name.

One way to solve this is by modifying trace_event_register_group() to
call into trace/simple.c (maybe with a TraceEventIter initialized to
iterate over the newly registered trace events group?).

Alternatively, simpletrace.c might be able to emit
TRACE_RECORD_TYPE_MAPPING on demand instead of ahead of time.

Stefan

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

Re: [PATCH v2 0/4] [RfC] fix tracing for modules

[Gerd Hoffmann]

  Hi,

> I noticed an issue with simpletrace: the trace file does not contain
> qxl_* TRACE_RECORD_TYPE_MAPPING records when ./configure
> --enable-modules is used. This happens because st_write_event_mapping()
> is called before the qxl module calls trace_event_register_group().
> 
> (The mapping records describe the integer ID to string name mapping used
> in a simpletrace file.)

It's not the only initialization issue.  "qemu-system-x86_64 -vga qxl
-trace qxl*" doesn't work either for simliar reasons.

> One way to solve this is by modifying trace_event_register_group() to
> call into trace/simple.c (maybe with a TraceEventIter initialized to
> iterate over the newly registered trace events group?).
> 
> Alternatively, simpletrace.c might be able to emit
> TRACE_RECORD_TYPE_MAPPING on demand instead of ahead of time.

I guess I'll have a look at the first option, or more general at
per-group initialization, that looks like the better approach on
a quick glance.

take care,
  Gerd

Re: [PATCH v2 0/4] [RfC] fix tracing for modules

[Gerd Hoffmann]

  Hi,

> One way to solve this is by modifying trace_event_register_group() to
> call into trace/simple.c (maybe with a TraceEventIter initialized to
> iterate over the newly registered trace events group?).

https://git.kraxel.org/cgit/qemu/log/?h=sirius/trace-modules

Not solved yet: -trace "events=qxl*" not working, because
trace_enable_events runs before modules are loaded.

take care,
  Gerd