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

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

[Ian Campbell]

QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
log") renamed the stderr trace backend to log, which breaks the xen
build when pointed at a QEMU tree after that point:

./configure of QEMU fail with:
"ERROR: invalid trace backends
        Please choose supported trace backends."

These changes are not (yet) present in qemu-xen-unstable.git and in
any case we want to support QEMU before and after this change. Use the
tracetool.py provided by QEMU to probe for supported trace backends.

This is now done unconditionally (not depending on debug=y), which is
simpler to arrange here but also follows upstream QEMU which in
baf86d6b3ca0 ("trace: switch default backend to "log"") switched the
default from "nop" to "log", so we would have got log in debug=no
builds from then on anyway.

Tested with current qemu-xen-unstable (f165e581d9a6) and current QEMU
upstream master (f075c89f0a9c), the latter picked up via:
    QEMU_UPSTREAM_URL := /path/to/qemu-xen.git
which therefore tested the out of tree build aspect of this change.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

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 \
+		enable_trace_backend='--enable-trace-backend=log'; \
+	elif $$source/scripts/tracetool.py --check-backends --backends stderr ; then \
+		enable_trace_backend='--enable-trace-backend=stderr'; \
+	else \
+		enable_trace_backend='' ; \
+	fi ; \
 	$$source/configure --enable-xen --target-list=i386-softmmu \
 		$(QEMU_XEN_ENABLE_DEBUG) \
+		$$enable_trace_backend \
 		--prefix=$(LIBEXEC) \
 		--libdir=$(LIBEXEC_LIB) \
 		--includedir=$(LIBEXEC_INC) \
-- 
2.1.4

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

[Stefano Stabellini]

On Wed, 10 Feb 2016, Ian Campbell wrote:
> QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
> log") renamed the stderr trace backend to log, which breaks the xen
> build when pointed at a QEMU tree after that point:
> 
> ./configure of QEMU fail with:
> "ERROR: invalid trace backends
>         Please choose supported trace backends."
> 
> These changes are not (yet) present in qemu-xen-unstable.git and in
> any case we want to support QEMU before and after this change. Use the
> tracetool.py provided by QEMU to probe for supported trace backends.
> 
> This is now done unconditionally (not depending on debug=y), which is
> simpler to arrange here but also follows upstream QEMU which in
> baf86d6b3ca0 ("trace: switch default backend to "log"") switched the
> default from "nop" to "log", so we would have got log in debug=no
> builds from then on anyway.
> 
> Tested with current qemu-xen-unstable (f165e581d9a6) and current QEMU
> upstream master (f075c89f0a9c), the latter picked up via:
>     QEMU_UPSTREAM_URL := /path/to/qemu-xen.git
> which therefore tested the out of tree build aspect of this change.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Paul Durrant <paul.durrant@citrix.com>
> Cc: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  tools/Makefile | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> 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.

Maybe:

if $$source/scripts/tracetool.py --check-backends --backends log &>/dev/null
then
    enable_trace_backend='--enable-trace-backend=log'
else
    enable_trace_backend='--enable-trace-backend=stderr'
fi

?


> +		enable_trace_backend='--enable-trace-backend=log'; \
> +	elif $$source/scripts/tracetool.py --check-backends --backends stderr ; then \
> +		enable_trace_backend='--enable-trace-backend=stderr'; \
> +	else \
> +		enable_trace_backend='' ; \
> +	fi ; \
>  	$$source/configure --enable-xen --target-list=i386-softmmu \
>  		$(QEMU_XEN_ENABLE_DEBUG) \
> +		$$enable_trace_backend \
>  		--prefix=$(LIBEXEC) \
>  		--libdir=$(LIBEXEC_LIB) \
>  		--includedir=$(LIBEXEC_INC) \
> -- 
> 2.1.4
> 

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

[Anthony PERARD]

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

