Daniel P Berrange writes:
> Since there will shortly be multiple event groups allowed,
> we can no longer use the TraceEventID and TraceEventVCPUID
> enums in the trace control APIs. There will in fact be
> multiple distinct enums, and the enum values will only be
> required to be unique per group.
This patch serves no purpose without the event group patches.
Also, AFAIR TraceEventVCPUID still needs to be a flat space (they're all used as
bitmask indexes), so keeping the enum won't lose any re-compilation benefit.
And without wanting to sound like a broken record, you can make the
"TRACE_${EVENTNAME}" IDs be global Event* variables (statically initialized in
"trace/generated-events.c"). That still allows using their names in the macros,
avoids having a (two-level) tree of events, and eliminates the need for the
Event::id member (and the trace_event_get_id() function).
Cheers,
Lluis
[...]
> diff --git a/trace/simple.c b/trace/simple.c
> index 2f09daf..6e8013c 100644
> --- a/trace/simple.c
> +++ b/trace/simple.c
> @@ -18,7 +18,7 @@
> #include "trace/simple.h"
> /** Trace file header event ID */
> -#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with
> TraceEventIDs */
> +#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with event IDs
> */
> /** Trace file magic number */
> #define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
> @@ -58,7 +58,7 @@ static char *trace_file_name;
> /* * Trace buffer entry */
> typedef struct {
> - uint64_t event; /* TraceEventID */
> + uint64_t event; /* event ID */
> uint64_t timestamp_ns;
> uint32_t length; /* in bytes */
> uint32_t pid;
> @@ -202,7 +202,7 @@ void trace_record_write_str(TraceBufferRecord *rec, const
> char *s, uint32_t slen
rec-> rec_off = write_to_buffer(rec->rec_off, (void*)s, slen);
> }
> -int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_t
> datasize)
> +int trace_record_start(TraceBufferRecord *rec, uint32_t event, size_t
> datasize)
> {
> unsigned int idx, rec_off, old_idx, new_idx;
> uint32_t rec_len = sizeof(TraceRecord) + datasize;
> diff --git a/trace/simple.h b/trace/simple.h
> index 1e7de45..17ce472 100644
> --- a/trace/simple.h
> +++ b/trace/simple.h
> @@ -33,7 +33,7 @@ typedef struct {
> *
> * @arglen number of bytes required for arguments
> */
> -int trace_record_start(TraceBufferRecord *rec, TraceEventID id, size_t
> arglen);
> +int trace_record_start(TraceBufferRecord *rec, uint32_t id, size_t arglen);
> /**
> * Append a 64-bit argument to a trace record
Not incorrect, but it's weird that the simple backend emits 64-bit identifiers
while QEMU uses 32-bit ones.
Cheers,
Lluis
