[Buildroot] [PATCH 1/2] libaio: introduce a BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS option

[Buildroot] [PATCH 1/2] libaio: introduce a BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS option

[Maxime Ripard]

In order to allow other packages to easily select mplayer without
duplicating its complicated architecture dependencies, this commit
introduces a BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS blind option.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 package/blktrace/Config.in      |  6 +-----
 package/gadgetfs-test/Config.in |  6 +-----
 package/libaio/Config.in        | 11 +++++++----
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/package/blktrace/Config.in b/package/blktrace/Config.in
index 09c95eb39ae7..ed5d78a542c6 100644
--- a/package/blktrace/Config.in
+++ b/package/blktrace/Config.in
@@ -2,11 +2,7 @@ config BR2_PACKAGE_BLKTRACE
 	bool "blktrace"
 	# Uses posix_spawn()
 	depends on BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_MUSL
-	# Because of libaio arch depends
-	depends on \
-		BR2_arm   || BR2_armeb || BR2_i386   || \
-		BR2_m68k  || BR2_mips  || BR2_mipsel || BR2_powerpc || \
-		BR2_sparc || BR2_x86_64
+	depends on BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS
 	select BR2_PACKAGE_LIBAIO
 	help
 	  blktrace is a block layer IO tracing mechanism which provides
diff --git a/package/gadgetfs-test/Config.in b/package/gadgetfs-test/Config.in
index 38358f2a1cd5..9a603066e7af 100644
--- a/package/gadgetfs-test/Config.in
+++ b/package/gadgetfs-test/Config.in
@@ -8,12 +8,8 @@ if BR2_PACKAGE_GADGETFS_TEST
 
 config BR2_PACKAGE_GADGETFS_TEST_USE_AIO
 	bool "use asynchronous i/o"
+	depends on BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS
 	select BR2_PACKAGE_LIBAIO
-	# libaio is available only on few architectures
-	depends on \
-		BR2_arm   || BR2_armeb || BR2_i386   || \
-		BR2_m68k  || BR2_mips  || BR2_mipsel || BR2_powerpc || \
-		BR2_sparc || BR2_x86_64
 	help
 	  Select this to have gadgetfs-test do asynchronous I/O using
 	  the libaio library.
diff --git a/package/libaio/Config.in b/package/libaio/Config.in
index 622fd55a5e83..cc44cb6dbcf6 100644
--- a/package/libaio/Config.in
+++ b/package/libaio/Config.in
@@ -1,8 +1,11 @@
+config BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS
+	bool
+	default y if BR2_arm || BR2_armeb || BR2_i386 || \
+		BR2_m68k || BR2_mips || BR2_mipsel || BR2_powerpc || \
+		BR2_sparc || BR2_x86_64
+
 config BR2_PACKAGE_LIBAIO
 	bool "libaio"
-	depends on \
-		BR2_arm   || BR2_armeb || BR2_i386   || \
-		BR2_m68k  || BR2_mips  || BR2_mipsel || BR2_powerpc || \
-		BR2_sparc || BR2_x86_64
+	depends on BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS
 	help
 	  Library for doing asynchronous I/O
-- 
2.4.3

[Buildroot] [PATCH 2/2] xen: new package

[Maxime Ripard]

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

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))
-- 
2.4.3

[Buildroot] [PATCH 2/2] xen: new package

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] xen: new package

[Maxime Ripard]

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150702/c0adb888/attachment.asc>

[Buildroot] [PATCH 2/2] xen: new package

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] xen: new package

[Maxime Ripard]

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150702/8a62b82b/attachment.asc>

[Buildroot] [PATCH 2/2] xen: new package

[Arnout Vandecappelle]

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

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/2] libaio: introduce a BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS option

[Thomas Petazzoni]

Dear Maxime Ripard,

On Thu, 25 Jun 2015 16:04:55 +0200, Maxime Ripard wrote:
> In order to allow other packages to easily select mplayer without

mplayer ?

I guess your copy/paste was a bit too aggressive :-)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] libaio: introduce a BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS option

[Thomas Petazzoni]

Dear Maxime Ripard,

On Thu, 25 Jun 2015 16:04:55 +0200, Maxime Ripard wrote:
> In order to allow other packages to easily select mplayer without
> duplicating its complicated architecture dependencies, this commit
> introduces a BR2_PACKAGE_LIBAIO_ARCH_SUPPORTS blind option.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  package/blktrace/Config.in      |  6 +-----
>  package/gadgetfs-test/Config.in |  6 +-----
>  package/libaio/Config.in        | 11 +++++++----
>  3 files changed, 9 insertions(+), 14 deletions(-)

Applied after fixing the 'mplayer' typo in the commit log, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com