On Fri, Jul 28, 2017 at 12:05:44PM +0100, Daniel P. Berrange wrote:
> On Fri, Jul 28, 2017 at 10:20:53AM +0100, Stefan Hajnoczi wrote:
> > Code that checks dstate is unaware of SystemTap and LTTng UST dstate, so
> > the following trace event will not fire when solely enabled by SystemTap
> > or LTTng UST:
> >
> > if (trace_event_get_state(TRACE_MY_EVENT)) {
> > str = g_strdup_printf("Expensive string to generate ...",
> > ...);
> > trace_my_event(str);
> > g_free(str);
> > }
> >
> > Most users of trace_event_get_state() want to know both QEMU and backend
> > dstate for the event. Update the macro to include backend dstate.
> >
> > Introduce trace_event_get_state_qemu() for those callers who really only
> > want QEMU dstate. This includes the trace backends (like 'log') which
> > should only trigger when event are enabled via QEMU.
>
> I'm not convinced this is quite right as is.
>
> The QEMU state for an event is set via cli flags to -trace and that
> is intended for use with with things like simpletrace or log which
> have no other way to filter which events are turned on at runtime.
> For these calling trace_event_get_state_dynamic_by_id() is right.
>
> For DTrace / LTT-NG, and event is active if the kernel has turned
> on the dynamic probe location. Any command line args like -trace
> should not influence this at all, so we should *not* involve QEMU
> state at all - only the backend state. So for these we should only
> be calling the backend specific test and ignoring
> trace_event_get_state_dynamic_by_id() entirely. Your patch though
> makes it consider both.
I think you're referring to this:
#define trace_event_get_state(id) \
((id ##_ENABLED) && (trace_event_get_state_dynamic_by_id(id) || \
id ##_BACKEND_DSTATE()))
We could change it to:
#define trace_event_get_state(id) \
((id ##_ENABLED) && id ##_BACKEND_DSTATE())
and modify the log, syslog, ftrace, etc backends to emit
trace_event_get_state_dynamic_by_id(id) as their backend dstate.
However, in the multi-backend case with ./configure
--enable-trace-backends=log,syslog this expands to:
((id ##_ENABLED) && (trace_event_get_state_dynamic_by_id(id) || \
trace_event_get_state_dynamic_by_id(id) || \
false))
I couldn't bring myself to have multiple calls to
trace_event_get_state_dynamic_by_id() :).
What do you think?
signature.asc
Description: PGP signature
