Re: [Qemu-devel] [PATCH v3 13/18] trace: dynamically allocate event IDs at runtime

[Lluís Vilanova]

Daniel P Berrange writes:

> On Mon, Sep 19, 2016 at 08:05:12PM +0200, Lluís Vilanova wrote:
>> Daniel P Berrange writes:
>> 
>> > Instead of having the code generator assign event IDs and
>> > event VCPU IDs, assign them when the events are registered
>> > at runtime. This will allow us allow code to be generated
>> > from individual trace-events without having to figure out
>> > globally unique numbering at build time.
>> 
>> > Signed-off-by: Daniel P. Berrange <address@hidden>
>> > ---
>> >  scripts/tracetool/format/events_c.py | 10 ++--------
>> >  scripts/tracetool/format/events_h.py |  7 -------
>> >  trace/control.c                      | 11 ++++++++++-
>> >  3 files changed, 12 insertions(+), 16 deletions(-)
>> 
>> > diff --git a/scripts/tracetool/format/events_c.py 
>> > b/scripts/tracetool/format/events_c.py
>> > index 7f00b50..1b5790d 100644
>> > --- a/scripts/tracetool/format/events_c.py
>> > +++ b/scripts/tracetool/format/events_c.py
>> > @@ -28,25 +28,19 @@ def generate(events, backend):
>> >      for e in events:
>> >          out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
>> 
>> > -    next_id = 0
>> > -    next_vcpu_id = 0
>> >      for e in events:
>> > -        id = next_id
>> > -        next_id += 1
>> >          if "vcpu" in e.properties:
>> > -            vcpu_id = next_vcpu_id
>> > -            next_vcpu_id += 1
>> > +            vcpu_id = 0
>> >          else:
>> >              vcpu_id = "TRACE_VCPU_EVENT_NONE"
>> >          out('TraceEvent %(event)s = {',
>> > -            '  .id = %(id)s,',
>> > +            '  .id = 0,',
>> 
>> Better to use some invalid value, like in TRACE_VCPU_EVENT_NONE.

> I don't think it adds anything todo that - TRACE_VCPU_EVENT_NONE is
> special because it is a persistent value for lifetime of QEMU
> that will be used by runtime comparisons. In contrast this is
> just a placeholder that gets replaced by the real value immediately
> at startup in trace_event_register_group().  We never even check
> the statically set value of 'id' - we just unconditionally replace
> it with the real value.

Makes sense.

Thanks,
  Lluis