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

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Wed, Sep 28, 2016 at 02:08:03PM +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
> 
> 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-3
> 
> 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 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

Great, this looks very close.  There are only a few comments from Lluís
and me.

Stefan

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