Re: [Qemu-devel] [PATCH v6 00/20] Refactor trace to allow modular build

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Tue, Oct 04, 2016 at 02:35:39PM +0100, Daniel P. Berrange wrote:
> These patches were previously posted as part of my giant
> trace events modular build series
> 
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg01714.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg03335.html
>   v3: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04282.html
>   v4: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg05467.html
>   v5: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg07435.html
> 
> This series does all the refactoring required to support a fully
> modular build of the trace probe points, but does not actually
> convert anything to use it. The 40+ patches to convert each subdir
> to use modular build will only be posted again, once the refactoring
> is reviewed & queued, in order to avoid patch-bombing the list
> more than is needed. The full series is visible at
> 
>    https://github.com/berrange/qemu/tree/trace-events-6
> 
> The key problem being tackled by this series is the assumption
> that there is a single statically declared enum which provides
> globally unique event IDs. Inside QEMU the event IDs were used
> as list indexes into the 'trace_events' array, while the event
> VCPU IDs were used as bitmap indexes in trace_dstate against
> the CPUState struct.  Externally to QEMU, the event IDs were
> also written in the simpletrace binary data format and used
> to lookup the entry in the trace-events file afterwards.
> 
> Inside QEMU the refactoring work managed to remove all need
> for event IDs for purposes of 'trace_events' array lookups.
> Instead we now have global variables per-event which can be
> referenced directly.
> 
> When QEMU starts up and the various event groups are registered,
> we now dynamically assign event IDs and VCPU IDs to each event.
> This removes the limitation in the v1 posting that all vCPU
> events had to be in one file. We also removed the limitation
> on the total number of vCPU events. So there is no regression
> in functionality of VCPU event support compared to current
> GIT master.
> Since the event IDs are allocated dynamically at runtime,
> the simpletrace.py script cannot assume they map directly
> to the 'trace-events' file entries. Thus, the simpletrace
> binary format is extended to include a record type that
> maps trace event IDs to trace event names. While it would
> be possible to take this even further and make the
> simpletrace binary format 100% self-describing this is left
> as an exercise for future developers, as it is not a
> pre-requisite for this modular build.
> 
> While some of the intermediate patches may seem pointless
> on their own, they exist in order to facilitate the review
> of later patches by ensuring each patch does the minimum
> possible refactoring work.
> 
> Changed in v6:
> 
>  - Fix mixed up break vs return in event match (Lluís)
>  - Fixed indentation of generated code (Lluís)
>  - Unpack tuple return value (Stefan)
>  - Misc typos / docs formatting (Stefan / Lluís)
>  
> 
> Changed in v5:
> 
>  - Use single '_' instead of double/triple '_' in
>    constants (Lluís)
>  - Use more pythonic loop iterator (Lluís)
>  - Misc typos (Lluís/Stefan)
>  - Fix filtering of QMP trace events (Lluís)
>  - Fix some new mistakes in trace-events in master
> 
> Changed in v4:
> 
>  - Misc typos / indentation (Lluís, Eric)
>  - Simplify do_trace_enable_events (Lluís)
>  - Use Event.api() in more places (Lluís)
>  - Actually delete events_{h,c}.py (Lluís)
>  - Use bitmap_new() for allocation (Lluís)
>  - Dont use .git to make relative path (Lluís)
> 
> Changed in v3:
> 
>  - Change simpletrace format to write a mapping of
>    IDs to names
>  - Declare a TraceEvent variable per event, instead
>    of just an array
>  - Make 'trace_events' array be 'TraceEvent **'
>    instead of 'TraceEvent *'
>  - Fix infinite loop in iterators due to bad
>    fix in v2 posting.
>  - Dynamically allocate the trace_dstate variable
>    in CPUState based on actual number of vcpu events
>    registered at runtime
>  - Push logic for determining group name into the
>    tracetool program
>  - Fix to ftrace to make it more practical for users
>    who sometimes run QEMU as non-root.
>  - Get rid of TraceEventID/VCPUID enums entirely
> 
> Changed in v2:
> 
>  - Fixed filtering of events on first call of
>    iterator_next (Stefan)
>  - Switch from size_t to uint32_t for event ID
>    type (Paolo/Stefan)
>  - Replace global 'dstate' array with individual
>    variables (Lluís)
> 
> Daniel P. Berrange (20):
>   trace: move colo trace events to net/ sub-directory
>   trace: remove double-underscore in event name
>   trace: add trace event iterator APIs
>   trace: convert code to use event iterators
>   trace: remove some now unused functions
>   trace: remove global 'uint16 dstate[]' array
>   trace: remove duplicate control.h includes in generated-tracers.h
>   trace: break circular dependency in event-internal.h
>   trace: give each trace event a named TraceEvent struct
>   trace: remove the TraceEventID and TraceEventVCPUID enums
>   trace: emit name <-> ID mapping in simpletrace header
>   trace: don't abort qemu if ftrace can't be initialized
>   trace: provide mechanism for registering trace events
>   trace: dynamically allocate trace_dstate in CPUState
>   trace: dynamically allocate event IDs at runtime
>   trace: get rid of generated-events.h/generated-events.c
>   trace: rename _read_events to read_events
>   trace: push reading of events up a level to tracetool main
>   trace: pass trace-events to tracetool as a positional param
>   trace: introduce a formal group name for trace events
> 
>  Makefile                                         |   3 -
>  Makefile.target                                  |   6 +-
>  hw/scsi/spapr_vscsi.c                            |   2 +-
>  hw/scsi/trace-events                             |   2 +-
>  include/qemu/module.h                            |   2 +
>  include/qom/cpu.h                                |   9 +-
>  include/trace-tcg.h                              |   1 -
>  include/trace.h                                  |   1 -
>  monitor.c                                        |  26 ++---
>  net/trace-events                                 |  16 +++
>  qemu-img.c                                       |   1 +
>  qemu-io.c                                        |   1 +
>  qemu-nbd.c                                       |   1 +
>  qom/cpu.c                                        |   7 +-
>  scripts/simpletrace.py                           |  56 +++++++----
>  scripts/tracetool.py                             |  20 +++-
>  scripts/tracetool/__init__.py                    |  28 ++++--
>  scripts/tracetool/backend/__init__.py            |  12 +--
>  scripts/tracetool/backend/dtrace.py              |   4 +-
>  scripts/tracetool/backend/ftrace.py              |   5 +-
>  scripts/tracetool/backend/log.py                 |   7 +-
>  scripts/tracetool/backend/simple.py              |  12 +--
>  scripts/tracetool/backend/syslog.py              |   5 +-
>  scripts/tracetool/backend/ust.py                 |   4 +-
>  scripts/tracetool/format/__init__.py             |   4 +-
>  scripts/tracetool/format/c.py                    |  56 +++++++++--
>  scripts/tracetool/format/d.py                    |   2 +-
>  scripts/tracetool/format/events_c.py             |  44 --------
>  scripts/tracetool/format/events_h.py             |  60 -----------
>  scripts/tracetool/format/h.py                    |  37 +++++--
>  scripts/tracetool/format/simpletrace_stap.py     |  26 ++++-
>  scripts/tracetool/format/stap.py                 |   2 +-
>  scripts/tracetool/format/tcg_h.py                |   8 +-
>  scripts/tracetool/format/tcg_helper_c.py         |   2 +-
>  scripts/tracetool/format/tcg_helper_h.py         |   2 +-
>  scripts/tracetool/format/tcg_helper_wrapper_h.py |   2 +-
>  scripts/tracetool/format/ust_events_c.py         |   2 +-
>  scripts/tracetool/format/ust_events_h.py         |   9 +-
>  stubs/trace-control.c                            |   9 +-
>  trace-events                                     |  16 ---
>  trace/Makefile.objs                              |  46 +++------
>  trace/control-internal.h                         |  48 ++++-----
>  trace/control-target.c                           |  35 +++----
>  trace/control.c                                  | 123 +++++++++++++++--------
>  trace/control.h                                  | 112 +++++++--------------
>  trace/event-internal.h                           |  18 +++-
>  trace/ftrace.c                                   |   6 ++
>  trace/qmp.c                                      |  16 +--
>  trace/simple.c                                   |  41 ++++++--
>  trace/simple.h                                   |   6 +-
>  vl.c                                             |   2 +
>  51 files changed, 502 insertions(+), 463 deletions(-)
>  delete mode 100644 scripts/tracetool/format/events_c.py
>  delete mode 100644 scripts/tracetool/format/events_h.py

The SystemTap simpletrace breakage I mentioned isn't introduced by this
series (I think?).  I've applied it for now and will include it in the
next pull request unless issues are discovered.

Thanks, applied to my master tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

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