| Message ID | 1405008961-17187-3-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 2014-07-10 at 17:16 +0100, Stefano Stabellini wrote: > Use them to build qemu-xen. > > Add them to the include path of CFLAGS_libxenguest, CFLAGS_libxenctrl > and CFLAGS_libxenstore. > > Use CFLAGS_libxenctrl and CFLAGS_libxenstore to build the python tools. All of these should be included in the patches which do the moves, otherwise you break bisectability. and I think you can drop -I$(XEN_XENSTORE) -I$(XEN_LIBXC) etc since those are supposed to be xenstore-internal headers, which oughtn't to be used even by intree users of the library. If doing that breaks the build we should look at why. > diff --git a/tools/python/Makefile b/tools/python/Makefile > index eee746d..7c4b281 100644 > --- a/tools/python/Makefile > +++ b/tools/python/Makefile > @@ -5,6 +5,8 @@ include $(XEN_ROOT)/tools/Rules.mk > all: build > > XENPATH = "xen/util/path.py" > +CFLAGS += $(CFLAGS_libxenctrl) > +CFLAGS += $(CFLAGS_libxenstore) You should be modifying tools/python/setup.py to change include_dirs for the various modules instead of this. Ian.
On Thu, 10 Jul 2014, Ian Campbell wrote: > On Thu, 2014-07-10 at 17:16 +0100, Stefano Stabellini wrote: > > Use them to build qemu-xen. > > > > Add them to the include path of CFLAGS_libxenguest, CFLAGS_libxenctrl > > and CFLAGS_libxenstore. > > > > Use CFLAGS_libxenctrl and CFLAGS_libxenstore to build the python tools. > > All of these should be included in the patches which do the moves, > otherwise you break bisectability. I'll send out a single big patch. > and I think you can drop -I$(XEN_XENSTORE) -I$(XEN_LIBXC) etc since > those are supposed to be xenstore-internal headers, which oughtn't to be > used even by intree users of the library. > > If doing that breaks the build we should look at why. It does break the build. If we want to go down that path, we need to move xc_dom.h to libxc/include too. But then should we install it too? After that we still need to fix: tools/misc/xen-mfndump.c includes xc_private.h, xc_core.h and xg_save_restore.h. tools/misc/xen-hptool.c includes xc_private.h and xc_core.h tools/xcutils/readnotes.c includes xg_private.h and xc_dom.h tools/blktap2/drivers/block-log.c includes xc_bitops.h tools/xenpaging/file_ops.c includes xc_private.h tools/xenpaging/xenpaging.c includes xc_bitops.h I could add I$(XEN_XENSTORE) and/or -I$(XEN_LIBXC) to the build line of these specific programs and avoid adding them to CFLAGS_libxenctrl and CFLAGS_libxenstore. > > > diff --git a/tools/python/Makefile b/tools/python/Makefile > > index eee746d..7c4b281 100644 > > --- a/tools/python/Makefile > > +++ b/tools/python/Makefile > > @@ -5,6 +5,8 @@ include $(XEN_ROOT)/tools/Rules.mk > > all: build > > > > XENPATH = "xen/util/path.py" > > +CFLAGS += $(CFLAGS_libxenctrl) > > +CFLAGS += $(CFLAGS_libxenstore) > > You should be modifying tools/python/setup.py to change include_dirs for > the various modules instead of this. That works.
On Fri, 2014-07-11 at 13:46 +0100, Stefano Stabellini wrote: > On Thu, 10 Jul 2014, Ian Campbell wrote: > > On Thu, 2014-07-10 at 17:16 +0100, Stefano Stabellini wrote: > > > Use them to build qemu-xen. > > > > > > Add them to the include path of CFLAGS_libxenguest, CFLAGS_libxenctrl > > > and CFLAGS_libxenstore. > > > > > > Use CFLAGS_libxenctrl and CFLAGS_libxenstore to build the python tools. > > > > All of these should be included in the patches which do the moves, > > otherwise you break bisectability. > > I'll send out a single big patch. > > > > and I think you can drop -I$(XEN_XENSTORE) -I$(XEN_LIBXC) etc since > > those are supposed to be xenstore-internal headers, which oughtn't to be > > used even by intree users of the library. > > > > If doing that breaks the build we should look at why. > > It does break the build. > If we want to go down that path, we need to move xc_dom.h to > libxc/include too. But then should we install it too? This is because libxl and a few other bits use it I suppose? It does strike me as a bit odd that this library isn't installed already, although I suppose the set of people want to write a domain builder directly is pretty small. But if you just wanted to move it and not tackle that aspect at the same that would be OK IMHO. > After that we still need to fix: > [...] All of these really have no business poking at the libxc internals (especially xc_private.h) like that. (Or possibly some of the libxc internals should be made public). I don't think you should be expected to fix this now though. > I could add I$(XEN_XENSTORE) and/or -I$(XEN_LIBXC) to the build line of > these specific programs and avoid adding them to CFLAGS_libxenctrl and > CFLAGS_libxenstore. I think this is the right answer. It's in keeping with what Andrew did to xen-mfndump in 7528dcd02de1. Please include a comment like: # $FOO incorrectly uses libxc internals, which is naughty. next to each one to mark it out (part of the reason for these is someone got it wrong once and it spread via cut and paste I think). Ian.
diff --git a/tools/Makefile b/tools/Makefile index f4aa200..5f32dcc 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -188,9 +188,9 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find --includedir=$(PREFIX)/lib/xen/include \ --source-path=$$source \ --extra-cflags="-I$(XEN_ROOT)/tools/include \ - -I$(XEN_ROOT)/tools/libxc \ - -I$(XEN_ROOT)/tools/xenstore \ - -I$(XEN_ROOT)/tools/xenstore/compat \ + -I$(XEN_ROOT)/tools/libxc/include \ + -I$(XEN_ROOT)/tools/xenstore/include \ + -I$(XEN_ROOT)/tools/xenstore/compat/include \ $(EXTRA_CFLAGS_QEMU_XEN)" \ --extra-ldflags="-L$(XEN_ROOT)/tools/libxc \ -L$(XEN_ROOT)/tools/xenstore \ diff --git a/tools/Rules.mk b/tools/Rules.mk index 13d8fc1..249bbbb 100644 --- a/tools/Rules.mk +++ b/tools/Rules.mk @@ -19,15 +19,15 @@ XEN_LIBVCHAN = $(XEN_ROOT)/tools/libvchan CFLAGS_xeninclude = -I$(XEN_INCLUDE) -CFLAGS_libxenctrl = -I$(XEN_LIBXC) $(CFLAGS_xeninclude) +CFLAGS_libxenctrl = -I$(XEN_LIBXC) -I$(XEN_LIBXC)/include $(CFLAGS_xeninclude) LDLIBS_libxenctrl = $(XEN_LIBXC)/libxenctrl.so SHLIB_libxenctrl = -Wl,-rpath-link=$(XEN_LIBXC) -CFLAGS_libxenguest = -I$(XEN_LIBXC) $(CFLAGS_xeninclude) +CFLAGS_libxenguest = -I$(XEN_LIBXC) -I$(XEN_LIBXC)/include $(CFLAGS_xeninclude) LDLIBS_libxenguest = $(XEN_LIBXC)/libxenguest.so SHLIB_libxenguest = -Wl,-rpath-link=L$(XEN_LIBXC) -CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_xeninclude) +CFLAGS_libxenstore = -I$(XEN_XENSTORE) -I$(XEN_XENSTORE)/include $(CFLAGS_xeninclude) LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.so SHLIB_libxenstore = -Wl,-rpath-link=$(XEN_XENSTORE) diff --git a/tools/python/Makefile b/tools/python/Makefile index eee746d..7c4b281 100644 --- a/tools/python/Makefile +++ b/tools/python/Makefile @@ -5,6 +5,8 @@ include $(XEN_ROOT)/tools/Rules.mk all: build XENPATH = "xen/util/path.py" +CFLAGS += $(CFLAGS_libxenctrl) +CFLAGS += $(CFLAGS_libxenstore) genpath-target = $(call buildmakevars2file,$(XENPATH)) $(eval $(genpath-target))
Use them to build qemu-xen. Add them to the include path of CFLAGS_libxenguest, CFLAGS_libxenctrl and CFLAGS_libxenstore. Use CFLAGS_libxenctrl and CFLAGS_libxenstore to build the python tools. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- tools/Makefile | 6 +++--- tools/Rules.mk | 6 +++--- tools/python/Makefile | 2 ++ 3 files changed, 8 insertions(+), 6 deletions(-)