Re: [PATCH] tools: probe for existence of qemu-xen trace backends.

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2016-02-10 at 15:42 +0000, Stefano Stabellini wrote:
> On Wed, 10 Feb 2016, Ian Campbell wrote:
> > On Wed, 2016-02-10 at 14:51 +0000, Stefano Stabellini wrote:
> > > On Wed, 10 Feb 2016, Ian Campbell wrote:
> > > > On Wed, 2016-02-10 at 14:27 +0000, Ian Campbell wrote:
> > > > > On Wed, 2016-02-10 at 14:18 +0000, Anthony PERARD wrote:
> > > > > > On Wed, Feb 10, 2016 at 02:14:13PM +0000, Stefano Stabellini
> > > > > > wrote:
> > > > > > > On Wed, 10 Feb 2016, Ian Campbell wrote:
> > > > > > > > diff --git a/tools/Makefile b/tools/Makefile
> > > > > > > > index 5688a7c..76a2235 100644
> > > > > > > > --- a/tools/Makefile
> > > > > > > > +++ b/tools/Makefile
> > > > > > > > @@ -228,7 +228,7 @@ qemu-xen-dir-force-update: qemu-xen-
> > > > > > > > dir-
> > > > > > > > find
> > > > > > > >  	fi
> > > > > > > >  
> > > > > > > >  ifeq ($(debug),y)
> > > > > > > > -QEMU_XEN_ENABLE_DEBUG := --enable-debug --enable-trace-
> > > > > > > > backend=stderr
> > > > > > > > +QEMU_XEN_ENABLE_DEBUG := --enable-debug
> > > > > > > >  else
> > > > > > > >  QEMU_XEN_ENABLE_DEBUG :=
> > > > > > > >  endif
> > > > > > > > @@ -240,8 +240,16 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-
> > > > > > > > find
> > > > > > > >  		source=.; \
> > > > > > > >  	fi; \
> > > > > > > >  	cd qemu-xen-dir; \
> > > > > > > > +	if $$source/scripts/tracetool.py --check-backends
> > > > > > > > --
> > > > > > > > backends 
> > > > > > > > log ; then \
> > > > > > > 
> > > > > > > --check-backends only works on qemu-xen >= 4.6, on the other
> > > > > > > hand
> > > > > > > we
> > > > > > > know that qemu-xen < 4.6 supports stderr.
> > > > > > 
> > > > > > But, if you use '--check-backend --backend' (no 's') instead,
> > > > > > the
> > > > > > check
> > > > > > would works with qemu-xen >= 4.3
> > > > > 
> > > > > That would be preferable to the below, IMHO. Any contrary
> > > > > opinions?
> > > > 
> > > > Actually, given that the upstream default is now "log", how about:
> > > > 
> > > >         if $$source/scripts/tracetool.py --check-backend --backend
> > > > stderr ; then \
> > > > 		enable_trace_backend='--enable-trace-
> > > > backend=stderr'; \
> > > > 	else \		enable_trace_backend='' ; \
> > > > 	
> > > >         fi ; \
> > > > 
> > > > IOW just accept the upstream default (be it log or whatever comes
> > > > next)
> > > > from this version onwards?
> > > 
> > > The risk associated with that is that in the past QEMU recommended
> > > 'simple', which has a binary output to a separate file.
> > 
> > I can't see any evidence of that in the git log over configure (which
> > has
> > trace_backend="<thing>" in it. The configure file gained this option in
> > "trace: Add trace-events file for declaring trace events" with "nop" as
> > the
> > default (circa QEMU 0.13.50) which it was until the recent switch to
> > "log".
> 
> From docs/tracing.txt:
> 
> "The "simple" backend supports common use cases and comes as part of the QEMU
> source tree.  It may not be as powerful as platform-specific or third-party
> trace backends but it is portable.  This is the recommended trace backend
> unless you have specific needs for more advanced backends."

Ah, I read "enabled by default" when you only said "recommended".

I don't think we need to care about this recommendation -- a user would
have to take a proactive step at compile time to actually follow this
recommendation.
> 
> > In any case what harm does have some other upstream default enable do?
> > AIUI
> > you need to also enable specific tracing on the QEMU command line for
> > it to
> > be useful, surely no version of QEMU was dumping some binary trace file
> > into $CWD by default?
> 
> This is true

Ian./

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel