Re: [Qemu-devel] [PATCH 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property

[Lluís Vilanova]

Lluís Vilanova writes:

> Paolo Bonzini writes:
>> First of all, a generic problem I see with your patches is that the
>> newly-introduced APIs are not providing a good abstraction.

>> If something is only used internally, as is the case for
>> trace_event_get_cpu_id, you don't need accessors.  On the other hand,
>> when you have a repeated expression such as

>> trace_event_get_cpu_id(ev) != trace_event_cpu_count()

>> then you need an API such as trace_event_is_vcpu(ev).

>> Another small ugliness is that you are using "vcpu" in trace-events and
>> in the generated files, but "cpu" in the C file.  My suggestion is to
>> prefix functions with vcpu_trace_event if they refer to per-VCPU trace
>> events, and only use the VCPU ids in those functions.

> I'll fix these two.

BTW, I'd rather keep the getters for this series, if only for the sake of
tracing API consistency (e.g., we already have 'trace_event_get_id()').

I will send a separate series removing the existing superfluous asserts (I won't
be adding more on this series), and can extend it to remove the trivial getters
on the tracing API if that's necessary.


Thanks,
  Lluis