* [PATCH] tools/qemu-xen: remove CFLAGS for qemu build @ 2012-02-23 16:45 Olaf Hering 2012-03-01 17:42 ` Ian Jackson 0 siblings, 1 reply; 9+ messages in thread From: Olaf Hering @ 2012-02-23 16:45 UTC (permalink / raw) To: xen-devel # HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1330015545 -3600 # Node ID 5bdbdcb03d60a7b58f41306ef39cb988100efbe4 # Parent 56214b978466914c1b9f8adb1158a3217a823e42 tools/qemu-xen: remove CFLAGS for qemu build Currently qemu-xen gets build with CFLAGS only if CFLAGS was already in the environment during make invocation. If CFLAGS is in environment then make will append all of the various flags specified in xen Makefiles, which is then passed to qemu configure. If CFLAGS is not set, then configure will use just "-O2 -g" because make does not export its own CFLAGS variable. To make qemu-xen build consistent this change removes CFLAGS from the environment so that only the CFLAGS from qemu configure script will be used. This matches what is done in kvm.rpm and qemu.rpm where for example RPM_OPT_FLAGS is not passes as CFLAGS. Otherwise those packages would not build as well. Passing makes CFLAGS to configure will lead to build errors: - xen Makefiles append -std=gnu99, this breaks qemu build due to a bug in header file: fpu/softfloat-specialize.h:107: error: initializer element is not constant - in 32bit builds, qemus configure script will append -mcpu=i486 in an odd way, which leads to unknown gcc cmdline options due to a missing space - xen Makefiles will append -Wall which will expose all sorts of style issues in the qemu code - in one case some of the asm() blocks will not compile with gcc 4.6 in openSuSE 12.1 Until upstream qemu has fixed all these issues use no extra CFLAGS to configure qemu-xen. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 56214b978466 -r 5bdbdcb03d60 tools/Makefile --- a/tools/Makefile +++ b/tools/Makefile @@ -146,6 +146,7 @@ subdir-all-qemu-xen-dir subdir-install-q source=.; \ fi; \ cd qemu-xen-dir; \ + env -u CFLAGS \ $$source/configure --enable-xen --target-list=i386-softmmu \ --source-path=$$source \ --extra-cflags="-I$(XEN_ROOT)/tools/include \ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-02-23 16:45 [PATCH] tools/qemu-xen: remove CFLAGS for qemu build Olaf Hering @ 2012-03-01 17:42 ` Ian Jackson 2012-03-01 17:53 ` Olaf Hering 0 siblings, 1 reply; 9+ messages in thread From: Ian Jackson @ 2012-03-01 17:42 UTC (permalink / raw) To: Olaf Hering; +Cc: xen-devel Olaf Hering writes ("[Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"): > tools/qemu-xen: remove CFLAGS for qemu build > > Currently qemu-xen gets build with CFLAGS only if CFLAGS was already in > the environment during make invocation. If CFLAGS is in environment then > make will append all of the various flags specified in xen Makefiles, > which is then passed to qemu configure. If CFLAGS is not set, then > configure will use just "-O2 -g" because make does not export its own > CFLAGS variable. > > To make qemu-xen build consistent this change removes CFLAGS from the > environment so that only the CFLAGS from qemu configure script will be > used. This matches what is done in kvm.rpm and qemu.rpm where for > example RPM_OPT_FLAGS is not passes as CFLAGS. Otherwise those packages > would not build as well. I think it is a bug to try to invoke the Xen build system with CFLAGS set. So I don't think it's a good idea to try to fix it up. Ian. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-03-01 17:42 ` Ian Jackson @ 2012-03-01 17:53 ` Olaf Hering 2012-03-01 18:20 ` Ian Jackson 0 siblings, 1 reply; 9+ messages in thread From: Olaf Hering @ 2012-03-01 17:53 UTC (permalink / raw) To: Ian Jackson; +Cc: xen-devel On Thu, Mar 01, Ian Jackson wrote: > Olaf Hering writes ("[Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"): > > tools/qemu-xen: remove CFLAGS for qemu build > > > > Currently qemu-xen gets build with CFLAGS only if CFLAGS was already in > > the environment during make invocation. If CFLAGS is in environment then > > make will append all of the various flags specified in xen Makefiles, > > which is then passed to qemu configure. If CFLAGS is not set, then > > configure will use just "-O2 -g" because make does not export its own > > CFLAGS variable. > > > > To make qemu-xen build consistent this change removes CFLAGS from the > > environment so that only the CFLAGS from qemu configure script will be > > used. This matches what is done in kvm.rpm and qemu.rpm where for > > example RPM_OPT_FLAGS is not passes as CFLAGS. Otherwise those packages > > would not build as well. > > I think it is a bug to try to invoke the Xen build system with CFLAGS > set. So I don't think it's a good idea to try to fix it up. Its a way to pass RPM_OPT_FLAGS to build the tools. This includes things like fortify-source (whatever the exact spelling is) and other global flags to build a distro. What other ways exist to pass that to the buildsystem for the tools? Olaf ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-03-01 17:53 ` Olaf Hering @ 2012-03-01 18:20 ` Ian Jackson 2012-03-01 19:08 ` Olaf Hering 2012-03-02 11:51 ` Olaf Hering 0 siblings, 2 replies; 9+ messages in thread From: Ian Jackson @ 2012-03-01 18:20 UTC (permalink / raw) To: Olaf Hering; +Cc: xen-devel@lists.xensource.com Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"): > Its a way to pass RPM_OPT_FLAGS to build the tools. This includes things > like fortify-source (whatever the exact spelling is) and other global > flags to build a distro. > What other ways exist to pass that to the buildsystem for the tools? This is what EXTRA_CFLAGS and PREPEND_INCLUDES and so forth are for. Ian. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-03-01 18:20 ` Ian Jackson @ 2012-03-01 19:08 ` Olaf Hering 2012-03-02 11:51 ` Olaf Hering 1 sibling, 0 replies; 9+ messages in thread From: Olaf Hering @ 2012-03-01 19:08 UTC (permalink / raw) To: Ian Jackson; +Cc: xen-devel@lists.xensource.com On Thu, Mar 01, Ian Jackson wrote: > Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"): > > Its a way to pass RPM_OPT_FLAGS to build the tools. This includes things > > like fortify-source (whatever the exact spelling is) and other global > > flags to build a distro. > > What other ways exist to pass that to the buildsystem for the tools? > > This is what EXTRA_CFLAGS and PREPEND_INCLUDES and so forth are for. With EXTRA_CFLAGS there is even more breakage: /usr/src/packages/BUILD/xen-4.2.24911/non-dbg/tools/firmware/etherboot/ipxe/src/arch/i386/interface/pcbios/int13.c:1729: undefined reference to `__stack_chk_fail' I will have a look tomorrow. Olaf ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-03-01 18:20 ` Ian Jackson 2012-03-01 19:08 ` Olaf Hering @ 2012-03-02 11:51 ` Olaf Hering 2012-03-14 12:04 ` Ian Jackson 1 sibling, 1 reply; 9+ messages in thread From: Olaf Hering @ 2012-03-02 11:51 UTC (permalink / raw) To: Ian Jackson, Roger Pau Monne; +Cc: xen-devel@lists.xensource.com On Thu, Mar 01, Ian Jackson wrote: > Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"): > > Its a way to pass RPM_OPT_FLAGS to build the tools. This includes things > > like fortify-source (whatever the exact spelling is) and other global > > flags to build a distro. > > What other ways exist to pass that to the buildsystem for the tools? > > This is what EXTRA_CFLAGS and PREPEND_INCLUDES and so forth are for. EXTRA_CFLAGS is only used by ipxe. I think that there should be a way to pass individual external CFLAGS to the tools, ipxe, qemu-traditional, qemu-xen, etc builds. >From a distro perspective, its required to build libraries and binaries with certain global cflags. Up to the point when qemu-upstream was imported it worked as expected by exporting CFLAGS before 'make tools'. Now qemu-upstream reuses these CFLAGS, but it cant deal with the result. How about something like this: env \ EXTRA_CFLAGS_XEN_TOOLS="$RPM_OPT_FLAGS" \ EXTRA_CFLAGS_QEMU_UPSTREAM="" \ EXTRA_CFLAGS_IPXE="" \ ./configure <more options> Then configure can pass these to the various Makefiles. In case of qemu its configure could be invoked like env \ CFLAGS="$(EXTRA_CFLAGS_QEMU_UPSTREAM)" \ $source/configure <qemu options> In case of ipxe it would be "EXTRA_CFLAGS=$(EXTRA_CFLAGS_IPXE)". I think you get the idea. Right now I see the need for external CFLAGS only for tools itself, but if the options are there one can use them also for the other targets if there is a demand for it. Olaf ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-03-02 11:51 ` Olaf Hering @ 2012-03-14 12:04 ` Ian Jackson 2012-03-14 16:46 ` Olaf Hering 0 siblings, 1 reply; 9+ messages in thread From: Ian Jackson @ 2012-03-14 12:04 UTC (permalink / raw) To: Olaf Hering; +Cc: Roger Pau Monne, xen-devel@lists.xensource.com Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"): > I think that there should be a way to pass individual external CFLAGS to > the tools, ipxe, qemu-traditional, qemu-xen, etc builds. Yes, but I think that's already supposed to exist. PREPEND_CFLAGS and APPEND_CFLAGS. Look in tools/Rules.mk. > From a distro perspective, its required to build libraries and binaries > with certain global cflags. Up to the point when qemu-upstream was > imported it worked as expected by exporting CFLAGS before 'make tools'. > Now qemu-upstream reuses these CFLAGS, but it cant deal with the result. > > How about something like this: > env \ > EXTRA_CFLAGS_XEN_TOOLS="$RPM_OPT_FLAGS" \ > EXTRA_CFLAGS_QEMU_UPSTREAM="" \ > EXTRA_CFLAGS_IPXE="" \ > ./configure <more options> It doesn't need to be plumbed through configure; it can just be used directly by make, although configure should use it for its tests I guess and probably doesn't right now. Ian. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-03-14 12:04 ` Ian Jackson @ 2012-03-14 16:46 ` Olaf Hering 2012-03-14 18:07 ` Olaf Hering 0 siblings, 1 reply; 9+ messages in thread From: Olaf Hering @ 2012-03-14 16:46 UTC (permalink / raw) To: Ian Jackson; +Cc: Roger Pau Monne, xen-devel@lists.xensource.com On Wed, Mar 14, Ian Jackson wrote: > Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"): > > I think that there should be a way to pass individual external CFLAGS to > > the tools, ipxe, qemu-traditional, qemu-xen, etc builds. > > Yes, but I think that's already supposed to exist. PREPEND_CFLAGS and > APPEND_CFLAGS. Look in tools/Rules.mk. PREPEND_CFLAGS sets just CFLAGS inside configure. APPEND_CFLAGS is the wrong place, the Makefiles must be able to override whats already in CFLAGS. > > From a distro perspective, its required to build libraries and binaries > > with certain global cflags. Up to the point when qemu-upstream was > > imported it worked as expected by exporting CFLAGS before 'make tools'. > > Now qemu-upstream reuses these CFLAGS, but it cant deal with the result. > > > > How about something like this: > > env \ > > EXTRA_CFLAGS_XEN_TOOLS="$RPM_OPT_FLAGS" \ > > EXTRA_CFLAGS_QEMU_UPSTREAM="" \ > > EXTRA_CFLAGS_IPXE="" \ > > ./configure <more options> > > It doesn't need to be plumbed through configure; it can just be used > directly by make, although configure should use it for its tests I > guess and probably doesn't right now. My idea is to use configure to write the desired strings into the Makefiles. The change below mostly works. I'm using it like this in xen.spec: env \ EXTRA_CFLAGS_XEN_TOOLS="${RPM_OPT_FLAGS} -DEXTRA_CFLAGS_XEN_TOOLS" \ EXTRA_CFLAGS_QEMU_TRADITIONAL="${RPM_OPT_FLAGS} -DEXTRA_CFLAGS_QEMU_TRADITIONAL" \ EXTRA_CFLAGS_QEMU_XEN="${RPM_OPT_FLAGS} -DEXTRA_CFLAGS_QEMU_XEN" \ ./configure \ --libdir=%{_libdir} \ --prefix=/usr However, there are a few issues with this approach: Now that CFLAGS starts as empty variable in config/Tools.mk, this file has to be included first. Therefore the order of include in tools/Rules.mk is changed. I will send patches to fix affected Makefiles, so far its only tools/libfsimage/zfs/Makefile. I will prepare a patch to move the include tools/libfsimage/Rules.mk in all libfsimage Makefiles. I noticed that @F is empty now, leading to ..d files in all directories. No idea how to fix that. tools/Rules.mk:CFLAGS += -MMD -MF .$(@F).d Unrelated to this change: in config/Tools.mk debug is forced to 'y'. I think the logic in tools/m4/{enable,disable}_feature.m4 is flipped. What do you think about the general approach of this patch? Olaf # HG changeset patch # Parent 773d0367087212c43faf8cdcc21cf443b1ea0046 diff -r 773d03670872 config/Tools.mk.in --- a/config/Tools.mk.in +++ b/config/Tools.mk.in @@ -23,6 +23,10 @@ PREPEND_LIB := @PREPEND_LIB@ APPEND_INCLUDES := @APPEND_INCLUDES@ APPEND_LIB := @APPEND_LIB@ +CFLAGS := @EXTRA_CFLAGS_XEN_TOOLS@ +EXTRA_CFLAGS_QEMU_TRADITIONAL := @EXTRA_CFLAGS_QEMU_TRADITIONAL@ +EXTRA_CFLAGS_QEMU_XEN := @EXTRA_CFLAGS_QEMU_XEN@ + # Download GIT repositories via HTTP or GIT's own protocol? # GIT's protocol is faster and more robust, when it works at all (firewalls # may block it). We make it the default, but if your GIT repository downloads diff -r 773d03670872 tools/Makefile --- a/tools/Makefile +++ b/tools/Makefile @@ -122,6 +122,8 @@ subdir-all-qemu-xen-traditional-dir subd set -e; \ $(buildmakevars2shellvars); \ cd qemu-xen-traditional-dir; \ + env CFLAGS="$(EXTRA_CFLAGS_QEMU_TRADITIONAL)" \ + PREFIX="$(PREFIX)" \ $(QEMU_ROOT)/xen-setup $(IOEMU_CONFIGURE_CROSS); \ $(MAKE) install @@ -147,6 +149,7 @@ subdir-all-qemu-xen-dir subdir-install-q source=.; \ fi; \ cd qemu-xen-dir; \ + env CFLAGS="$(EXTRA_CFLAGS_QEMU_XEN)" \ $$source/configure --enable-xen --target-list=i386-softmmu \ --source-path=$$source \ --extra-cflags="-I$(XEN_ROOT)/tools/include \ diff -r 773d03670872 tools/Rules.mk --- a/tools/Rules.mk +++ b/tools/Rules.mk @@ -3,8 +3,8 @@ # `all' is the default target all: +include $(XEN_ROOT)/config/Tools.mk include $(XEN_ROOT)/Config.mk -include $(XEN_ROOT)/config/Tools.mk export _INSTALL := $(INSTALL) INSTALL = $(XEN_ROOT)/tools/cross-install diff -r 773d03670872 tools/configure.ac --- a/tools/configure.ac +++ b/tools/configure.ac @@ -54,6 +54,12 @@ AC_ARG_VAR([APPEND_INCLUDES], [List of include folders to append to CFLAGS (without -I)]) AC_ARG_VAR([APPEND_LIB], [List of library folders to append to LDFLAGS (without -L)]) +AC_ARG_VAR([EXTRA_CFLAGS_XEN_TOOLS], + [Extra CFLAGS to build tools]) +AC_ARG_VAR([EXTRA_CFLAGS_QEMU_TRADITIONAL], + [Extra CFLAGS to build qemu-traditional]) +AC_ARG_VAR([EXTRA_CFLAGS_QEMU_XEN], + [Extra CFLAGS to build qemu-xen]) AX_SET_FLAGS ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build 2012-03-14 16:46 ` Olaf Hering @ 2012-03-14 18:07 ` Olaf Hering 0 siblings, 0 replies; 9+ messages in thread From: Olaf Hering @ 2012-03-14 18:07 UTC (permalink / raw) To: Ian Jackson; +Cc: Roger Pau Monne, xen-devel@lists.xensource.com On Wed, Mar 14, Olaf Hering wrote: > I noticed that @F is empty now, leading to ..d files in all directories. > No idea how to fix that. > tools/Rules.mk:CFLAGS += -MMD -MF .$(@F).d > diff -r 773d03670872 config/Tools.mk.in > --- a/config/Tools.mk.in > +++ b/config/Tools.mk.in > @@ -23,6 +23,10 @@ PREPEND_LIB := @PREPEND_LIB@ > APPEND_INCLUDES := @APPEND_INCLUDES@ > APPEND_LIB := @APPEND_LIB@ > > +CFLAGS := @EXTRA_CFLAGS_XEN_TOOLS@ > +EXTRA_CFLAGS_QEMU_TRADITIONAL := @EXTRA_CFLAGS_QEMU_TRADITIONAL@ > +EXTRA_CFLAGS_QEMU_XEN := @EXTRA_CFLAGS_QEMU_XEN@ The empty @F is fixed by using '+=' instead of ':=' above. With this small change my attempt works for me, CFLAGS get passed properly to qemu and tools itself. Olaf ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-03-14 18:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-23 16:45 [PATCH] tools/qemu-xen: remove CFLAGS for qemu build Olaf Hering 2012-03-01 17:42 ` Ian Jackson 2012-03-01 17:53 ` Olaf Hering 2012-03-01 18:20 ` Ian Jackson 2012-03-01 19:08 ` Olaf Hering 2012-03-02 11:51 ` Olaf Hering 2012-03-14 12:04 ` Ian Jackson 2012-03-14 16:46 ` Olaf Hering 2012-03-14 18:07 ` Olaf Hering
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).