| Message ID | 1435241096-23969-2-git-send-email-maxime.ripard@free-electrons.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Maxime Ripard, On Thu, 25 Jun 2015 16:04:56 +0200, Maxime Ripard wrote: > This package allows to compile both the Xen hypervisor image, and the Xen > tool stack intended to be used in the priviledged guest (aka dom0). > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> Completely side question: do you know if there is a Qemu configuration that emulates the ARM and/or x86 virtualization features that would allow to run this Xen stuff ? It could be useful for quick testing purposes (but isn't of course mandatory at all to get this patch merged). > diff --git a/package/xen/0001-xencommons-create-log-directory-before-using-it.patch b/package/xen/0001-xencommons-create-log-directory-before-using-it.patch > new file mode 100644 > index 000000000000..365cdc5b5a9b > --- /dev/null > +++ b/package/xen/0001-xencommons-create-log-directory-before-using-it.patch > @@ -0,0 +1,31 @@ > +From a7d1fde9a14ca0cf9c3f7c7950a6f9ea89ff58a6 Mon Sep 17 00:00:00 2001 > +From: Maxime Ripard <maxime.ripard@free-electrons.com> > +Date: Thu, 25 Jun 2015 15:47:42 +0200 > +Subject: [PATCH] xencommons: create log directory before using it > + > +In the case where /var/log is volatile, for example when using a tmpfs, the > +/var/log/xen directory will not be found on the system, and every attempt > +to log something to one of the files in that directory will fail. > + > +Create it in the xencommons init script > + > +Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > +--- > + tools/hotplug/Linux/init.d/xencommons.in | 1 + > + 1 file changed, 1 insertion(+) > + > +diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in > +index a1095c29ce0f..89210a02120a 100644 > +--- a/tools/hotplug/Linux/init.d/xencommons.in > ++++ b/tools/hotplug/Linux/init.d/xencommons.in > +@@ -61,6 +61,7 @@ do_start () { > + > + mkdir -p ${XEN_RUN_DIR} > + mkdir -p ${XEN_LOCK_DIR} > ++ mkdir -p /var/log/xen > + > + if ! `${BINDIR}/xenstore-read -s / >/dev/null 2>&1` > + then > +-- > +2.4.3 > + Has this patch been submitted upstream? > diff --git a/package/xen/Config.in b/package/xen/Config.in > new file mode 100644 > index 000000000000..33eb11c50a8e > --- /dev/null > +++ b/package/xen/Config.in > @@ -0,0 +1,30 @@ > +config BR2_PACKAGE_XEN > + bool "xen" > + depends on BR2_arm || BR2_arm64 || \ > + BR2_i386 || BR2_x86_64 > + depends on BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS > + depends on !BR2_STATIC_LIBS # dtc (libfdt) > + depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2 > + depends on BR2_USE_WCHAR # libglib2, util-linux You need a comment to match these 'depends on' toolchain features. > + select BR2_PACKAGE_DTC > + select BR2_PACKAGE_LIBAIO > + select BR2_PACKAGE_LIBGLIB2 > + select BR2_PACKAGE_NCURSES > + select BR2_PACKAGE_OPENSSL > + select BR2_PACKAGE_PIXMAN > + select BR2_PACKAGE_UTIL_LINUX > + select BR2_PACKAGE_UTIL_LINUX_LIBUUID > + select BR2_PACKAGE_YAJL Are all these dependencies needed when you don't install the Xen tools? > + help > + This builds the Xen hypervisor and toolstack > + > + http://www.xenproject.org/ > + > +if BR2_PACKAGE_XEN > + > +config BR2_PACKAGE_XEN_HYPERVISOR > + bool "Build the Xen hypervisor" > + > +config BR2_PACKAGE_XEN_TOOLS > + bool "Build the Xen tools" > +endif Empty newline missing after "endif". Also, if neither the hypervisor nor the tools are enabled, what gets built? Nothing? If so, maybe you want: select BR2_PACKAGE_XEN_HYPERVISOR if !BR2_PACKAGE_XEN_TOOLS in the top-level option. > diff --git a/package/xen/xen.mk b/package/xen/xen.mk > new file mode 100644 > index 000000000000..fee9fdc12cdf > --- /dev/null > +++ b/package/xen/xen.mk > @@ -0,0 +1,49 @@ > +################################################################################ > +# > +# Xen > +# > +################################################################################ > + > +XEN_VERSION = 4.5.0 > +XEN_SITE = http://bits.xensource.com/oss-xen/release/$(XEN_VERSION) Missing LICENSE + LICENSE_FILES. > +XEN_INSTALL_IMAGES = YES > + > +XEN_DEPENDENCIES += dtc libaio libglib2 ncurses openssl pixman util-linux yajl > + > +XEN_MAKE_ENV = \ > + XEN_TARGET_ARCH=arm32 \ Hard-coded arm32 doesn't seem right. > + CROSS_COMPILE=$(TARGET_CROSS) \ > + CXXFLAGS="$(TARGET_CXXFLAGS) -D_FILE_OFFSET_BITS=64" \ > + CFLAGS="$(TARGET_CFLAGS) -D_FILE_OFFSET_BITS=64" \ Why do you pass -D_FILE_OFFSET_BITS=64 ? It is already in TARGET_CFLAGS / TARGET_CXXFLAGS. > + PKG_CONFIG=$(PKG_CONFIG_HOST_BINARY) > + > +XEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) This seems weird, you're losing the "install" target. > + > +ifeq ($(BR2_PACKAGE_XEN_HYPERVISOR),y) > +XEN_MAKE_OPTS += dist-xen > + > +define XEN_INSTALL_IMAGES_CMDS > + cp $(@D)/xen/xen $(BINARIES_DIR) > +endef > +else > +XEN_CONF_OPTS += --disable-xen > +endif > + > +XEN_CONF_OPTS += --disable-ocamltools We generally put the common options towards the top of the file, before all the conditional stuff. And the += is unneeded then, it can be just a '='. > +ifeq ($(BR2_PACKAGE_XEN_TOOLS),y) > +XEN_MAKE_OPTS += dist-tools > +XEN_INSTALL_TARGET_OPTS += install-tools Ah, so here it comes. So maybe instead you want to do XEN_INSTALL_TARGET = NO by default, and do XEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) install-tools when BR2_PACKAGE_XEN_TOOLS=y. > +define XEN_RENAME_INIT_SCRIPTS > + mv $(TARGET_DIR)/etc/init.d/xencommons $(TARGET_DIR)/etc/init.d/S50xencommons > + mv $(TARGET_DIR)/etc/init.d/xen-watchdog $(TARGET_DIR)/etc/init.d/S50xen-watchdog > + mv $(TARGET_DIR)/etc/init.d/xendomains $(TARGET_DIR)/etc/init.d/S60xendomains Don't know if it's better or not: (cd $(TARGET_DIR)/etc/init.d; \ mv xencommons S50xencommons; \ mv ... ; \ mv ...) > +endef > +else > +XEN_CONF_OPTS += --disable-tools > +endif > + > +XEN_POST_INSTALL_TARGET_HOOKS += XEN_RENAME_INIT_SCRIPTS Should be part of the ifeq BR2_PACKAGE_XEN_TOOLS=y. Also, here it doesn't build, because it cannot find libyajl due to the following failure: configure:8342: /home/thomas/projets/buildroot/output/host/usr/bin/arm-linux-gcc -o conftest -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FI LE_OFFSET_BITS=64 conftest.c -lyajl -lcrypto -laio -lutil >&5 /home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libcrypto.so: warning: gethostbyname is obsolescent, use getnameinfo() instead. /home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libyajl.so: undefined reference to `__isnan' /home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libyajl.so: undefined reference to `__isinf' You need to change: AC_CHECK_LIB([yajl], [yajl_alloc], [], [AC_MSG_ERROR([Could not find yajl])]) to: AC_CHECK_LIB([yajl], [yajl_alloc], [], [AC_MSG_ERROR([Could not find yajl])], [m]) So that it links with the math library. I used the following defconfig to produce the problem: BR2_arm=y BR2_TOOLCHAIN_EXTERNAL=y BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.05.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_0=y BR2_TOOLCHAIN_EXTERNAL_LOCALE=y # BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y BR2_TOOLCHAIN_EXTERNAL_CXX=y BR2_INIT_NONE=y BR2_SYSTEM_BIN_SH_NONE=y # BR2_PACKAGE_BUSYBOX is not set BR2_PACKAGE_NGINX=y BR2_PACKAGE_XEN=y BR2_PACKAGE_XEN_HYPERVISOR=y BR2_PACKAGE_XEN_TOOLS=y # BR2_TARGET_ROOTFS_TAR is not set Thanks, Thomas
Hi, On Mon, Jun 29, 2015 at 12:12:02PM +0200, Thomas Petazzoni wrote: > > diff --git a/package/xen/0001-xencommons-create-log-directory-before-using-it.patch b/package/xen/0001-xencommons-create-log-directory-before-using-it.patch > > new file mode 100644 > > index 000000000000..365cdc5b5a9b > > --- /dev/null > > +++ b/package/xen/0001-xencommons-create-log-directory-before-using-it.patch > > @@ -0,0 +1,31 @@ > > +From a7d1fde9a14ca0cf9c3f7c7950a6f9ea89ff58a6 Mon Sep 17 00:00:00 2001 > > +From: Maxime Ripard <maxime.ripard@free-electrons.com> > > +Date: Thu, 25 Jun 2015 15:47:42 +0200 > > +Subject: [PATCH] xencommons: create log directory before using it > > + > > +In the case where /var/log is volatile, for example when using a tmpfs, the > > +/var/log/xen directory will not be found on the system, and every attempt > > +to log something to one of the files in that directory will fail. > > + > > +Create it in the xencommons init script > > + > > +Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > +--- > > + tools/hotplug/Linux/init.d/xencommons.in | 1 + > > + 1 file changed, 1 insertion(+) > > + > > +diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in > > +index a1095c29ce0f..89210a02120a 100644 > > +--- a/tools/hotplug/Linux/init.d/xencommons.in > > ++++ b/tools/hotplug/Linux/init.d/xencommons.in > > +@@ -61,6 +61,7 @@ do_start () { > > + > > + mkdir -p ${XEN_RUN_DIR} > > + mkdir -p ${XEN_LOCK_DIR} > > ++ mkdir -p /var/log/xen > > + > > + if ! `${BINDIR}/xenstore-read -s / >/dev/null 2>&1` > > + then > > +-- > > +2.4.3 > > + > > Has this patch been submitted upstream? Not yet. > > > diff --git a/package/xen/Config.in b/package/xen/Config.in > > new file mode 100644 > > index 000000000000..33eb11c50a8e > > --- /dev/null > > +++ b/package/xen/Config.in > > @@ -0,0 +1,30 @@ > > +config BR2_PACKAGE_XEN > > + bool "xen" > > + depends on BR2_arm || BR2_arm64 || \ > > + BR2_i386 || BR2_x86_64 > > + depends on BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS > > + depends on !BR2_STATIC_LIBS # dtc (libfdt) > > + depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2 > > + depends on BR2_USE_WCHAR # libglib2, util-linux > > You need a comment to match these 'depends on' toolchain features. Ack. > > + select BR2_PACKAGE_DTC > > + select BR2_PACKAGE_LIBAIO > > + select BR2_PACKAGE_LIBGLIB2 > > + select BR2_PACKAGE_NCURSES > > + select BR2_PACKAGE_OPENSSL > > + select BR2_PACKAGE_PIXMAN > > + select BR2_PACKAGE_UTIL_LINUX > > + select BR2_PACKAGE_UTIL_LINUX_LIBUUID > > + select BR2_PACKAGE_YAJL > > Are all these dependencies needed when you don't install the Xen tools? Probably not, the hypervisor is completely standalone, so I don't think it would require any of these. BTW, I just found out that I left something out of this patch. The initscripts are using some bashism that don't work with busybox' ash. What's the way Buildroot deal with this usually? Add a dependency on bash? > > + help > > + This builds the Xen hypervisor and toolstack > > + > > + http://www.xenproject.org/ > > + > > +if BR2_PACKAGE_XEN > > + > > +config BR2_PACKAGE_XEN_HYPERVISOR > > + bool "Build the Xen hypervisor" > > + > > +config BR2_PACKAGE_XEN_TOOLS > > + bool "Build the Xen tools" > > +endif > > Empty newline missing after "endif". > > Also, if neither the hypervisor nor the tools are enabled, what gets > built? Nothing? If so, maybe you want: > > select BR2_PACKAGE_XEN_HYPERVISOR if !BR2_PACKAGE_XEN_TOOLS > > in the top-level option. I think it would make more sense to have the tools built by default, which would reverse the select statement I guess? > > > diff --git a/package/xen/xen.mk b/package/xen/xen.mk > > new file mode 100644 > > index 000000000000..fee9fdc12cdf > > --- /dev/null > > +++ b/package/xen/xen.mk > > @@ -0,0 +1,49 @@ > > +################################################################################ > > +# > > +# Xen > > +# > > +################################################################################ > > + > > +XEN_VERSION = 4.5.0 > > +XEN_SITE = http://bits.xensource.com/oss-xen/release/$(XEN_VERSION) > > Missing LICENSE + LICENSE_FILES. Ah, right. It uses the GPL as its license, with some 2c and 3c BSD exceptions. How is that usually dealt with ? Using LICENSE = GPLv2 with exceptions Or making an exhaustive list of the licenses used? > > > +XEN_INSTALL_IMAGES = YES > > + > > +XEN_DEPENDENCIES += dtc libaio libglib2 ncurses openssl pixman util-linux yajl > > + > > +XEN_MAKE_ENV = \ > > + XEN_TARGET_ARCH=arm32 \ > > Hard-coded arm32 doesn't seem right. No it's not. > > + CROSS_COMPILE=$(TARGET_CROSS) \ > > + CXXFLAGS="$(TARGET_CXXFLAGS) -D_FILE_OFFSET_BITS=64" \ > > + CFLAGS="$(TARGET_CFLAGS) -D_FILE_OFFSET_BITS=64" \ > > Why do you pass -D_FILE_OFFSET_BITS=64 ? It is already in > TARGET_CFLAGS / TARGET_CXXFLAGS. It was required during the make invocation, while Buildroot only passes it during the configure. > > + PKG_CONFIG=$(PKG_CONFIG_HOST_BINARY) > > + > > +XEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) > > This seems weird, you're losing the "install" target. It's actually on purpose. Xen has a specific install target for the tools (install-target), that is added a bit later when compiling the tools. > > + > > +ifeq ($(BR2_PACKAGE_XEN_HYPERVISOR),y) > > +XEN_MAKE_OPTS += dist-xen > > + > > +define XEN_INSTALL_IMAGES_CMDS > > + cp $(@D)/xen/xen $(BINARIES_DIR) > > +endef > > +else > > +XEN_CONF_OPTS += --disable-xen > > +endif > > + > > +XEN_CONF_OPTS += --disable-ocamltools > > We generally put the common options towards the top of the file, before > all the conditional stuff. And the += is unneeded then, it can be just > a '='. Ok. > > > +ifeq ($(BR2_PACKAGE_XEN_TOOLS),y) > > +XEN_MAKE_OPTS += dist-tools > > +XEN_INSTALL_TARGET_OPTS += install-tools > > Ah, so here it comes. So maybe instead you want to do > XEN_INSTALL_TARGET = NO by default, and do XEN_INSTALL_TARGET_OPTS = > DESTDIR=$(TARGET_DIR) install-tools when BR2_PACKAGE_XEN_TOOLS=y. Now I see that I forgot that as well, but the hypervisor can be installed in /boot using install-xen. Maybe I could just add a new option to ask whether we want to install it to /boot, and add install-xen to INSTALL_TARGET_OPTS if relevant. > > +define XEN_RENAME_INIT_SCRIPTS > > + mv $(TARGET_DIR)/etc/init.d/xencommons $(TARGET_DIR)/etc/init.d/S50xencommons > > + mv $(TARGET_DIR)/etc/init.d/xen-watchdog $(TARGET_DIR)/etc/init.d/S50xen-watchdog > > + mv $(TARGET_DIR)/etc/init.d/xendomains $(TARGET_DIR)/etc/init.d/S60xendomains > > Don't know if it's better or not: > > (cd $(TARGET_DIR)/etc/init.d; \ > mv xencommons S50xencommons; \ > mv ... ; \ > mv ...) I don't really have a preference. I'll change this if you want. > > +endef > > +else > > +XEN_CONF_OPTS += --disable-tools > > +endif > > + > > +XEN_POST_INSTALL_TARGET_HOOKS += XEN_RENAME_INIT_SCRIPTS > > Should be part of the ifeq BR2_PACKAGE_XEN_TOOLS=y. Is it? My understanding was that the macro wouldn't be defined and it would call an empty one in such a case? > Also, here it doesn't build, because it cannot find libyajl due to the > following failure: > > configure:8342: /home/thomas/projets/buildroot/output/host/usr/bin/arm-linux-gcc -o conftest -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FI > LE_OFFSET_BITS=64 conftest.c -lyajl -lcrypto -laio -lutil >&5 > /home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libcrypto.so: warning: gethostbyname is obsolescent, use getnameinfo() instead. > /home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libyajl.so: undefined reference to `__isnan' > /home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libyajl.so: undefined reference to `__isinf' > > You need to change: > > AC_CHECK_LIB([yajl], [yajl_alloc], [], > [AC_MSG_ERROR([Could not find yajl])]) > > to: > > AC_CHECK_LIB([yajl], [yajl_alloc], [], > [AC_MSG_ERROR([Could not find yajl])], [m]) Ack. Thanks! Maxime
On Mon, Jun 29, 2015 at 11:22:01AM +0100, Ian Campbell wrote: > > > +++ b/package/xen/0001-xencommons-create-log-directory-before-using-it.patch > > > @@ -0,0 +1,31 @@ > > > +From a7d1fde9a14ca0cf9c3f7c7950a6f9ea89ff58a6 Mon Sep 17 00:00:00 2001 > > > +From: Maxime Ripard <maxime.ripard@free-electrons.com> > > > +Date: Thu, 25 Jun 2015 15:47:42 +0200 > > > +Subject: [PATCH] xencommons: create log directory before using it > > > + > > > +In the case where /var/log is volatile, for example when using a tmpfs, the > > > +/var/log/xen directory will not be found on the system, and every attempt > > > +to log something to one of the files in that directory will fail. > > > + > > > +Create it in the xencommons init script > > > + > > > +Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > +--- > > > + tools/hotplug/Linux/init.d/xencommons.in | 1 + > > > + 1 file changed, 1 insertion(+) > > > + > > > +diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in > > > +index a1095c29ce0f..89210a02120a 100644 > > > +--- a/tools/hotplug/Linux/init.d/xencommons.in > > > ++++ b/tools/hotplug/Linux/init.d/xencommons.in > > > +@@ -61,6 +61,7 @@ do_start () { > > > + > > > + mkdir -p ${XEN_RUN_DIR} > > > + mkdir -p ${XEN_LOCK_DIR} > > > ++ mkdir -p /var/log/xen > > > + > > > + if ! `${BINDIR}/xenstore-read -s / >/dev/null 2>&1` > > > + then > > > +-- > > > +2.4.3 > > > + > > > > Has this patch been submitted upstream? > > It would be great if it could be, ideally it should use ${XEN_LOG_DIR}, > which would involve adding it to Config.mk:BUILD_MAKE_VARS such that it > will appear in hotplugpath.sh (which is included by xencommons.in at > runtime). > > You might also want to create ${XEN_LOG_DIR}/console to support folks > enabling guest console logging too. Ok, I'll change that and submit the patches. Thanks! Maxime
Hello, On Thu, 2 Jul 2015 11:43:08 +0200, Maxime Ripard wrote: > > > + select BR2_PACKAGE_DTC > > > + select BR2_PACKAGE_LIBAIO > > > + select BR2_PACKAGE_LIBGLIB2 > > > + select BR2_PACKAGE_NCURSES > > > + select BR2_PACKAGE_OPENSSL > > > + select BR2_PACKAGE_PIXMAN > > > + select BR2_PACKAGE_UTIL_LINUX > > > + select BR2_PACKAGE_UTIL_LINUX_LIBUUID > > > + select BR2_PACKAGE_YAJL > > > > Are all these dependencies needed when you don't install the Xen tools? > > Probably not, the hypervisor is completely standalone, so I don't > think it would require any of these. Ok, so all those select can be moved to the "tools" option, then. Also, and maybe this is something Ian could clarify: do we really need pixman or ncurses when you just want to start text-only Xen VMs ? > BTW, I just found out that I left something out of this patch. The > initscripts are using some bashism that don't work with busybox' > ash. What's the way Buildroot deal with this usually? Add a dependency > on bash? We generally prefer to have the scripts fixed so that they don't use bashisms. If that's not doable, then selecting bash is a possibility (but don't add it to XEN_DEPENDENCIES, since it's a runtime dependency only). Of course, make sure the scripts use bash explicitly in their shebang, because you're not guaranteed that bash will be the default shell. > > Also, if neither the hypervisor nor the tools are enabled, what gets > > built? Nothing? If so, maybe you want: > > > > select BR2_PACKAGE_XEN_HYPERVISOR if !BR2_PACKAGE_XEN_TOOLS > > > > in the top-level option. > > I think it would make more sense to have the tools built by default, > which would reverse the select statement I guess? Correct. > > Missing LICENSE + LICENSE_FILES. > > Ah, right. > > It uses the GPL as its license, with some 2c and 3c BSD > exceptions. How is that usually dealt with ? > > Using > LICENSE = GPLv2 with exceptions > > Or making an exhaustive list of the licenses used? GPLv2 with expections means "the license is the text of the GPLv2 to which some additional clauses/wording has been added". Which is not really what you have IIUC. Maybe: LICENSE = GPLv2, BSD-2c, BSD-3c or better (if doable): LICENSE = GPLv2 (this), BSD-2c (that), BSD-3c (this other thing) > > > + CROSS_COMPILE=$(TARGET_CROSS) \ > > > + CXXFLAGS="$(TARGET_CXXFLAGS) -D_FILE_OFFSET_BITS=64" \ > > > + CFLAGS="$(TARGET_CFLAGS) -D_FILE_OFFSET_BITS=64" \ > > > > Why do you pass -D_FILE_OFFSET_BITS=64 ? It is already in > > TARGET_CFLAGS / TARGET_CXXFLAGS. > > It was required during the make invocation, while Buildroot only > passes it during the configure. Alright. Xen's build system could use some improvement, but I guess that outside of the scope of Buildroot packaging :-) > > > + PKG_CONFIG=$(PKG_CONFIG_HOST_BINARY) > > > + > > > +XEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) > > > > This seems weird, you're losing the "install" target. > > It's actually on purpose. Xen has a specific install target for the > tools (install-target), that is added a bit later when compiling the > tools. Then why not do it just in one go, since you're using install-tools only? Like: XEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) install-tools Or maybe it's because you don't want "make install" to be called when only the hypervisor is enabled. In this case, just do XEN_INSTALL_TARGET = NO. ifeq ($(...hypervisor...),y) XEN_INSTALL_IMAGES = YES XEN_INSTALL_TARGET = NO ... else ifeq ($(...tools...),y) XEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) install-tools ... endif > > Ah, so here it comes. So maybe instead you want to do > > XEN_INSTALL_TARGET = NO by default, and do XEN_INSTALL_TARGET_OPTS = > > DESTDIR=$(TARGET_DIR) install-tools when BR2_PACKAGE_XEN_TOOLS=y. > > Now I see that I forgot that as well, but the hypervisor can be > installed in /boot using install-xen. > > Maybe I could just add a new option to ask whether we want to install > it to /boot, and add install-xen to INSTALL_TARGET_OPTS if relevant. Right. So you would have to change the above proposal, though :) > > > +define XEN_RENAME_INIT_SCRIPTS > > > + mv $(TARGET_DIR)/etc/init.d/xencommons $(TARGET_DIR)/etc/init.d/S50xencommons > > > + mv $(TARGET_DIR)/etc/init.d/xen-watchdog $(TARGET_DIR)/etc/init.d/S50xen-watchdog > > > + mv $(TARGET_DIR)/etc/init.d/xendomains $(TARGET_DIR)/etc/init.d/S60xendomains > > > > Don't know if it's better or not: > > > > (cd $(TARGET_DIR)/etc/init.d; \ > > mv xencommons S50xencommons; \ > > mv ... ; \ > > mv ...) > > I don't really have a preference. I'll change this if you want. Maybe your proposal is simpler. You can keep it as is. > > > > +endef > > > +else > > > +XEN_CONF_OPTS += --disable-tools > > > +endif > > > + > > > +XEN_POST_INSTALL_TARGET_HOOKS += XEN_RENAME_INIT_SCRIPTS > > > > Should be part of the ifeq BR2_PACKAGE_XEN_TOOLS=y. > > Is it? My understanding was that the macro wouldn't be defined and it > would call an empty one in such a case? Correct: it works fine. But we generally prefer to register the hook only when needed. > > You need to change: > > > > AC_CHECK_LIB([yajl], [yajl_alloc], [], > > [AC_MSG_ERROR([Could not find yajl])]) > > > > to: > > > > AC_CHECK_LIB([yajl], [yajl_alloc], [], > > [AC_MSG_ERROR([Could not find yajl])], [m]) > > Ack. Note that I haven't tested this change, it was purely based on a quick reading of Xen's configure.ac. Thomas
On 25-06-15 15:04, Maxime Ripard wrote: > This package allows to compile both the Xen hypervisor image, and the Xen > tool stack intended to be used in the priviledged guest (aka dom0). > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> Hi Maxime, There were some comments that still need to be addressed so I've marked this patch as Changes Requested in patchwork. Please resubmit when you're ready. Regards, Arnout
diff --git a/package/Config.in b/package/Config.in index e0c2e2ac422b..6e0124b2eceb 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1421,6 +1421,7 @@ if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS source "package/sysvinit/Config.in" endif source "package/util-linux/Config.in" + source "package/xen/Config.in" endmenu menu "Text editors and viewers" diff --git a/package/xen/0001-xencommons-create-log-directory-before-using-it.patch b/package/xen/0001-xencommons-create-log-directory-before-using-it.patch new file mode 100644 index 000000000000..365cdc5b5a9b --- /dev/null +++ b/package/xen/0001-xencommons-create-log-directory-before-using-it.patch @@ -0,0 +1,31 @@ +From a7d1fde9a14ca0cf9c3f7c7950a6f9ea89ff58a6 Mon Sep 17 00:00:00 2001 +From: Maxime Ripard <maxime.ripard@free-electrons.com> +Date: Thu, 25 Jun 2015 15:47:42 +0200 +Subject: [PATCH] xencommons: create log directory before using it + +In the case where /var/log is volatile, for example when using a tmpfs, the +/var/log/xen directory will not be found on the system, and every attempt +to log something to one of the files in that directory will fail. + +Create it in the xencommons init script + +Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> +--- + tools/hotplug/Linux/init.d/xencommons.in | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in +index a1095c29ce0f..89210a02120a 100644 +--- a/tools/hotplug/Linux/init.d/xencommons.in ++++ b/tools/hotplug/Linux/init.d/xencommons.in +@@ -61,6 +61,7 @@ do_start () { + + mkdir -p ${XEN_RUN_DIR} + mkdir -p ${XEN_LOCK_DIR} ++ mkdir -p /var/log/xen + + if ! `${BINDIR}/xenstore-read -s / >/dev/null 2>&1` + then +-- +2.4.3 + diff --git a/package/xen/Config.in b/package/xen/Config.in new file mode 100644 index 000000000000..33eb11c50a8e --- /dev/null +++ b/package/xen/Config.in @@ -0,0 +1,30 @@ +config BR2_PACKAGE_XEN + bool "xen" + depends on BR2_arm || BR2_arm64 || \ + BR2_i386 || BR2_x86_64 + depends on BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS + depends on !BR2_STATIC_LIBS # dtc (libfdt) + depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2 + depends on BR2_USE_WCHAR # libglib2, util-linux + select BR2_PACKAGE_DTC + select BR2_PACKAGE_LIBAIO + select BR2_PACKAGE_LIBGLIB2 + select BR2_PACKAGE_NCURSES + select BR2_PACKAGE_OPENSSL + select BR2_PACKAGE_PIXMAN + select BR2_PACKAGE_UTIL_LINUX + select BR2_PACKAGE_UTIL_LINUX_LIBUUID + select BR2_PACKAGE_YAJL + help + This builds the Xen hypervisor and toolstack + + http://www.xenproject.org/ + +if BR2_PACKAGE_XEN + +config BR2_PACKAGE_XEN_HYPERVISOR + bool "Build the Xen hypervisor" + +config BR2_PACKAGE_XEN_TOOLS + bool "Build the Xen tools" +endif diff --git a/package/xen/xen.hash b/package/xen/xen.hash new file mode 100644 index 000000000000..7e9bcc123261 --- /dev/null +++ b/package/xen/xen.hash @@ -0,0 +1,2 @@ +# Locally computed +sha256 5bdb40e2b28d2eeb541bd71a9777f40cbe2ae444b987521d33f099541a006f3b xen-4.5.0.tar.gz diff --git a/package/xen/xen.mk b/package/xen/xen.mk new file mode 100644 index 000000000000..fee9fdc12cdf --- /dev/null +++ b/package/xen/xen.mk @@ -0,0 +1,49 @@ +################################################################################ +# +# Xen +# +################################################################################ + +XEN_VERSION = 4.5.0 +XEN_SITE = http://bits.xensource.com/oss-xen/release/$(XEN_VERSION) +XEN_INSTALL_IMAGES = YES + +XEN_DEPENDENCIES += dtc libaio libglib2 ncurses openssl pixman util-linux yajl + +XEN_MAKE_ENV = \ + XEN_TARGET_ARCH=arm32 \ + CROSS_COMPILE=$(TARGET_CROSS) \ + CXXFLAGS="$(TARGET_CXXFLAGS) -D_FILE_OFFSET_BITS=64" \ + CFLAGS="$(TARGET_CFLAGS) -D_FILE_OFFSET_BITS=64" \ + PKG_CONFIG=$(PKG_CONFIG_HOST_BINARY) + +XEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) + +ifeq ($(BR2_PACKAGE_XEN_HYPERVISOR),y) +XEN_MAKE_OPTS += dist-xen + +define XEN_INSTALL_IMAGES_CMDS + cp $(@D)/xen/xen $(BINARIES_DIR) +endef +else +XEN_CONF_OPTS += --disable-xen +endif + +XEN_CONF_OPTS += --disable-ocamltools + +ifeq ($(BR2_PACKAGE_XEN_TOOLS),y) +XEN_MAKE_OPTS += dist-tools +XEN_INSTALL_TARGET_OPTS += install-tools + +define XEN_RENAME_INIT_SCRIPTS + mv $(TARGET_DIR)/etc/init.d/xencommons $(TARGET_DIR)/etc/init.d/S50xencommons + mv $(TARGET_DIR)/etc/init.d/xen-watchdog $(TARGET_DIR)/etc/init.d/S50xen-watchdog + mv $(TARGET_DIR)/etc/init.d/xendomains $(TARGET_DIR)/etc/init.d/S60xendomains +endef +else +XEN_CONF_OPTS += --disable-tools +endif + +XEN_POST_INSTALL_TARGET_HOOKS += XEN_RENAME_INIT_SCRIPTS + +$(eval $(autotools-package))
This package allows to compile both the Xen hypervisor image, and the Xen tool stack intended to be used in the priviledged guest (aka dom0). Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- package/Config.in | 1 + ...mons-create-log-directory-before-using-it.patch | 31 ++++++++++++++ package/xen/Config.in | 30 +++++++++++++ package/xen/xen.hash | 2 + package/xen/xen.mk | 49 ++++++++++++++++++++++ 5 files changed, 113 insertions(+) create mode 100644 package/xen/0001-xencommons-create-log-directory-before-using-it.patch create mode 100644 package/xen/Config.in create mode 100644 package/xen/xen.hash create mode 100644 package/xen/xen.mk