| Message ID | 1455108154-12084-1-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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 > >
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.
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
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
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.
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.
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.
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.
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.
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
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./
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) \
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(-)