Re: [Qemu-devel] [PATCH v2 1/2] trace: add TRACE_<event>_BACKEND_DSTATE()

["Daniel P. Berrange" <berrange@redhat.com>]

On Mon, Jul 31, 2017 at 05:35:11PM +0100, Stefan Hajnoczi wrote:
> On Mon, Jul 31, 2017 at 04:16:39PM +0100, Daniel P. Berrange wrote:
> > On Mon, Jul 31, 2017 at 03:07:17PM +0100, Stefan Hajnoczi wrote:
> > > diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
> > > index c6812b70a2..17f902cc62 100644
> > > --- a/scripts/tracetool/backend/dtrace.py
> > > +++ b/scripts/tracetool/backend/dtrace.py
> > > @@ -44,8 +44,20 @@ def generate_h_begin(events, group):
> > >      out('#include "%s"' % header,
> > >          '')
> > >  
> > > +    # SystemTap defines <provider>_<name>_ENABLED() but other DTrace
> > > +    # implementations might not.
> > > +    for e in events:
> > > +        out('#ifndef QEMU_%(uppername)s_ENABLED',
> > > +            '#define QEMU_%(uppername)s_ENABLED() false',
> > > +            '#endif',
> > > +            uppername=e.name.upper())
> > 
> > 
> > IIUC, this means that on other DTrace impls, any trace points guarded by
> > QEMU_*_ENABLED() will never run, even if DTrace probes are set. Having
> > some trace points silently never run makes it pretty useless IMHO.
> > 
> > IOW, you need the opposite, #define it to true. The probe will still
> > only be executed if DTrace has enabled it, but you'll have to take the
> > hit of evaluating the probe arguments regardless. Not as optimized
> > performance-wise, but functionally correct at least.
> 
> Good point!  Thank you, will fix when merging.

Ok, You can add

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

if the change of false to true is made.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|