> Maybe:
> 
> if $$source/scripts/tracetool.py --check-backends --backends log &>/dev/null
> then
>     enable_trace_backend='--enable-trace-backend=log'
> else
>     enable_trace_backend='--enable-trace-backend=stderr'
> fi
> 
> ?
> 
> 
> > +		enable_trace_backend='--enable-trace-backend=log'; \
> > +	elif $$source/scripts/tracetool.py --check-backends --backends stderr ; then \
> > +		enable_trace_backend='--enable-trace-backend=stderr'; \
> > +	else \
> > +		enable_trace_backend='' ; \
> > +	fi ; \
> >  	$$source/configure --enable-xen --target-list=i386-softmmu \
> >  		$(QEMU_XEN_ENABLE_DEBUG) \
> > +		$$enable_trace_backend \
> >  		--prefix=$(LIBEXEC) \
> >  		--libdir=$(LIBEXEC_LIB) \
> >  		--includedir=$(LIBEXEC_INC) \
> > -- 
> > 2.1.4
> > 

-- 
Anthony PERARD

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

[Ian Campbell]

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?

Ian.

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

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

[Ian Campbell]

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

BTW, I think those correspond to

qemu-mainline == QEMU 2.5.50
qemu-xen-unstable == QEMU 2.4.1
qemu-xen-4.6 == QEMU 2.2.1
qemu-xen-4.5 == QEMU 2.0.2

and the change to use --backends was between 2.0.50 and 2.0.90 (AKA 2.1.0-
rc0).

Do we require that Xen 4.7 will work with QEMU 2.0.x? Given that AFAICT the
oldest QEMU upstream supported version is 2.2.1?

Backporting this fix wasn't something I was envisaging.

Ian.

> 
> That would be preferable to the below, IMHO. Any contrary opinions?
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

On Wed, 10 Feb 2016, 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?

Nope, that would be OK

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

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?

Ian.

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

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

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
>
> BTW, I think those correspond to
>
> qemu-mainline == QEMU 2.5.50
> qemu-xen-unstable == QEMU 2.4.1
> qemu-xen-4.6 == QEMU 2.2.1
> qemu-xen-4.5 == QEMU 2.0.2
>
> and the change to use --backends was between 2.0.50 and 2.0.90 (AKA 2.1.0-
> rc0).
>
> Do we require that Xen 4.7 will work with QEMU 2.0.x? Given that AFAICT the
> oldest QEMU upstream supported version is 2.2.1?

No, but if somebody wants to use qemu-xen-4.5 with xen-unstable, which
is not that crazy given that qemu-xen-4.5 is still supported (even
qemu-xen-4.3 still is), this change in the Makefile would break the
build. Anthony's suggestion is a good compromise.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]

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.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

On Wed, 2016-02-10 at 14:44 +0000, Stefano Stabellini wrote:
> > > > But, if you use '--check-backend --backend' (no 's') instead, the
> > > > check
> > > > would works with qemu-xen >= 4.3
> > 
> > BTW, I think those correspond to
> > 
> > qemu-mainline == QEMU 2.5.50
> > qemu-xen-unstable == QEMU 2.4.1
> > qemu-xen-4.6 == QEMU 2.2.1
> > qemu-xen-4.5 == QEMU 2.0.2
> > 
> > and the change to use --backends was between 2.0.50 and 2.0.90 (AKA
> > 2.1.0-
> > rc0).
> > 
> > Do we require that Xen 4.7 will work with QEMU 2.0.x? Given that AFAICT
> > the
> > oldest QEMU upstream supported version is 2.2.1?
> 
> No, but if somebody wants to use qemu-xen-4.5 with xen-unstable, which
> is not that crazy given that qemu-xen-4.5 is still supported (even
> qemu-xen-4.3 still is), this change in the Makefile would break the
> build.

"supported" is a bit of a stretch IMHO. qemu-xen-4.5 is supported as part
of xen-4.5, not as a standalone independent entity, surely.

>  Anthony's suggestion is a good compromise.

I'd prefer the option given in my second followup suggestion in <1455115410
.19857.167.camel@citrix.com>.

Ian.

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

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

[Ian Campbell]

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".

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?

Ian.

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

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 3195 bytes --]

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."


> 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

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

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