[bug#29409] GuixSD ARM port.

[bug#29409] GuixSD ARM port.

[Mathieu Othacehe]

Hi,

I'm currently working on porting GuixSD to ARM using a BBB as a first
target. As I'm progressing slowly patches are coming one by one. I'll
use this debbugs id to gather them.

Thanks,

Mathieu

[bug#29409] [PATCH] linux-libre: Adapt some arm options to match intel conf.

[Mathieu Othacehe]

From: Mathieu Othacehe <mathieu.othacehe@parrot.com>

* gnu/packages/aux-files/linux-libre/4.14-arm.conf: GuixSD base-initrd expects
  ext4 and fat filesystem to be built-in. Adapt configuration
  accordingly. Also set devtmpfs_mount to yes.
---
 gnu/packages/aux-files/linux-libre/4.14-arm.conf | 25 ++++++++++++------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/gnu/packages/aux-files/linux-libre/4.14-arm.conf b/gnu/packages/aux-files/linux-libre/4.14-arm.conf
index 7f82c29..2bde470 100644
--- a/gnu/packages/aux-files/linux-libre/4.14-arm.conf
+++ b/gnu/packages/aux-files/linux-libre/4.14-arm.conf
@@ -1816,7 +1816,7 @@ CONFIG_TEGRA_AHB=y
 #
 # CONFIG_UEVENT_HELPER is not set
 CONFIG_DEVTMPFS=y
-# CONFIG_DEVTMPFS_MOUNT is not set
+CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_STANDALONE=y
 CONFIG_PREVENT_FIRMWARE_BUILD=y
 CONFIG_FW_LOADER=y
@@ -7466,16 +7466,15 @@ CONFIG_DCACHE_WORD_ACCESS=y
 CONFIG_FS_IOMAP=y
 # CONFIG_EXT2_FS is not set
 # CONFIG_EXT3_FS is not set
-CONFIG_EXT4_FS=m
+CONFIG_EXT4_FS=y
 CONFIG_EXT4_USE_FOR_EXT2=y
 CONFIG_EXT4_FS_POSIX_ACL=y
 CONFIG_EXT4_FS_SECURITY=y
-CONFIG_EXT4_ENCRYPTION=y
-CONFIG_EXT4_FS_ENCRYPTION=y
+# CONFIG_EXT4_ENCRYPTION is not set
 # CONFIG_EXT4_DEBUG is not set
-CONFIG_JBD2=m
+CONFIG_JBD2=y
 # CONFIG_JBD2_DEBUG is not set
-CONFIG_FS_MBCACHE=m
+CONFIG_FS_MBCACHE=y
 CONFIG_REISERFS_FS=m
 # CONFIG_REISERFS_CHECK is not set
 # CONFIG_REISERFS_PROC_INFO is not set
@@ -7567,12 +7566,12 @@ CONFIG_UDF_NLS=y
 #
 # DOS/FAT/NT Filesystems
 #
-CONFIG_FAT_FS=m
+CONFIG_FAT_FS=y
 CONFIG_MSDOS_FS=m
-CONFIG_VFAT_FS=m
+CONFIG_VFAT_FS=y
 CONFIG_FAT_DEFAULT_CODEPAGE=437
-CONFIG_FAT_DEFAULT_IOCHARSET="ascii"
-CONFIG_FAT_DEFAULT_UTF8=y
+CONFIG_FAT_DEFAULT_IOCHARSET="iso8859-1"
+# CONFIG_FAT_DEFAULT_UTF8 is not set
 CONFIG_NTFS_FS=m
 # CONFIG_NTFS_DEBUG is not set
 # CONFIG_NTFS_RW is not set
@@ -7744,7 +7743,7 @@ CONFIG_9P_FS_POSIX_ACL=y
 CONFIG_9P_FS_SECURITY=y
 CONFIG_NLS=y
 CONFIG_NLS_DEFAULT="utf8"
-CONFIG_NLS_CODEPAGE_437=m
+CONFIG_NLS_CODEPAGE_437=y
 CONFIG_NLS_CODEPAGE_737=m
 CONFIG_NLS_CODEPAGE_775=m
 CONFIG_NLS_CODEPAGE_850=m
@@ -8162,7 +8161,7 @@ CONFIG_CRYPTO_VMAC=m
 #
 # Digest
 #
-CONFIG_CRYPTO_CRC32C=m
+CONFIG_CRYPTO_CRC32C=y
 CONFIG_CRYPTO_CRC32=m
 CONFIG_CRYPTO_CRCT10DIF=y
 CONFIG_CRYPTO_GHASH=m
@@ -8274,7 +8273,7 @@ CONFIG_GENERIC_IO=y
 CONFIG_STMP_DEVICE=y
 CONFIG_ARCH_USE_CMPXCHG_LOCKREF=y
 CONFIG_CRC_CCITT=m
-CONFIG_CRC16=m
+CONFIG_CRC16=y
 CONFIG_CRC_T10DIF=y
 CONFIG_CRC_ITU_T=m
 CONFIG_CRC32=y
-- 
2.7.4

[bug#29409] [PATCH] linux-libre: Adapt some arm options to match intel conf.

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

> From: Mathieu Othacehe <mathieu.othacehe@parrot.com>
>
> * gnu/packages/aux-files/linux-libre/4.14-arm.conf: GuixSD base-initrd expects
>   ext4 and fat filesystem to be built-in. Adapt configuration
>   accordingly. Also set devtmpfs_mount to yes.

LGTM, thanks!

Ludo'.

bug#29409: [PATCH] linux-libre: Adapt some arm options to match intel conf.

[Mathieu Othacehe]

> LGTM, thanks!

Pushed !

Thanks,

Mathieu

[bug#29409] [PATCH] linux-libre: Adapt some arm options to match intel conf.

[Danny Milosavljevic]

Any chance that this patch is not necessary if the correct initrd is used? Because it should have been able to load the modules from the initrd just fine.

[bug#29409] [PATCH] linux-libre: Adapt some arm options to match intel conf.

[Mathieu Othacehe]

> Any chance that this patch is not necessary if the correct initrd is used? Because it should have been able to load the modules from the initrd just fine.

Well the initrd used on intel platforms only loads a specific set of
modules ("ahci" "usb-storage" ...), see linux-modules in
linux-initrd.scm.

I'd like to reuse the same code on arm platform, so I think the better
option is for kernel configurations to converge instead of defining
specific initrd for specific platforms.

I plan on adding a gnu/system/installers/ or a gnu/system/boards/
directory with one file per specific platform. Then each board will have
a specific bootloader: grub, u-boot-... but, if possible, the same
kernel : linux-libre and the same initrd : base-initrd (with some
#:extra-modules if needed).

WDYT ?

Mathieu

[bug#29409] [PATCH] linux-libre: Adapt some arm options to match intel conf.

[Danny Milosavljevic]

Hi Mathieu,

> I'd like to reuse the same code on arm platform, so I think the better
> option is for kernel configurations to converge instead of defining
> specific initrd for specific platforms.

Yeah, good point.  I agree.

> I plan on adding a gnu/system/installers/ or a gnu/system/boards/
> directory with one file per specific platform. Then each board will have
> a specific bootloader: grub, u-boot-... but, if possible, the same
> kernel : linux-libre and the same initrd : base-initrd (with some
> #:extra-modules if needed).

Makes sense.

[bug#29409] [PATCH] vm: Use os-defined initrd intead of base-initrd.

[Mathieu Othacehe]

* gnu/system/vm.scm (system-disk-image, system-qemu-image,
  virtualized-operating-system): Replace base-initrd by
  (operating-system-initrd os).

The system produced were always using base-initrd even if the user had
defined a custom initrd based on raw-initrd in the os declaration.
---
Hi,

I was able to test this fix with the following command :

make check-system TESTS="installed-os"

and also with an os with a custom raw-initrd. The produced system uses
the specified raw-initrd instead of always using base-initrd.

Thanks,

Mathieu
 gnu/system/vm.scm | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 4424608..3ddb41d 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -423,7 +423,8 @@ to USB sticks meant to be read-only."
               ;; install QEMU networking or anything like that.  Assume USB
               ;; mass storage devices (usb-storage.ko) are available.
               (initrd (lambda (file-systems . rest)
-                        (apply base-initrd file-systems
+                        (apply (operating-system-initrd os)
+                               file-systems
                                #:volatile-root? #t
                                rest)))
 
@@ -488,7 +489,8 @@ of the GNU system as described by OS."
   (let ((os (operating-system (inherit os)
               ;; Use an initrd with the whole QEMU shebang.
               (initrd (lambda (file-systems . rest)
-                        (apply base-initrd file-systems
+                        (apply (operating-system-initrd os)
+                               file-systems
                                #:virtio? #t
                                rest)))
 
@@ -574,7 +576,8 @@ environment with the store shared with the host.  MAPPINGS is a list of
                   (target "/dev/vda")))
 
     (initrd (lambda (file-systems . rest)
-              (apply base-initrd file-systems
+              (apply (operating-system-initrd os)
+                     file-systems
                      #:volatile-root? #t
                      #:virtio? #t
                      rest)))
-- 
2.7.4

[bug#29409] [PATCH] vm: Use os-defined initrd intead of base-initrd.

[Ludovic Courtès]

Hello!

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

> * gnu/system/vm.scm (system-disk-image, system-qemu-image,
>   virtualized-operating-system): Replace base-initrd by
>   (operating-system-initrd os).
>
> The system produced were always using base-initrd even if the user had
> defined a custom initrd based on raw-initrd in the os declaration.

[...]

>      (initrd (lambda (file-systems . rest)
> -              (apply base-initrd file-systems
> +              (apply (operating-system-initrd os)
> +                     file-systems
>                       #:volatile-root? #t
>                       #:virtio? #t
>                       rest)))

A potential issue is that we don’t know whether the user-provided initrd
procedure honors #:volatile-root? et al.  Maybe that’s not much of a
problem in practice: we can assume that it’s up to the user to do the
right thing.

I’d say go for it and we’ll adjust if/when it’s a problem.

Thoughts?

Thanks,
Ludo’.

[bug#29409] [PATCH] vm: Use os-defined initrd intead of base-initrd.

[Mathieu Othacehe]

Hey Ludo !

> A potential issue is that we don’t know whether the user-provided initrd
> procedure honors #:volatile-root? et al.  Maybe that’s not much of a
> problem in practice: we can assume that it’s up to the user to do the
> right thing.
>
> I’d say go for it and we’ll adjust if/when it’s a problem.
>
> Thoughts?

Well i agree, it's true that this raw-initrd thing is a bit fragile but
that's the only way for users to define handcrafted initrds.

So i'll push but maybe we need to find something better than
raw-initrd to cover this need.

Thanks,

Mathieu

[bug#29409] [PATCH] gnu: dtc: Fix build on 32 bits platforms.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/packages/bootloaders.scm (dtc)[patches]: Add dtc-32-bits-check.patch and
  dtc-format-modifier.patch to fix build and tests on 32 bits platforms.
* gnu/packages/patches/dtc-32-bits-check.patch : New file.
* gnu/packages/patches/dtc-format-modifier.patch : New file.
* gnu/local.mk (dist_patch_DATA): Add two above patches.
---
 gnu/local.mk                                   |   2 +
 gnu/packages/bootloaders.scm                   |   6 +-
 gnu/packages/patches/dtc-32-bits-check.patch   | 134 +++++++++++++++++++++++++
 gnu/packages/patches/dtc-format-modifier.patch |  38 +++++++
 4 files changed, 179 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/dtc-32-bits-check.patch
 create mode 100644 gnu/packages/patches/dtc-format-modifier.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index ebff708..55c961d 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -602,6 +602,8 @@ dist_patch_DATA =						\
   %D%/packages/patches/doc++-include-directives.patch		\
   %D%/packages/patches/doc++-segfault-fix.patch			\
   %D%/packages/patches/doxygen-test.patch			\
+  %D%/packages/patches/dtc-format-modifier.patch		\
+  %D%/packages/patches/dtc-32-bits-check.patch			\
   %D%/packages/patches/dvd+rw-tools-add-include.patch 		\
   %D%/packages/patches/elfutils-tests-ptrace.patch		\
   %D%/packages/patches/elixir-disable-failing-tests.patch	\
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 20f38b2..d28fe32 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -296,7 +296,11 @@ menu to select one of the installed operating systems.")
                     "dtc-" version ".tar.xz"))
               (sha256
                (base32
-                "08gnl39i4xy3dm8iqwlz2ygx0ml1bgc5kpiys5ll1wvah1j72b04"))))
+                "08gnl39i4xy3dm8iqwlz2ygx0ml1bgc5kpiys5ll1wvah1j72b04"))
+              ;; Fix build and tests on 32 bits platforms.
+              ;; Will probably be fixed in 1.4.6 release.
+              (patches (search-patches "dtc-format-modifier.patch"
+                                       "dtc-32-bits-check.patch"))))
     (build-system gnu-build-system)
     (native-inputs
      `(("bison" ,bison)
diff --git a/gnu/packages/patches/dtc-32-bits-check.patch b/gnu/packages/patches/dtc-32-bits-check.patch
new file mode 100644
index 0000000..cf15be3
--- /dev/null
+++ b/gnu/packages/patches/dtc-32-bits-check.patch
@@ -0,0 +1,134 @@
+This fixes tests on 32 bits platforms. Patch taken from upstream.
+
+commit f8872e29ce06d78d3db71b3ab26a7465fc8a9586
+Author: David Gibson <david@gibson.dropbear.id.au>
+Date:   Fri Oct 6 23:07:30 2017 +1100
+
+    tests: Avoid 64-bit arithmetic in assembler
+    
+    For testing we (ab)use the assembler to build us a sample dtb, independent
+    of the other tools (dtc and libfdt) that we're trying to test.  In a few
+    places this uses 64-bit arithmetic to decompose 64-bit constants into
+    the individual bytes in the blob.
+    
+    Unfortunately, it seems that some builds of GNU as don't support >32 bit
+    arithmetic, though it's not entirely clear to me which do and which don't
+    (Fedora i386 does support 64-bit, Debian arm32 doesn't).
+    
+    Anyway, to be safe, this avoids 64-bit arithmetic in assembler at the cost
+    of some extra awkwardness because we have to define the values in 32-bit
+    halves.
+    
+    Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
+
+diff --git a/tests/testdata.h b/tests/testdata.h
+index 3588778..f6bbe1d 100644
+--- a/tests/testdata.h
++++ b/tests/testdata.h
+@@ -4,15 +4,25 @@
+ #define ASM_CONST_LL(x)	(x##ULL)
+ #endif
+ 
+-#define TEST_ADDR_1	ASM_CONST_LL(0xdeadbeef00000000)
+-#define TEST_SIZE_1	ASM_CONST_LL(0x100000)
+-#define TEST_ADDR_2	ASM_CONST_LL(123456789)
+-#define TEST_SIZE_2	ASM_CONST_LL(010000)
++#define TEST_ADDR_1H	ASM_CONST_LL(0xdeadbeef)
++#define TEST_ADDR_1L	ASM_CONST_LL(0x00000000)
++#define TEST_ADDR_1	((TEST_ADDR_1H << 32) | TEST_ADDR_1L)
++#define TEST_SIZE_1H	ASM_CONST_LL(0x00000000)
++#define TEST_SIZE_1L	ASM_CONST_LL(0x00100000)
++#define TEST_SIZE_1	((TEST_SIZE_1H << 32) | TEST_SIZE_1L)
++#define TEST_ADDR_2H	ASM_CONST_LL(0)
++#define TEST_ADDR_2L	ASM_CONST_LL(123456789)
++#define TEST_ADDR_2	((TEST_ADDR_2H << 32) | TEST_ADDR_2L)
++#define TEST_SIZE_2H	ASM_CONST_LL(0)
++#define TEST_SIZE_2L	ASM_CONST_LL(010000)
++#define TEST_SIZE_2	((TEST_SIZE_2H << 32) | TEST_SIZE_2L)
+ 
+ #define TEST_VALUE_1	0xdeadbeef
+ #define TEST_VALUE_2	123456789
+ 
+-#define TEST_VALUE64_1	ASM_CONST_LL(0xdeadbeef01abcdef)
++#define TEST_VALUE64_1H	ASM_CONST_LL(0xdeadbeef)
++#define TEST_VALUE64_1L	ASM_CONST_LL(0x01abcdef)
++#define TEST_VALUE64_1	((TEST_VALUE64_1H << 32) | TEST_VALUE64_1L)
+ 
+ #define PHANDLE_1	0x2000
+ #define PHANDLE_2	0x2001
+diff --git a/tests/trees.S b/tests/trees.S
+index 9854d1d..9859914 100644
+--- a/tests/trees.S
++++ b/tests/trees.S
+@@ -7,16 +7,6 @@
+ 	.byte	((val) >> 8) & 0xff ; \
+ 	.byte	(val) & 0xff	;
+ 
+-#define FDTQUAD(val) \
+-	.byte	((val) >> 56) & 0xff ; \
+-	.byte	((val) >> 48) & 0xff ; \
+-	.byte	((val) >> 40) & 0xff ; \
+-	.byte	((val) >> 32) & 0xff ; \
+-	.byte	((val) >> 24) & 0xff ; \
+-	.byte	((val) >> 16) & 0xff ; \
+-	.byte	((val) >> 8) & 0xff ; \
+-	.byte	(val) & 0xff	;
+-
+ #define TREE_HDR(tree) \
+ 	.balign	8		; \
+ 	.globl	_##tree		; \
+@@ -33,14 +23,16 @@ tree:	\
+ 	FDTLONG(tree##_strings_end - tree##_strings) ; \
+ 	FDTLONG(tree##_struct_end - tree##_struct) ;
+ 
+-#define RSVMAP_ENTRY(addr, len) \
+-	FDTQUAD(addr)		; \
+-	FDTQUAD(len)		; \
++#define RSVMAP_ENTRY(addrh, addrl, lenh, lenl) \
++	FDTLONG(addrh)		; \
++	FDTLONG(addrl)		; \
++	FDTLONG(lenh)		; \
++	FDTLONG(lenl)
+ 
+ #define EMPTY_RSVMAP(tree) \
+ 	.balign	8		; \
+ tree##_rsvmap:			; \
+-	RSVMAP_ENTRY(0, 0) \
++	RSVMAP_ENTRY(0, 0, 0, 0) \
+ tree##_rsvmap_end:		;
+ 
+ #define PROPHDR(tree, name, len) \
+@@ -52,9 +44,10 @@ tree##_rsvmap_end:		;
+ 	PROPHDR(tree, name, 4) \
+ 	FDTLONG(val)		;
+ 
+-#define PROP_INT64(tree, name, val) \
++#define PROP_INT64(tree, name, valh, vall) \
+ 	PROPHDR(tree, name, 8) \
+-	FDTQUAD(val)		;
++	FDTLONG(valh)		; \
++	FDTLONG(vall)		;
+ 
+ #define PROP_STR(tree, name, str) \
+ 	PROPHDR(tree, name, 55f - 54f) \
+@@ -81,16 +74,16 @@ tree##_##name:			; \
+ 
+ 	.balign	8
+ test_tree1_rsvmap:
+-	RSVMAP_ENTRY(TEST_ADDR_1, TEST_SIZE_1)
+-	RSVMAP_ENTRY(TEST_ADDR_2, TEST_SIZE_2)
+-	RSVMAP_ENTRY(0, 0)
++	RSVMAP_ENTRY(TEST_ADDR_1H, TEST_ADDR_1L, TEST_SIZE_1H, TEST_SIZE_1L)
++	RSVMAP_ENTRY(TEST_ADDR_2H, TEST_ADDR_2L, TEST_SIZE_2H, TEST_SIZE_2L)
++	RSVMAP_ENTRY(0, 0, 0, 0)
+ test_tree1_rsvmap_end:
+ 
+ test_tree1_struct:
+ 	BEGIN_NODE("")
+ 	PROP_STR(test_tree1, compatible, "test_tree1")
+ 	PROP_INT(test_tree1, prop_int, TEST_VALUE_1)
+-	PROP_INT64(test_tree1, prop_int64, TEST_VALUE64_1)
++	PROP_INT64(test_tree1, prop_int64, TEST_VALUE64_1H, TEST_VALUE64_1L)
+ 	PROP_STR(test_tree1, prop_str, TEST_STRING_1)
+ 	PROP_INT(test_tree1, address_cells, 1)
+ 	PROP_INT(test_tree1, size_cells, 0)
diff --git a/gnu/packages/patches/dtc-format-modifier.patch b/gnu/packages/patches/dtc-format-modifier.patch
new file mode 100644
index 0000000..c33d168
--- /dev/null
+++ b/gnu/packages/patches/dtc-format-modifier.patch
@@ -0,0 +1,38 @@
+This fixes build on 32 bits platforms. This patch is taken from upstream.
+
+commit 497432fd2131967f349e69dc5d259072151cc4b4
+Author: Thierry Reding <treding@nvidia.com>
+Date:   Wed Sep 27 15:04:09 2017 +0200
+
+    checks: Use proper format modifier for size_t
+    
+    The size of size_t can vary between architectures, so using %ld isn't
+    going to work on 32-bit builds. Use the %zu modifier to make sure it is
+    always correct.
+    
+    Signed-off-by: Thierry Reding <treding@nvidia.com>
+    Acked-by: Rob Herring <robh@kernel.org>
+    Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
+
+diff --git a/checks.c b/checks.c
+index 902f2e3..08a3a29 100644
+--- a/checks.c
++++ b/checks.c
+@@ -972,7 +972,7 @@ static void check_property_phandle_args(struct check *c,
+ 	int cell, cellsize = 0;
+ 
+ 	if (prop->val.len % sizeof(cell_t)) {
+-		FAIL(c, dti, "property '%s' size (%d) is invalid, expected multiple of %ld in node %s",
++		FAIL(c, dti, "property '%s' size (%d) is invalid, expected multiple of %zu in node %s",
+ 		     prop->name, prop->val.len, sizeof(cell_t), node->fullpath);
+ 		return;
+ 	}
+@@ -1163,7 +1163,7 @@ static void check_interrupts_property(struct check *c,
+ 		return;
+ 
+ 	if (irq_prop->val.len % sizeof(cell_t))
+-		FAIL(c, dti, "property '%s' size (%d) is invalid, expected multiple of %ld in node %s",
++		FAIL(c, dti, "property '%s' size (%d) is invalid, expected multiple of %zu in node %s",
+ 		     irq_prop->name, irq_prop->val.len, sizeof(cell_t),
+ 		     node->fullpath);
+ 
-- 
2.7.4

[bug#29409] [PATCH] gnu: grub: Disable tests on ARM platforms.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/packages/bootloaders (grub): Disable tests on ARM platforms.
Half of the tests are failing on ARM.
---
 gnu/packages/bootloaders.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index d28fe32..c66d374 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -75,7 +75,7 @@
                "03vvdfhdmf16121v7xs8is2krwnv15wpkhkf16a4yf8nsfc3f2w1"))))
     (build-system gnu-build-system)
     (arguments
-     '(#:phases (modify-phases %standard-phases
+     `(#:phases (modify-phases %standard-phases
                   (add-after 'unpack 'patch-stuff
                    (lambda* (#:key inputs #:allow-other-keys)
                      (substitute* "grub-core/Makefile.in"
@@ -102,7 +102,10 @@
                       (substitute* "Makefile.in"
                         (("grub_cmd_date grub_cmd_set_date grub_cmd_sleep")
                           "grub_cmd_date grub_cmd_sleep"))
-                      #t)))))
+                      #t)))
+       ;; Disable tests on ARM platforms.
+       #:tests? ,(not (string-prefix? "arm" (or (%current-target-system)
+                                                (%current-system))))))
     (inputs
      `(("gettext" ,gettext-minimal)
 
-- 
2.7.4

[bug#29409] [PATCH] gnu: grub: Disable tests on ARM platforms.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

On Tue, Nov 28, 2017 at 10:28:15AM +0100, m.othacehe@gmail.com wrote:
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> 
> * gnu/packages/bootloaders (grub): Disable tests on ARM platforms.
> Half of the tests are failing on ARM.

Okay. Can you also report the test failures upstream?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#29409] [PATCH] gnu: grub: Disable tests on ARM platforms.

[Mathieu Othacehe]

Hi Leo,

> Okay. Can you also report the test failures upstream?

Thanks pushed, i reported it at bug-grub@gnu.org.

Mathieu

[bug#29409] [PATCH] utils: Add target-arm? procedure.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* guix/utils.scm (target-arm?): New exported procedure.
---

Hi,

This procedure may be replaced by let-system when operational, see :

https://lists.nongnu.org/archive/html/guix-patches/2017-11/msg00274.html

Thanks,

Mathieu

 guix/utils.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/guix/utils.scm b/guix/utils.scm
index c0ffed1..9c652b6 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -75,6 +75,7 @@
             %current-target-system
             package-name->name+version
             target-mingw?
+            target-arm?
             version-compare
             version>?
             version>=?
@@ -467,6 +468,9 @@ a character other than '@'."
   (and target
        (string-suffix? "-mingw32" target)))
 
+(define (target-arm?)
+  (string-prefix? "arm" (or (%current-target-system) (%current-system))))
+
 (define version-compare
   (let ((strverscmp
          (let ((sym (or (dynamic-func "strverscmp" (dynamic-link))
-- 
2.7.4

[bug#29409] [PATCH] system: vm: Do not add EFI partition on ARM system.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/system/vm.scm (qemu-img): Do not add EFI partition if we are targetting
  ARM.

UEFI support on u-boot is still experimental, so do not add EFI partition on
ARM for now.
---
 gnu/system/vm.scm | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 3ddb41d..7d0fa07 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -29,6 +29,7 @@
   #:use-module (guix monads)
   #:use-module (guix records)
   #:use-module (guix modules)
+  #:use-module (guix utils)
 
   #:use-module ((gnu build vm)
                 #:select (qemu-command))
@@ -312,26 +313,30 @@ the image."
                                              graphs)))
                                     (- disk-image-size
                                        (* 50 (expt 2 20)))))
-                  (partitions (list (partition
-                                     (size root-size)
-                                     (label #$file-system-label)
-                                     (uuid #$(and=> file-system-uuid
-                                                    uuid-bytevector))
-                                     (file-system #$file-system-type)
-                                     (flags '(boot))
-                                     (initializer initialize))
-                                    ;; Append a small EFI System Partition for
-                                    ;; use with UEFI bootloaders.
-                                    (partition
-                                     ;; The standalone grub image is about 10MiB, but
-                                     ;; leave some room for custom or multiple images.
-                                     (size (* 40 (expt 2 20)))
-                                     (label "GNU-ESP")             ;cosmetic only
-                                     ;; Use "vfat" here since this property is used
-                                     ;; when mounting. The actual FAT-ness is based
-                                     ;; on filesystem size (16 in this case).
-                                     (file-system "vfat")
-                                     (flags '(esp))))))
+                  (partitions
+                   (append
+                    (list (partition
+                           (size root-size)
+                           (label #$file-system-label)
+                           (uuid #$(and=> file-system-uuid
+                                          uuid-bytevector))
+                           (file-system #$file-system-type)
+                           (flags '(boot))
+                           (initializer initialize)))
+                    ;; Append a small EFI System Partition for use with UEFI
+                    ;; bootloaders if we are not targetting ARM.
+                    (if #$(target-arm?)
+                        '()
+                        (list (partition
+                               ;; The standalone grub image is about 10MiB, but
+                               ;; leave some room for custom or multiple images.
+                               (size (* 40 (expt 2 20)))
+                               (label "GNU-ESP")             ;cosmetic only
+                               ;; Use "vfat" here since this property is used
+                               ;; when mounting. The actual FAT-ness is based
+                               ;; on filesystem size (16 in this case).
+                               (file-system "vfat")
+                               (flags '(esp))))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
                                    #:grub-efi #$grub-efi
-- 
2.7.4

[bug#29409] [PATCH] system: vm: Do not add EFI partition on ARM system.

[Danny Milosavljevic]

> +                    ;; bootloaders if we are not targetting ARM.

"targeting" (one "t").

LGTM otherwise.

Be advised that there are ARM processors that do use EFI (some ARMv8 and aarch64 boards).  I'm not opposed to this patch but in the future it might be better to have a flag specify whether to use UEFI.

[bug#29409] [PATCH] system: vm: Do not add EFI partition on ARM system.

[Mathieu Othacehe]

> Be advised that there are ARM processors that do use EFI (some ARMv8 and aarch64 boards).  I'm not opposed to this patch but in the future it might be better to have a flag specify whether to use UEFI.

Ok. Then, when GuixSD will support ARMv8 and aarch64 boards, maybe we
will have to add an "esp-partition?" flag to "qemu-image" procedure.

Thanks for reviewing,

Mathieu

[bug#29409] [PATCH] system: vm: Do not add EFI partition on ARM system.

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Be advised that there are ARM processors that do use EFI (some ARMv8 and aarch64 boards).  I'm not opposed to this patch but in the future it might be better to have a flag specify whether to use UEFI.

The SoftIron OverDrive 1000 boxes (aarch64) that we were just donated
use UEFI.

This might actually make porting easier since it’s pretty much
indistinguishable from x86_64/UEFI, isn’t it?

Ludo’.

[bug#29409] [PATCH] utils: Add target-arm? procedure.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * guix/utils.scm (target-arm?): New exported procedure.

Should it match aarch64 as well?  If not, should it be called
’target-arm32?’ or something like that?

> This procedure may be replaced by let-system when operational, see :
>
> https://lists.nongnu.org/archive/html/guix-patches/2017-11/msg00274.html

I think it would be used in the body of ‘let-system’ forms, but
‘let-system’ alone doesn’t render it useless.

Thanks,
Ludo’.

[bug#29409] [PATCH] utils: Add target-arm? procedure.

[Mathieu Othacehe]

> Should it match aarch64 as well?  If not, should it be called
> ’target-arm32?’ or something like that?

For now, i can't test my patches on aarch64 and i'm not sure how they
behave. So maybe we can use target-armv7? or target-arm32? for now and
replace them with target-arm? when it's proven that the serie works on
aarch64 ?

Thanks,

Mathieu

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/tests/install.scm (%extlinux-gpt-installation-script): Increase
  partition size from 1G to 2G. 1G is not enough anymore to initialize the
  system.
---
 gnu/tests/install.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 0e97de0..fa9d9c6 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -173,7 +173,7 @@ guix --version
 export GUIX_BUILD_OPTIONS=--no-grafts
 guix build isc-dhcp
 parted --script /dev/vdb mklabel gpt \\
-  mkpart ext2 1M 1G \\
+  mkpart ext2 1M 2G \\
   set 1 legacy_boot on
 mkfs.ext4 -L my-root -O '^64bit' /dev/vdb1
 mount /dev/vdb1 /mnt
-- 
2.7.4

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/tests/install.scm (%extlinux-gpt-installation-script): Increase
>   partition size from 1G to 2G. 1G is not enough anymore to initialize the
>   system.

Does 53d26163c6ec1a1c48a1ec775e67b739b9005270 allow bare-bones to fit in
1G again?  That was the main motivation behind this patch.

Thanks,
Ludo’.

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Mathieu Othacehe]

> Does 53d26163c6ec1a1c48a1ec775e67b739b9005270 allow bare-bones to fit in
> 1G again?  That was the main motivation behind this patch.

Ok, i'm running the test again on 1776d5cd but it fails when building
guix with the following errors :

--8<---------------cut here---------------start------------->8---
  GUILEC   guix/scripts/import/json.go
  GUILEC   guix/scripts/import/pypi.go
  GUILEC   guix/scripts/import/stackage.go
  GUILEC   guix/ssh.go
  GUILEC   guix/scripts/copy.go
  GUILEC   guix/store/ssh.go
  GUILEC   guix/scripts/offload.go
  GUILEC   guix/config.go
  GUILEC   guix/tests.go
  GUILEC   guix/tests/http.go
random seed for tests: 1512112612
;;; Failed to autoload make-page-map in (charting):
;;; Failed to autoload make-page-map in (charting):
;;; ERROR: missing interface for module (charting)
;;; Failed to autoload make-page-map in (charting):
;;; ERROR: missing interface for module (charting)
guix/scripts/size.scm:221:2: warning: possibly unbound variable `make-page-map'
;;; Failed to autoload make-page-map in (charting):
;;; ERROR: missing interface for module (charting)
;;; Failed to autoload make-page-map in (charting):
;;; ERROR: missing interface for module (charting)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload exec-command in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload exec-command in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
gnu/build/shepherd.scm:100:13: warning: possibly unbound variable `read-pid-file'
gnu/build/shepherd.scm:161:32: warning: possibly unbound variable `exec-command'
gnu/build/shepherd.scm:172:14: warning: possibly unbound variable `read-pid-file'
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload exec-command in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload exec-command in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
;;; Failed to autoload read-pid-file in (shepherd service):
;;; ERROR: missing interface for module (shepherd service)
Backtrace:
Exception thrown while printing backtrace:
ERROR: In procedure public-lookup: Module named (system repl debug) does not exist


Some deprecated features have been used.  Set the environment 
variable GUILE_WARN_DEPRECATED to "detailed" and rerun the
program to get more information.  Set it to "no" to suppress  
this message.
make[2]: *** [Makefile:5309: make-go] Error 1
make[2]: Leaving directory '/tmp/guix-build-guix-0.13.0-12.0a154c1+.drv-0/source'
--8<---------------cut here---------------end--------------->8---

The test "installed-os" also fails with the same error.

Any idea of what is going on ?

Thanks,

Mathieu

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Mathieu Othacehe]

> The test "installed-os" also fails with the same error.
>
> Any idea of what is going on ?

After git clean and make clean this issue disappears.
However, another issue appears :

when running "guix system init /mnt/etc/config.scm /mnt
--no-substitutes" in qemu, it tries to build several packages including
binutils.

This fails because there is no network in qemu machine.

--8<---------------cut here---------------start------------->8---
Starting download of /gnu/store/a62j9z64i667zi6c2g9xhm15pld6rnyz-binutils-2.23.2.tar.xz
From ftp://alpha.gnu.org/gnu/guix/bootstrap/x86_64-linux/20131110/binutils-2.23.2.tar.xz...
ERROR: In procedure getaddrinfo: Name or service not known
--8<---------------cut here---------------end--------------->8---

Mathieu

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> The test "installed-os" also fails with the same error.
>>
>> Any idea of what is going on ?
>
> After git clean and make clean this issue disappears.
> However, another issue appears :
>
> when running "guix system init /mnt/etc/config.scm /mnt
> --no-substitutes" in qemu, it tries to build several packages including
> binutils.
>
> This fails because there is no network in qemu machine.
>
> Starting download of /gnu/store/a62j9z64i667zi6c2g9xhm15pld6rnyz-binutils-2.23.2.tar.xz
> From ftp://alpha.gnu.org/gnu/guix/bootstrap/x86_64-linux/20131110/binutils-2.23.2.tar.xz...
> ERROR: In procedure getaddrinfo: Name or service not known

Presumably that’s because of grafts.

The tests in (gnu tests install) go to great lengths to ensure that this
doesn’t happen.  At least you’ll have to use --no-grafts.

HTH,
Ludo’.

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Mathieu Othacehe]

Hey Ludo,

> Presumably that’s because of grafts.
>
> The tests in (gnu tests install) go to great lengths to ensure that this
> doesn’t happen.  At least you’ll have to use --no-grafts.

On two different machines, with those two commands :

--8<---------------cut here---------------start------------->8---
GUIX_BUILD_OPTIONS="--no-grafts" && make check-system TESTS="installed-os"
make check-system TESTS="installed-os"
--8<---------------cut here---------------end--------------->8---

i have the same result, the vm tries to download binutils sources.

Do you see another reason ?

Thanks,

Mathieu

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Mathieu Othacehe]

I meant :

--8<---------------cut here---------------start------------->8---
GUIX_BUILD_OPTIONS="--no-grafts" make check-system TESTS="installed-os"
make check-system TESTS="installed-os"
--8<---------------cut here---------------end--------------->8---

Mathieu

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Ludovic Courtès]

Hi,

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> Presumably that’s because of grafts.
>>
>> The tests in (gnu tests install) go to great lengths to ensure that this
>> doesn’t happen.  At least you’ll have to use --no-grafts.
>
> On two different machines, with those two commands :
>
> GUIX_BUILD_OPTIONS="--no-grafts" && make check-system TESTS="installed-os"
> make check-system TESTS="installed-os"
>
> i have the same result, the vm tries to download binutils sources.

Indeed, I experience the same thing, and Marius and I were discussing it
on IRC.

I’m pretty sure it went well
ca. 15c2ddc12460c9d1e26fb89639b460b8a0b3ffc0, but I haven’t found what’s
causing the problem.

To be continued…

Ludo’.

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Ludovic Courtès]

Hi,

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> The test "installed-os" also fails with the same error.
>>
>> Any idea of what is going on ?
>
> After git clean and make clean this issue disappears.
> However, another issue appears :
>
> when running "guix system init /mnt/etc/config.scm /mnt
> --no-substitutes" in qemu, it tries to build several packages including
> binutils.
>
> This fails because there is no network in qemu machine.
>
> Starting download of /gnu/store/a62j9z64i667zi6c2g9xhm15pld6rnyz-binutils-2.23.2.tar.xz
> From ftp://alpha.gnu.org/gnu/guix/bootstrap/x86_64-linux/20131110/binutils-2.23.2.tar.xz...
> ERROR: In procedure getaddrinfo: Name or service not known

It took me a while to figure it out, but the fix is here:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=cdc938daf91f159e082c5b81a44b074f7bf6d991

Let me know if anything’s amiss!

Thanks,
Ludo’.

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]


Hey Ludo !

>   https://git.savannah.gnu.org/cgit/guix.git/commit/?id=cdc938daf91f159e082c5b81a44b074f7bf6d991
>
> Let me know if anything’s amiss!

Thanks, it looks rather tricky ! I run the test suite again and got a
few failures when building guix :

The error seems to come from :

--8<---------------cut here---------------start------------->8---
ERROR: In procedure setvbuf:
ERROR: Wrong type (expecting exact integer): line
builder for `/tmp/guix-tests/store/c12sgw3ip2pmwbrmz9yld5ndfq3ck5x3-profile.drv' failed with exit code 1
--8<---------------cut here---------------end--------------->8---

I attached the report, and will have a clother look tomorrow.

Mathieu


[-- Attachment #2: test-suite.log --]
[-- Type: application/octet-stream, Size: 420744 bytes --]

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Ludovic Courtès]

Heya,

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>>   https://git.savannah.gnu.org/cgit/guix.git/commit/?id=cdc938daf91f159e082c5b81a44b074f7bf6d991
>>
>> Let me know if anything’s amiss!
>
> Thanks, it looks rather tricky ! I run the test suite again and got a
> few failures when building guix :
>
> The error seems to come from :
>
> ERROR: In procedure setvbuf:
> ERROR: Wrong type (expecting exact integer): line
> builder for `/tmp/guix-tests/store/c12sgw3ip2pmwbrmz9yld5ndfq3ck5x3-profile.drv' failed with exit code 1

Thing is, every time I push a fix, I also push a new bug.  ;-)

This one was fixed a few minutes later in
2815fca1423cf72e6f3d0e774f1058bcbf8dfdbf.

Thanks,
Ludo’.

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Mathieu Othacehe]

> This one was fixed a few minutes later in
> 2815fca1423cf72e6f3d0e774f1058bcbf8dfdbf.

I was able to test "installed-os" and "installed-extlinux-os" everything
seems back to normal.

Thanks again,

Mathieu

[bug#29409] [PATCH] tests: install: Increase extlinux install partition size.

[Mathieu Othacehe]

> Does 53d26163c6ec1a1c48a1ec775e67b739b9005270 allow bare-bones to fit in
> 1G again?  That was the main motivation behind this patch.

Yes it does, so this patch becomes useless.

Mathieu

[bug#29409] [PATCH] build: utils: Introduce dd.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* guix/build/utils.scm (dd): New exported procedure.
* gnu/bootloader/extlinux.scm (dd): Remove it,
(install-extlinux): replace gexp dd with dd added above.
---
Hi,

dd will be used in different bootloader related gexp. So it may be
good to add it to (guix build utils). The problem is that
it triggers a big rebuild. I was able to test this path with
"installed-extlinux-os".

Thanks,

Mathieu

 gnu/bootloader/extlinux.scm | 13 ++++---------
 guix/build/utils.scm        | 28 +++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm
index 0db5598..b0b463e 100644
--- a/gnu/bootloader/extlinux.scm
+++ b/gnu/bootloader/extlinux.scm
@@ -85,14 +85,6 @@ TIMEOUT ~a~%"
 ;;; Install procedures.
 ;;;
 
-(define dd
-  #~(lambda (bs count if of)
-      (zero? (system* "dd"
-                      (string-append "bs=" (number->string bs))
-                      (string-append "count=" (number->string count))
-                      (string-append "if=" if)
-                      (string-append "of=" of)))))
-
 (define (install-extlinux mbr)
   #~(lambda (bootloader device mount-point)
       (let ((extlinux (string-append bootloader "/sbin/extlinux"))
@@ -103,7 +95,10 @@ TIMEOUT ~a~%"
                   (find-files syslinux-dir "\\.c32$"))
 
         (unless (and (zero? (system* extlinux "--install" install-dir))
-                     (#$dd 440 1 (string-append syslinux-dir "/" #$mbr) device))
+                     (zero? (dd (string-append syslinux-dir "/" #$mbr)
+                                device
+                                #:bs 440
+                                #:count 1)))
           (error "failed to install SYSLINUX")))))
 
 (define install-extlinux-mbr
diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 7391307..2ed5ddc 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2013 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -86,7 +87,8 @@
             wrap-program
             invoke
 
-            locale-category->string))
+            locale-category->string
+            dd))
 
 
 ;;;
@@ -1089,6 +1091,30 @@ returned."
              LC_NAME LC_NUMERIC LC_PAPER LC_TELEPHONE
              LC_TIME)))
 
+\f
+;;;
+;;; dd.
+;;;
+
+(define* (dd input output #:key bs count seek (extras '()))
+  "Call dd command with provided INPUT and OUTPUT arguments. BS, COUNT, SEEK
+  and EXTRAS parameters are optional. EXTRAS is a list of string arguments to
+  be passed directly to dd."
+  (apply system* "dd"
+         (string-append "if=" input)
+         (string-append "of=" output)
+         (append
+          (if bs
+              `(,(string-append "bs=" (number->string bs)))
+              '())
+          (if count
+              `(,(string-append "count=" (number->string count)))
+              '())
+          (if seek
+              `(,(string-append "seek=" (number->string seek)))
+              '())
+          extras)))
+
 ;;; Local Variables:
 ;;; eval: (put 'call-with-output-file/atomic 'scheme-indent-function 1)
 ;;; eval: (put 'call-with-ascii-input-file 'scheme-indent-function 1)
-- 
2.7.4

[bug#29409] [PATCH] build: utils: Introduce dd.

[Ludovic Courtès]

Hello,

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * guix/build/utils.scm (dd): New exported procedure.
> * gnu/bootloader/extlinux.scm (dd): Remove it,
> (install-extlinux): replace gexp dd with dd added above.
> ---
> Hi,
>
> dd will be used in different bootloader related gexp. So it may be
> good to add it to (guix build utils). The problem is that
> it triggers a big rebuild. I was able to test this path with
> "installed-extlinux-os".

Yes, changing (guix build utils) triggers a full rebuild because
everything depends on it.

> +(define* (dd input output #:key bs count seek (extras '()))
> +  "Call dd command with provided INPUT and OUTPUT arguments. BS, COUNT, SEEK
> +  and EXTRAS parameters are optional. EXTRAS is a list of string arguments to
> +  be passed directly to dd."
> +  (apply system* "dd"
> +         (string-append "if=" input)
> +         (string-append "of=" output)
> +         (append
> +          (if bs
> +              `(,(string-append "bs=" (number->string bs)))
> +              '())
> +          (if count
> +              `(,(string-append "count=" (number->string count)))
> +              '())
> +          (if seek
> +              `(,(string-append "seek=" (number->string seek)))
> +              '())
> +          extras)))

I’m not quite convinced.  :-)  It seems to me that it doesn’t buy us
much to have it in (guix build utils), because we don’t need it very
often anyway, and secondly, I think we can use ‘dump-port’ or other I/O
procedures instead.

Namely:

+                     (zero? (dd (string-append syslinux-dir "/" #$mbr)
+                                device
+                                #:bs 440
+                                #:count 1)))

would become:

  (call-with-input-file (string-append syslinux-dir "/" #$mbr)
    (lambda (input)
      (let ((bv (get-bytevector-n input 440))
            (output (open-file device "wb0")))
        (put-bytevector output bv)
        (close-port output))))

Granted, that’s a bit more verbose, but it’s also very lightweight
compared to using ‘dd’.

WDYT?

Ludo’.

[bug#29409] [PATCH] build: utils: Introduce dd.

[Mathieu Othacehe]

>   (call-with-input-file (string-append syslinux-dir "/" #$mbr)
>     (lambda (input)
>       (let ((bv (get-bytevector-n input 440))
>             (output (open-file device "wb0")))
>         (put-bytevector output bv)
>         (close-port output))))
>
> Granted, that’s a bit more verbose, but it’s also very lightweight
> compared to using ‘dd’.

It's much more elegant anyway ! I'll adapt the patch.

Thanks,

Mathieu

[bug#29409] [PATCH] build: utils: Introduce dd.

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]


> Granted, that’s a bit more verbose, but it’s also very lightweight
> compared to using ‘dd’.

Here's a new implementation, using your snippet. I tested it with
"installed-extlinux-os", it seems ok.

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-bootloader-extlinux-Stop-using-dd-binary.patch --]
[-- Type: text/x-diff, Size: 3865 bytes --]

From 742662ceec2a40d664520f01977ddc4cbe64d369 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Fri, 1 Dec 2017 14:09:38 +0100
Subject: [PATCH] bootloader: extlinux: Stop using dd binary.

* gnu/bootloader/extlinux.scm (dd): Remove it,
(install-extlinux): replace dd call by Guile I/O procedures.
* gnu/system/vm.scm (qemu-image): Add (ice-9 binary-ports) to module-closure
and used-modules list to provide "get-bytevector-n" and "put-bytevector".
* guix/scripts/system.scm (bootloader-installer-derivation): Ditto.
---
 gnu/bootloader/extlinux.scm | 18 ++++++++----------
 gnu/system/vm.scm           |  6 ++++--
 guix/scripts/system.scm     |  6 ++++--
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm
index 0db5598..98fad0c 100644
--- a/gnu/bootloader/extlinux.scm
+++ b/gnu/bootloader/extlinux.scm
@@ -85,14 +85,6 @@ TIMEOUT ~a~%"
 ;;; Install procedures.
 ;;;
 
-(define dd
-  #~(lambda (bs count if of)
-      (zero? (system* "dd"
-                      (string-append "bs=" (number->string bs))
-                      (string-append "count=" (number->string count))
-                      (string-append "if=" if)
-                      (string-append "of=" of)))))
-
 (define (install-extlinux mbr)
   #~(lambda (bootloader device mount-point)
       (let ((extlinux (string-append bootloader "/sbin/extlinux"))
@@ -102,8 +94,14 @@ TIMEOUT ~a~%"
                     (install-file file install-dir))
                   (find-files syslinux-dir "\\.c32$"))
 
-        (unless (and (zero? (system* extlinux "--install" install-dir))
-                     (#$dd 440 1 (string-append syslinux-dir "/" #$mbr) device))
+        (unless
+            (and (zero? (system* extlinux "--install" install-dir))
+                 (call-with-input-file (string-append syslinux-dir "/" #$mbr)
+                   (lambda (input)
+                     (let ((bv (get-bytevector-n input 440))
+                           (output (open-file device "wb0")))
+                       (put-bytevector output bv)
+                       (close-port output)))))
           (error "failed to install SYSLINUX")))))
 
 (define install-extlinux-mbr
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index b68cce3..b5fe786 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -274,11 +274,13 @@ the image."
   (expression->derivation-in-linux-vm
    name
    (with-imported-modules (source-module-closure '((gnu build vm)
-                                                   (guix build utils)))
+                                                   (guix build utils)
+                                                   (ice-9 binary-ports)))
      #~(begin
          (use-modules (gnu build vm)
                       (guix build utils)
-                      (srfi srfi-26))
+                      (srfi srfi-26)
+                      (ice-9 binary-ports))
 
          (let ((inputs
                 '#$(append (list qemu parted e2fsprogs dosfstools)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 91d151d..5116b82 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -674,9 +674,11 @@ any, are available.  Raise an error if they're not."
 and TARGET arguments."
   (with-monad %store-monad
     (gexp->file "bootloader-installer"
-                (with-imported-modules '((guix build utils))
+                (with-imported-modules '((guix build utils)
+                                         (ice-9 binary-ports))
                   #~(begin
-                      (use-modules (guix build utils))
+                      (use-modules (guix build utils)
+                                   (ice-9 binary-ports))
                       (#$installer #$bootloader #$device #$target))))))
 
 (define* (perform-action action os
-- 
2.7.4

[bug#29409] [PATCH] build: utils: Introduce dd.

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> Granted, that’s a bit more verbose, but it’s also very lightweight
>> compared to using ‘dd’.
>
> Here's a new implementation, using your snippet. I tested it with
> "installed-extlinux-os", it seems ok.
>
> Mathieu
>
> From 742662ceec2a40d664520f01977ddc4cbe64d369 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> Date: Fri, 1 Dec 2017 14:09:38 +0100
> Subject: [PATCH] bootloader: extlinux: Stop using dd binary.
>
> * gnu/bootloader/extlinux.scm (dd): Remove it,
> (install-extlinux): replace dd call by Guile I/O procedures.
> * gnu/system/vm.scm (qemu-image): Add (ice-9 binary-ports) to module-closure
> and used-modules list to provide "get-bytevector-n" and "put-bytevector".
> * guix/scripts/system.scm (bootloader-installer-derivation): Ditto.

Nice!

> +        (unless
> +            (and (zero? (system* extlinux "--install" install-dir))
> +                 (call-with-input-file (string-append syslinux-dir "/" #$mbr)
> +                   (lambda (input)
> +                     (let ((bv (get-bytevector-n input 440))
> +                           (output (open-file device "wb0")))
> +                       (put-bytevector output bv)
> +                       (close-port output)))))

I think you can use ‘call-with-output-file’ for DEVICE no?

> --- a/gnu/system/vm.scm
> +++ b/gnu/system/vm.scm
> @@ -274,11 +274,13 @@ the image."
>    (expression->derivation-in-linux-vm
>     name
>     (with-imported-modules (source-module-closure '((gnu build vm)
> -                                                   (guix build utils)))
> +                                                   (guix build utils)
> +                                                   (ice-9 binary-ports)))

No: this would import the host Guile’s (ice-9 binary-ports) module into
the guest, thereby making the result dependent on the Guile version
being used “outside”.

>       #~(begin
>           (use-modules (gnu build vm)
>                        (guix build utils)
> -                      (srfi srfi-26))
> +                      (srfi srfi-26)
> +                      (ice-9 binary-ports))

This yes.

>           (let ((inputs
>                  '#$(append (list qemu parted e2fsprogs dosfstools)
> diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
> index 91d151d..5116b82 100644
> --- a/guix/scripts/system.scm
> +++ b/guix/scripts/system.scm
> @@ -674,9 +674,11 @@ any, are available.  Raise an error if they're not."
>  and TARGET arguments."
>    (with-monad %store-monad
>      (gexp->file "bootloader-installer"
> -                (with-imported-modules '((guix build utils))
> +                (with-imported-modules '((guix build utils)
> +                                         (ice-9 binary-ports))

No.

>                    #~(begin
> -                      (use-modules (guix build utils))
> +                      (use-modules (guix build utils)
> +                                   (ice-9 binary-ports))

OK.

Could you send an updated patch?

Thanks!

Ludo’.

[bug#29409] [PATCH] build: utils: Introduce dd.

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]


> I think you can use ‘call-with-output-file’ for DEVICE no?

Sure.

> No: this would import the host Guile’s (ice-9 binary-ports) module into
> the guest, thereby making the result dependent on the Guile version
> being used “outside”.

Ok got it.

> Could you send an updated patch?

You'll find the updated patch attached.

Thanks for your fast review !

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-bootloader-extlinux-Stop-using-dd-binary.patch --]
[-- Type: text/x-diff, Size: 3367 bytes --]

From 143057ead198a75e8a7a2bb1bb706402dcb7d4c3 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Fri, 1 Dec 2017 14:09:38 +0100
Subject: [PATCH] bootloader: extlinux: Stop using dd binary.

* gnu/bootloader/extlinux.scm (dd): Remove it,
(install-extlinux): replace dd call by Guile I/O procedures.
* gnu/system/vm.scm (qemu-image): Add (ice-9 binary-ports) to used-modules
list to provide "get-bytevector-n" and "put-bytevector".
* guix/scripts/system.scm (bootloader-installer-derivation): Ditto.
---
 gnu/bootloader/extlinux.scm | 20 +++++++++-----------
 gnu/system/vm.scm           |  3 ++-
 guix/scripts/system.scm     |  3 ++-
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm
index 0db5598..9b6e2c7 100644
--- a/gnu/bootloader/extlinux.scm
+++ b/gnu/bootloader/extlinux.scm
@@ -85,14 +85,6 @@ TIMEOUT ~a~%"
 ;;; Install procedures.
 ;;;
 
-(define dd
-  #~(lambda (bs count if of)
-      (zero? (system* "dd"
-                      (string-append "bs=" (number->string bs))
-                      (string-append "count=" (number->string count))
-                      (string-append "if=" if)
-                      (string-append "of=" of)))))
-
 (define (install-extlinux mbr)
   #~(lambda (bootloader device mount-point)
       (let ((extlinux (string-append bootloader "/sbin/extlinux"))
@@ -101,9 +93,15 @@ TIMEOUT ~a~%"
         (for-each (lambda (file)
                     (install-file file install-dir))
                   (find-files syslinux-dir "\\.c32$"))
-
-        (unless (and (zero? (system* extlinux "--install" install-dir))
-                     (#$dd 440 1 (string-append syslinux-dir "/" #$mbr) device))
+        (unless
+            (and (zero? (system* extlinux "--install" install-dir))
+                 (call-with-input-file (string-append syslinux-dir "/" #$mbr)
+                   (lambda (input)
+                     (let ((bv (get-bytevector-n input 440)))
+                       (call-with-output-file device
+                         (lambda (output)
+                           (put-bytevector output bv))
+                         #:binary #t)))))
           (error "failed to install SYSLINUX")))))
 
 (define install-extlinux-mbr
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index b68cce3..d754ac7 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -278,7 +278,8 @@ the image."
      #~(begin
          (use-modules (gnu build vm)
                       (guix build utils)
-                      (srfi srfi-26))
+                      (srfi srfi-26)
+                      (ice-9 binary-ports))
 
          (let ((inputs
                 '#$(append (list qemu parted e2fsprogs dosfstools)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 91d151d..e2ff426 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -676,7 +676,8 @@ and TARGET arguments."
     (gexp->file "bootloader-installer"
                 (with-imported-modules '((guix build utils))
                   #~(begin
-                      (use-modules (guix build utils))
+                      (use-modules (guix build utils)
+                                   (ice-9 binary-ports))
                       (#$installer #$bootloader #$device #$target))))))
 
 (define* (perform-action action os
-- 
2.7.4

[bug#29409] [PATCH] build: utils: Introduce dd.

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

> From 143057ead198a75e8a7a2bb1bb706402dcb7d4c3 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> Date: Fri, 1 Dec 2017 14:09:38 +0100
> Subject: [PATCH] bootloader: extlinux: Stop using dd binary.
>
> * gnu/bootloader/extlinux.scm (dd): Remove it,
> (install-extlinux): replace dd call by Guile I/O procedures.
> * gnu/system/vm.scm (qemu-image): Add (ice-9 binary-ports) to used-modules
> list to provide "get-bytevector-n" and "put-bytevector".
> * guix/scripts/system.scm (bootloader-installer-derivation): Ditto.

Perfect, thank you!

Ludo’.

[bug#29409] [PATCH] build: utils: Introduce dd.

[Mathieu Othacehe]

> Perfect, thank you!

Thanks, pushed !

Mathieu

[bug#29409] [PATCH] utils: Add target-arm32? procedure.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* guix/utils.scm (target-arm?): New exported procedure.
---
 guix/utils.scm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/guix/utils.scm b/guix/utils.scm
index c0ffed1..fed31f4 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2017 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -75,6 +76,7 @@
             %current-target-system
             package-name->name+version
             target-mingw?
+            target-arm32?
             version-compare
             version>?
             version>=?
@@ -467,6 +469,9 @@ a character other than '@'."
   (and target
        (string-suffix? "-mingw32" target)))
 
+(define (target-arm32?)
+  (string-prefix? "arm" (or (%current-target-system) (%current-system))))
+
 (define version-compare
   (let ((strverscmp
          (let ((sym (or (dynamic-func "strverscmp" (dynamic-link))
-- 
2.7.4

[bug#29409] [PATCH] system: vm: Do not add EFI partition on ARM system.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/system/vm.scm (qemu-img): Do not add EFI partition if we are targetting
  ARM.

UEFI support on u-boot is still experimental, so do not add EFI partition on
ARM for now.
---
 gnu/system/vm.scm | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index a5fe48e..b7a0e45 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -29,6 +29,7 @@
   #:use-module (guix monads)
   #:use-module (guix records)
   #:use-module (guix modules)
+  #:use-module (guix utils)
 
   #:use-module ((gnu build vm)
                 #:select (qemu-command))
@@ -312,26 +313,30 @@ the image."
                                              graphs)))
                                     (- disk-image-size
                                        (* 50 (expt 2 20)))))
-                  (partitions (list (partition
-                                     (size root-size)
-                                     (label #$file-system-label)
-                                     (uuid #$(and=> file-system-uuid
-                                                    uuid-bytevector))
-                                     (file-system #$file-system-type)
-                                     (flags '(boot))
-                                     (initializer initialize))
-                                    ;; Append a small EFI System Partition for
-                                    ;; use with UEFI bootloaders.
-                                    (partition
-                                     ;; The standalone grub image is about 10MiB, but
-                                     ;; leave some room for custom or multiple images.
-                                     (size (* 40 (expt 2 20)))
-                                     (label "GNU-ESP")             ;cosmetic only
-                                     ;; Use "vfat" here since this property is used
-                                     ;; when mounting. The actual FAT-ness is based
-                                     ;; on filesystem size (16 in this case).
-                                     (file-system "vfat")
-                                     (flags '(esp))))))
+                  (partitions
+                   (append
+                    (list (partition
+                           (size root-size)
+                           (label #$file-system-label)
+                           (uuid #$(and=> file-system-uuid
+                                          uuid-bytevector))
+                           (file-system #$file-system-type)
+                           (flags '(boot))
+                           (initializer initialize)))
+                    ;; Append a small EFI System Partition for use with UEFI
+                    ;; bootloaders if we are not targetting ARM.
+                    (if #$(target-arm32?)
+                        '()
+                        (list (partition
+                               ;; The standalone grub image is about 10MiB, but
+                               ;; leave some room for custom or multiple images.
+                               (size (* 40 (expt 2 20)))
+                               (label "GNU-ESP")             ;cosmetic only
+                               ;; Use "vfat" here since this property is used
+                               ;; when mounting. The actual FAT-ness is based
+                               ;; on filesystem size (16 in this case).
+                               (file-system "vfat")
+                               (flags '(esp))))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
                                    #:grub-efi #$grub-efi
-- 
2.7.4

[bug#29409] [PATCH] system: vm: Do not add EFI partition on ARM system.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/system/vm.scm (qemu-img): Do not add EFI partition if we are targetting
>   ARM.
>
> UEFI support on u-boot is still experimental, so do not add EFI partition on
> ARM for now.

[...]

> +                    ;; Append a small EFI System Partition for use with UEFI
> +                    ;; bootloaders if we are not targetting ARM.

+ “because UEFI support in U-Boot is experimental.”

> +                    (if #$(target-arm32?)
> +                        '()

Also can you add something like:

  ;; FIXME: ‘target-arm32?’ may be not operate on the right
  ;; system/target values.  Rewrite using ‘let-system’ when available.

OK with these changes, thank you!

And sorry for not completing ‘let-system’ in type.  I plan to resume
soonish but I figured I’d rather focus on the new release now.

Ludo’.

[bug#29409] [PATCH] system: vm: Do not add EFI partition on ARM system.

[Mathieu Othacehe]

> OK with these changes, thank you!

Thanks for reviewing i pushed this one and the previous one with the
changes you suggested.

> And sorry for not completing ‘let-system’ in type.  I plan to resume
> soonish but I figured I’d rather focus on the new release now.

It's fine, i know you cannot be on all fronts :)

Thanks,

Mathieu

[bug#29409] [PATCH] utils: Add target-arm32? procedure.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * guix/utils.scm (target-arm?): New exported procedure.
                             ^^
target-arm32?  :-)

LGTM, thanks!

Ludo'.

[bug#29409] [PATCH] build: vm: Use netdev qemu parameter.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/build/vm.scm (load-in-linux-vm): "-net nic" option is
deprecated, replace it with "-netdev" and "-device" options.

See https://wiki.qemu.org/Documentation/Networking.
---
 gnu/build/vm.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 20ee127..9e868f6 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -113,7 +113,8 @@ the #:references-graphs parameter of 'derivation'."
   (unless (zero?
            (apply system* qemu "-nographic" "-no-reboot"
                   "-m" (number->string memory-size)
-                  "-net" "nic,model=virtio"
+                  "-device" "virtio-net-pci,netdev=mynet"
+                  "-netdev" "user,id=mynet"
                   "-virtfs"
                   (string-append "local,id=store_dev,path="
                                  (%store-directory)
-- 
2.7.4

[bug#29409] [PATCH] build: vm: Use netdev qemu parameter.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

On Mon, Dec 04, 2017 at 05:52:04PM +0100, m.othacehe@gmail.com wrote:
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> 
> * gnu/build/vm.scm (load-in-linux-vm): "-net nic" option is
> deprecated, replace it with "-netdev" and "-device" options.
> 
> See https://wiki.qemu.org/Documentation/Networking.
> ---
>  gnu/build/vm.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
> index 20ee127..9e868f6 100644
> --- a/gnu/build/vm.scm
> +++ b/gnu/build/vm.scm
> @@ -113,7 +113,8 @@ the #:references-graphs parameter of 'derivation'."
>    (unless (zero?
>             (apply system* qemu "-nographic" "-no-reboot"
>                    "-m" (number->string memory-size)
> -                  "-net" "nic,model=virtio"
> +                  "-device" "virtio-net-pci,netdev=mynet"
> +                  "-netdev" "user,id=mynet"

Does this work if you run multiple instances at one time? Or does the
'id' parameter need to be unique?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#29409] [PATCH] build: vm: Use netdev qemu parameter.

[Mathieu Othacehe]

> Does this work if you run multiple instances at one time? Or does the
> 'id' parameter need to be unique?

Good remark, I had a doubt and tested it, you can spawn multiple qemu
machine with duplicated 'id' parameters (at least for drive and network).

Mathieu

[bug#29409] [PATCH] build: vm: Use qemu drive device parameter.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/build/vm.scm (load-in-linux-vm): As per network arguments, use device
parameter to define drive device.
---
 gnu/build/vm.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 9e868f6..96c57ab 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -126,10 +126,11 @@ the #:references-graphs parameter of 'derivation'."
                   "-initrd" initrd
                   "-append" (string-append "console=ttyS0 --load="
                                            builder)
+                  "-device" "virtio-blk,drive=hd"
                   (append
                    (if make-disk-image?
-                       `("-drive" ,(string-append "file=" output
-                                                  ",if=virtio"))
+                       `("-drive" ,(string-append "if=none,file=" output
+                                                  ",id=hd"))
                        '())
                    ;; Only enable kvm if we see /dev/kvm exists.
                    ;; This allows users without hardware virtualization to still
-- 
2.7.4

[bug#29409] [PATCH] build: vm: Use qemu drive device parameter.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/build/vm.scm (load-in-linux-vm): As per network arguments, use device
> parameter to define drive device.
> ---
>  gnu/build/vm.scm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
> index 9e868f6..96c57ab 100644
> --- a/gnu/build/vm.scm
> +++ b/gnu/build/vm.scm
> @@ -126,10 +126,11 @@ the #:references-graphs parameter of 'derivation'."
>                    "-initrd" initrd
>                    "-append" (string-append "console=ttyS0 --load="
>                                             builder)
> +                  "-device" "virtio-blk,drive=hd"
>                    (append
>                     (if make-disk-image?
> -                       `("-drive" ,(string-append "file=" output
> -                                                  ",if=virtio"))
> +                       `("-drive" ,(string-append "if=none,file=" output
> +                                                  ",id=hd"))

Is there a difference between ‘virtio’ and ‘virtio-blk’?

If confess I always have a hard time parsing QEMU’s command-line
options, but as long as it still works, it LGTM.  :-)

Thanks,
Ludo’.

[bug#29409] [PATCH] build: vm: Use qemu drive device parameter.

[Mathieu Othacehe]

Hey Ludo,

> Is there a difference between ‘virtio’ and ‘virtio-blk’?
>
> If confess I always have a hard time parsing QEMU’s command-line
> options, but as long as it still works, it LGTM.  :-)

Yes I had a hard time finding a command-line working both on intel and
arm systems.

if=virtio === -device
virtio-blk-pci,drive=DRIVE-ID,class=C,vectors=V,ioeventfd=IOEVENTFD

according to :

https://github.com/qemu/qemu/blob/master/docs/qdev-device-use.txt

I submitted an updated version of this patch in the same serie. It adds
"format=xxx" parameter to avoid this kind of warnings :

--8<---------------cut here---------------start------------->8---
WARNING: Image format was not specified for ... and probing guessed raw.
         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
         Specify the 'raw' format explicitly to remove the restrictions.
--8<---------------cut here---------------end--------------->8---

Thanks,

Mathieu

[bug#29409] [PATCH 0/4] ARM port.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

Hi,

Here are the four last patches required before adding BBB support.

Thanks,

Mathieu

Mathieu Othacehe (4):
  build: vm: Use netdev qemu parameter.
  build: vm: Use qemu drive device parameter.
  build: vm: Adapt qemu command to ARM.
  bootloader: Factorize write-file-on-device.

 gnu/bootloader.scm          | 21 ++++++++++++++++++++-
 gnu/bootloader/extlinux.scm |  9 ++-------
 gnu/build/vm.scm            | 28 ++++++++++++++++++++++------
 gnu/system/vm.scm           |  4 ++++
 4 files changed, 48 insertions(+), 14 deletions(-)

-- 
2.7.4

[bug#29409] [PATCH 1/4] build: vm: Use netdev qemu parameter.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/build/vm.scm (load-in-linux-vm): "-net nic" option is
deprecated, replace it with "-netdev" and "-device" options.

See https://wiki.qemu.org/Documentation/Networking.
---
 gnu/build/vm.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 20ee127..9e868f6 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -113,7 +113,8 @@ the #:references-graphs parameter of 'derivation'."
   (unless (zero?
            (apply system* qemu "-nographic" "-no-reboot"
                   "-m" (number->string memory-size)
-                  "-net" "nic,model=virtio"
+                  "-device" "virtio-net-pci,netdev=mynet"
+                  "-netdev" "user,id=mynet"
                   "-virtfs"
                   (string-append "local,id=store_dev,path="
                                  (%store-directory)
-- 
2.7.4

[bug#29409] [PATCH 1/4] build: vm: Use netdev qemu parameter.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/build/vm.scm (load-in-linux-vm): "-net nic" option is
> deprecated, replace it with "-netdev" and "-device" options.
>
> See https://wiki.qemu.org/Documentation/Networking.
> ---
>  gnu/build/vm.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
> index 20ee127..9e868f6 100644
> --- a/gnu/build/vm.scm
> +++ b/gnu/build/vm.scm
> @@ -113,7 +113,8 @@ the #:references-graphs parameter of 'derivation'."
>    (unless (zero?
>             (apply system* qemu "-nographic" "-no-reboot"
>                    "-m" (number->string memory-size)
> -                  "-net" "nic,model=virtio"
> +                  "-device" "virtio-net-pci,netdev=mynet"
> +                  "-netdev" "user,id=mynet"

IIUC the “user” part means that it does more than just update to the new
syntax: it also enable user-level networking support (built-in DHCP
server, etc.).  Is this correct?

If so, can we leave out “user”?

Thanks,
Ludo’.

[bug#29409] [PATCH 1/4] build: vm: Use netdev qemu parameter.

[Mathieu Othacehe]

Hi Ludo,

> IIUC the “user” part means that it does more than just update to the new
> syntax: it also enable user-level networking support (built-in DHCP
> server, etc.).  Is this correct?
>
> If so, can we leave out “user”?

Well it seems that you're right, "-net nic" is not functionnaly
equivalent to "-net user" (=== "-netdev user").

However, "-net nic" does not seem to be supported with -M virt on
qemu-system-arm :

--8<---------------cut here---------------start------------->8---
qemu-system-arm -M virt -net nic
Warning: requested NIC (anonymous, model unspecified) was not created
(not supported by this machine?)
--8<---------------cut here---------------end--------------->8---

trying to get a list of available devices (as explained in man page),
gives :

--8<---------------cut here---------------start------------->8---
qemu-system-arm -M virt -net nic,model=help
Warning: requested NIC (anonymous, model help) was not created (not
supported by this machine?)
--8<---------------cut here---------------end--------------->8---

So it seems that on ARM we will have to use "-netdev user", would it be a
problem to use it too on intel systems ?

Thanks,

Mathieu

[bug#29409] [PATCH 1/4] build: vm: Use netdev qemu parameter.

[Ludovic Courtès]

Hi Mathieu,

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> IIUC the “user” part means that it does more than just update to the new
>> syntax: it also enable user-level networking support (built-in DHCP
>> server, etc.).  Is this correct?
>>
>> If so, can we leave out “user”?
>
> Well it seems that you're right, "-net nic" is not functionnaly
> equivalent to "-net user" (=== "-netdev user").
>
> However, "-net nic" does not seem to be supported with -M virt on
> qemu-system-arm :
>
> qemu-system-arm -M virt -net nic
> Warning: requested NIC (anonymous, model unspecified) was not created
> (not supported by this machine?)
>
>
> trying to get a list of available devices (as explained in man page),
> gives :
>
> qemu-system-arm -M virt -net nic,model=help
> Warning: requested NIC (anonymous, model help) was not created (not
> supported by this machine?)
>
> So it seems that on ARM we will have to use "-netdev user", would it be a
> problem to use it too on intel systems ?

It’s not a problem per se, but it provides additional functionality
(QEMU’s built-in DHCP server and all that.)  It’s weird because it’s not
supposed to turn on emulation of a NIC, IIUC.

Maybe add it to the ARM-specific list of flags?

Ludo’.

[bug#29409] [PATCH 2/4] build: vm: Use qemu drive device parameter.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/build/vm.scm (load-in-linux-vm): As per network arguments, use device
parameter to define drive device.
---
 gnu/build/vm.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 9e868f6..0b1b9da 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -126,10 +126,12 @@ the #:references-graphs parameter of 'derivation'."
                   "-initrd" initrd
                   "-append" (string-append "console=ttyS0 --load="
                                            builder)
+                  "-device" "virtio-blk,drive=myhd"
                   (append
                    (if make-disk-image?
-                       `("-drive" ,(string-append "file=" output
-                                                  ",if=virtio"))
+                       `("-drive" ,(string-append "if=none,file=" output
+                                                  ",format=" disk-image-format
+                                                  ",id=myhd"))
                        '())
                    ;; Only enable kvm if we see /dev/kvm exists.
                    ;; This allows users without hardware virtualization to still
-- 
2.7.4

[bug#29409] [PATCH 2/4] build: vm: Use qemu drive device parameter.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/build/vm.scm (load-in-linux-vm): As per network arguments, use device
> parameter to define drive device.

LGTM!

[bug#29409] [PATCH 2/4] build: vm: Use qemu drive device parameter.

[Ludovic Courtès]

Nitpick: Please use “vm:” (and not “build:”) as the prefix in the
subject line.  I use “build:” for things like Makefile.am or
configure.ac changes.

Ludo’.

[bug#29409] [PATCH 3/4] build: vm: Adapt qemu command to ARM.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/build/vm.scm (load-in-linux-vm): New argument #:target-arm32.
Use it to adapt command for qemu-system-arm.  This implies to choose a
machine ("virt"), use the correct console port "ttyAMA0" and disable KVM use
that is buggy on some ARM boards (Odroid XU4 for example).
* gnu/system/vm.scm (expression->derivation-in-linux-vm): Pass
to load-in-linux-vm "#:target-arm32?" argument.
---
 gnu/build/vm.scm  | 19 ++++++++++++++++---
 gnu/system/vm.scm |  4 ++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 0b1b9da..0fbba62 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -77,6 +77,7 @@
                            linux initrd
                            make-disk-image?
                            single-file-output?
+                           target-arm32?
                            (disk-image-size (* 100 (expt 2 20)))
                            (disk-image-format "qcow2")
                            (references-graphs '()))
@@ -124,8 +125,10 @@ the #:references-graphs parameter of 'derivation'."
                                  ",security_model=none,mount_tag=xchg")
                   "-kernel" linux
                   "-initrd" initrd
-                  "-append" (string-append "console=ttyS0 --load="
-                                           builder)
+                  "-append"
+                  (if target-arm32?
+                      (string-append "console=ttyAMA0 --load=" builder)
+                      (string-append "console=ttyS0 --load=" builder))
                   "-device" "virtio-blk,drive=myhd"
                   (append
                    (if make-disk-image?
@@ -133,10 +136,20 @@ the #:references-graphs parameter of 'derivation'."
                                                   ",format=" disk-image-format
                                                   ",id=myhd"))
                        '())
+                   ;; On ARM, a machine has to be specified. Use
+                   ;; "virt" machine to avoid hardware limits imposed
+                   ;; by other machines.
+                   (if target-arm32?
+                       '("-M" "virt")
+                       '())
                    ;; Only enable kvm if we see /dev/kvm exists.
                    ;; This allows users without hardware virtualization to still
                    ;; use these commands.
-                   (if (file-exists? "/dev/kvm")
+                   ;;
+                   ;; KVM support is still buggy on some ARM32 boards. Do not
+                   ;; use it even if available.
+                   (if (and (file-exists? "/dev/kvm")
+                            (not target-arm32?))
                        '("-enable-kvm")
                        '()))))
     (error "qemu failed" qemu))
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index d754ac7..33c65de 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -175,6 +175,10 @@ made available under the /xchg CIFS share."
                                 #:memory-size #$memory-size
                                 #:make-disk-image? #$make-disk-image?
                                 #:single-file-output? #$single-file-output?
+                                ;; FIXME: ‘target-arm32?’ may be not operate
+                                ;; on the right system/target values.  Rewrite
+                                ;; using ‘let-system’ when available.
+                                #:target-arm32? #$(target-arm32?)
                                 #:disk-image-format #$disk-image-format
                                 #:disk-image-size size
                                 #:references-graphs graphs)))))
-- 
2.7.4

[bug#29409] [PATCH 3/4] build: vm: Adapt qemu command to ARM.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/build/vm.scm (load-in-linux-vm): New argument #:target-arm32.
> Use it to adapt command for qemu-system-arm.  This implies to choose a
> machine ("virt"), use the correct console port "ttyAMA0" and disable KVM use
> that is buggy on some ARM boards (Odroid XU4 for example).
> * gnu/system/vm.scm (expression->derivation-in-linux-vm): Pass
> to load-in-linux-vm "#:target-arm32?" argument.
> ---
>  gnu/build/vm.scm  | 19 ++++++++++++++++---
>  gnu/system/vm.scm |  4 ++++
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
> index 0b1b9da..0fbba62 100644
> --- a/gnu/build/vm.scm
> +++ b/gnu/build/vm.scm
> @@ -77,6 +77,7 @@
>                             linux initrd
>                             make-disk-image?
>                             single-file-output?
> +                           target-arm32?

Do we really need this new parameter and could we use the
‘target-arm32?’ procedure that you added directly?

> +                  (if target-arm32?
> +                      (string-append "console=ttyAMA0 --load=" builder)
> +                      (string-append "console=ttyS0 --load=" builder))

Could you add a short comment explaining this?

>                    "-device" "virtio-blk,drive=myhd"
>                    (append
>                     (if make-disk-image?
> @@ -133,10 +136,20 @@ the #:references-graphs parameter of 'derivation'."
>                                                    ",format=" disk-image-format
>                                                    ",id=myhd"))
>                         '())
> +                   ;; On ARM, a machine has to be specified. Use
> +                   ;; "virt" machine to avoid hardware limits imposed
> +                   ;; by other machines.
> +                   (if target-arm32?
> +                       '("-M" "virt")
> +                       '())
>                     ;; Only enable kvm if we see /dev/kvm exists.
>                     ;; This allows users without hardware virtualization to still
>                     ;; use these commands.
> -                   (if (file-exists? "/dev/kvm")
> +                   ;;
> +                   ;; KVM support is still buggy on some ARM32 boards. Do not
> +                   ;; use it even if available.
> +                   (if (and (file-exists? "/dev/kvm")
> +                            (not target-arm32?))
>                         '("-enable-kvm")
>                         '()))))

For clarity, it might be best to collect all the platform-specific
options separately, like:

  (define arch-specific-flags
    `(,@(if target-arm32? '("-M" "virt") '())
      ,@(if (and (file-exists? "/dev/kvm") …) …)
      …))

WDYT?

Ludo’.

[bug#29409] [PATCH 3/4] build: vm: Adapt qemu command to ARM.

[Mathieu Othacehe]

> Do we really need this new parameter and could we use the
> ‘target-arm32?’ procedure that you added directly?

The problem is "target-arm32" is defined in (guix utils) which is not
included at build time. Because it is supposed to be superseded by
"let-system" in a near future, I tought it would be better than moving
"target-arm32" in (guix build utils), am I right ?

> Could you add a short comment explaining this?

Sure.

> For clarity, it might be best to collect all the platform-specific
> options separately, like:
>
>   (define arch-specific-flags
>     `(,@(if target-arm32? '("-M" "virt") '())
>       ,@(if (and (file-exists? "/dev/kvm") …) …)
>       …))
>
> WDYT?

Seems better, I'll propose an updated patch.

Thanks,

Mathieu

[bug#29409] [PATCH 3/4] build: vm: Adapt qemu command to ARM.

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> Do we really need this new parameter and could we use the
>> ‘target-arm32?’ procedure that you added directly?
>
> The problem is "target-arm32" is defined in (guix utils) which is not
> included at build time. Because it is supposed to be superseded by
> "let-system" in a near future, I tought it would be better than moving
> "target-arm32" in (guix build utils), am I right ?

Oh you’re right (I hadn’t realized this was on the build side.)  You can
keep it this way.

Thanks,
Ludo’.

[bug#29409] [PATCH 4/4] bootloader: Factorize write-file-on-device.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/bootloader/extlinux.scm (install-extlinux): Factorize bootloader
  writing in a new procedure ...
* gnu/bootloader.scm (write-file-on-device): ... defined and exported here.
---
 gnu/bootloader.scm          | 21 ++++++++++++++++++++-
 gnu/bootloader/extlinux.scm |  9 ++-------
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/gnu/bootloader.scm b/gnu/bootloader.scm
index 736f119..a715ba1 100644
--- a/gnu/bootloader.scm
+++ b/gnu/bootloader.scm
@@ -22,6 +22,7 @@
   #:use-module (guix discovery)
   #:use-module (guix records)
   #:use-module (guix ui)
+  #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
   #:export (menu-entry
             menu-entry?
@@ -55,7 +56,8 @@
             bootloader-configuration-additional-configuration
 
             %bootloaders
-            lookup-bootloader-by-name))
+            lookup-bootloader-by-name
+            write-file-on-device))
 
 \f
 ;;;
@@ -163,3 +165,20 @@
               (eq? name (bootloader-name bootloader)))
             (force %bootloaders))
       (leave (G_ "~a: no such bootloader~%") name)))
+
+\f
+;;;
+;;; Writing utils.
+;;;
+
+(define write-file-on-device
+  #~(lambda (file size device offset)
+      ;; Write SIZE bytes from FILE to DEVICE starting at OFFSET.
+      (call-with-input-file file
+        (lambda (input)
+          (let ((bv (get-bytevector-n input size)))
+            (call-with-output-file device
+              (lambda (output)
+                (seek output offset SEEK_SET)
+                (put-bytevector output bv))
+              #:binary #t))))))
diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm
index 9b6e2c7..304fb98 100644
--- a/gnu/bootloader/extlinux.scm
+++ b/gnu/bootloader/extlinux.scm
@@ -95,13 +95,8 @@ TIMEOUT ~a~%"
                   (find-files syslinux-dir "\\.c32$"))
         (unless
             (and (zero? (system* extlinux "--install" install-dir))
-                 (call-with-input-file (string-append syslinux-dir "/" #$mbr)
-                   (lambda (input)
-                     (let ((bv (get-bytevector-n input 440)))
-                       (call-with-output-file device
-                         (lambda (output)
-                           (put-bytevector output bv))
-                         #:binary #t)))))
+                 (#$write-file-on-device
+                  (string-append syslinux-dir "/" #$mbr) 440 device 0))
           (error "failed to install SYSLINUX")))))
 
 (define install-extlinux-mbr
-- 
2.7.4

[bug#29409] [PATCH 4/4] bootloader: Factorize write-file-on-device.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/bootloader/extlinux.scm (install-extlinux): Factorize bootloader
>   writing in a new procedure ...
> * gnu/bootloader.scm (write-file-on-device): ... defined and exported here.

LGTM!

> +(define write-file-on-device
> +  #~(lambda (file size device offset)
> +      ;; Write SIZE bytes from FILE to DEVICE starting at OFFSET.
> +      (call-with-input-file file
> +        (lambda (input)
> +          (let ((bv (get-bytevector-n input size)))
> +            (call-with-output-file device
> +              (lambda (output)
> +                (seek output offset SEEK_SET)
> +                (put-bytevector output bv))
> +              #:binary #t))))))

Eventually we might want to move it to (gnu build vm) or to create a new
(gnu build bootloader) module with build-side code for bootloaders.

Thanks!

Ludo’.

[bug#29409] [PATCH 4/4] bootloader: Factorize write-file-on-device.

[Mathieu Othacehe]

> Eventually we might want to move it to (gnu build vm) or to create a new
> (gnu build bootloader) module with build-side code for bootloaders.

I'll go for (gnu build bootloader) then :)

Thanks,

Mathieu

[bug#29409] [PATCH] system: Add BeagleBone Black installer.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/bootloader/u-boot.scm (u-boot-beaglebone-black-bootloader): New exported
  bootloader.
* gnu/system/boards/beaglebone-black.scm: New file returning bbb installer.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add new file.
---
 gnu/bootloader/u-boot.scm              | 18 +++++++++++++++++-
 gnu/local.mk                           |  4 +++-
 gnu/system/boards/beaglebone-black.scm | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 gnu/system/boards/beaglebone-black.scm

diff --git a/gnu/bootloader/u-boot.scm b/gnu/bootloader/u-boot.scm
index 963b0d7..2169680 100644
--- a/gnu/bootloader/u-boot.scm
+++ b/gnu/bootloader/u-boot.scm
@@ -26,13 +26,23 @@
   #:use-module (guix monads)
   #:use-module (guix records)
   #:use-module (guix utils)
-  #:export (u-boot-bootloader))
+  #:export (u-boot-bootloader
+            u-boot-beaglebone-black-bootloader))
 
 (define install-u-boot
   #~(lambda (bootloader device mount-point)
       (if bootloader
         (error "Failed to install U-Boot"))))
 
+(define install-beaglebone-black-u-boot
+  #~(lambda (bootloader device mount-point)
+      (let ((mlo (string-append bootloader "/libexec/MLO"))
+            (u-boot (string-append bootloader "/libexec/u-boot.img")))
+        (#$write-file-on-device mlo (* 256 512)
+                                device (* 256 512))
+        (#$write-file-on-device u-boot (* 1024 512)
+                                device (* 768 512)))))
+
 \f
 
 ;;;
@@ -45,3 +55,9 @@
    (name 'u-boot)
    (package #f)
    (installer install-u-boot)))
+
+(define u-boot-beaglebone-black-bootloader
+  (bootloader
+   (inherit u-boot-bootloader)
+   (package u-boot-beagle-bone-black)
+   (installer install-beaglebone-black-u-boot)))
diff --git a/gnu/local.mk b/gnu/local.mk
index 434bbb0..53fc04e 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -487,7 +487,9 @@ GNU_SYSTEM_MODULES =				\
   %D%/system/shadow.scm				\
   %D%/system/uuid.scm				\
   %D%/system/vm.scm				\
-						\
+                                                \
+  %D%/system/boards/beaglebone-black.scm	\
+                                                \
   %D%/build/activation.scm			\
   %D%/build/cross-toolchain.scm			\
   %D%/build/file-systems.scm			\
diff --git a/gnu/system/boards/beaglebone-black.scm b/gnu/system/boards/beaglebone-black.scm
new file mode 100644
index 0000000..19ef412
--- /dev/null
+++ b/gnu/system/boards/beaglebone-black.scm
@@ -0,0 +1,33 @@
+(define-module (gnu system boards beaglebone-black)
+  #:use-module (gnu)
+  #:use-module (gnu system install)
+  #:use-module (gnu bootloader u-boot)
+  #:use-module (gnu packages bootloaders)
+  #:use-module (gnu packages linux)
+  #:export (beaglebone-black-installation-os))
+
+(define beaglebone-black-installation-os
+  (operating-system
+    (inherit installation-os)
+    (bootloader (bootloader-configuration
+                 (bootloader u-boot-beaglebone-black-bootloader)
+                 (target "/dev/sda")))
+    (kernel linux-libre)
+    (initrd (lambda (fs . rest)
+              (apply base-initrd fs
+                     ;; This module is required to mount the sd card.
+                     #:extra-modules (list "omap_hsmmc")
+                     rest)))
+    (services (append
+               ;; mingetty does not work on serial lines.
+               ;; Use agetty with board-specific serial parameters.
+               (list (agetty-service
+                      (agetty-configuration
+                       (extra-options '("-L"))
+                       (baud-rate "115200")
+                       (term "vt100")
+                       (tty "ttyO0"))))
+               (operating-system-user-services installation-os)))))
+
+;; Return it here so 'guix system' can consume it directly.
+beaglebone-black-installation-os
-- 
2.7.4

[bug#29409] [PATCH] system: Add BeagleBone Black installer.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/bootloader/u-boot.scm (u-boot-beaglebone-black-bootloader): New exported
>   bootloader.
> * gnu/system/boards/beaglebone-black.scm: New file returning bbb installer.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add new file.

[...]

> +(define install-beaglebone-black-u-boot
> +  #~(lambda (bootloader device mount-point)
> +      (let ((mlo (string-append bootloader "/libexec/MLO"))
> +            (u-boot (string-append bootloader "/libexec/u-boot.img")))

Could you add a comment explaining what’s specific to the BBB, perhaps
with a link to relevant documentation?

> +++ b/gnu/system/boards/beaglebone-black.scm
> @@ -0,0 +1,33 @@
> +(define-module (gnu system boards beaglebone-black)
> +  #:use-module (gnu)
> +  #:use-module (gnu system install)
> +  #:use-module (gnu bootloader u-boot)
> +  #:use-module (gnu packages bootloaders)
> +  #:use-module (gnu packages linux)
> +  #:export (beaglebone-black-installation-os))
> +
> +(define beaglebone-black-installation-os
> +  (operating-system
> +    (inherit installation-os)

I’d be tempted to just add it to (gnu system install) and then we’d do:

  guix system disk-image -e '(@ (gnu system install) beaglebone-black-installation-os)'

WDYT?

Of course we first need to add support for -e but that should be fine.
;-)

> +beaglebone-black-installation-os

Now that sounds really awesome.  :-)

So you’ve successfully installed GuixSD on BBB using this image?

Is the BBB powerful enough to run ‘guix pull’?  (I have an Olimex A20
with 1G of RAM that just dies.)

For small devices, it would also make sense to (cross-)build a
ready-to-use GuixSD image and just write it to the device.

Thoughts?

Anyway, kudos on that one!  I think you owe us a blog post with pictures
of the BBB running GuixSD.  :-)

Ludo’.

[bug#29409] [PATCH] system: Add BeagleBone Black installer.

[ng0]

[-- Attachment #1: Type: text/plain, Size: 2966 bytes --]

Ludovic Courtès transcribed 1.9K bytes:
> m.othacehe@gmail.com skribis:
> 
> > From: Mathieu Othacehe <m.othacehe@gmail.com>
> >
> > * gnu/bootloader/u-boot.scm (u-boot-beaglebone-black-bootloader): New exported
> >   bootloader.
> > * gnu/system/boards/beaglebone-black.scm: New file returning bbb installer.
> > * gnu/local.mk (GNU_SYSTEM_MODULES): Add new file.
> 
> [...]
> 
> > +(define install-beaglebone-black-u-boot
> > +  #~(lambda (bootloader device mount-point)
> > +      (let ((mlo (string-append bootloader "/libexec/MLO"))
> > +            (u-boot (string-append bootloader "/libexec/u-boot.img")))
> 
> Could you add a comment explaining what’s specific to the BBB, perhaps
> with a link to relevant documentation?
> 
> > +++ b/gnu/system/boards/beaglebone-black.scm
> > @@ -0,0 +1,33 @@
> > +(define-module (gnu system boards beaglebone-black)
> > +  #:use-module (gnu)
> > +  #:use-module (gnu system install)
> > +  #:use-module (gnu bootloader u-boot)
> > +  #:use-module (gnu packages bootloaders)
> > +  #:use-module (gnu packages linux)
> > +  #:export (beaglebone-black-installation-os))
> > +
> > +(define beaglebone-black-installation-os
> > +  (operating-system
> > +    (inherit installation-os)
> 
> I’d be tempted to just add it to (gnu system install) and then we’d do:
> 
>   guix system disk-image -e '(@ (gnu system install) beaglebone-black-installation-os)'
> 
> WDYT?
> 
> Of course we first need to add support for -e but that should be fine.
> ;-)

For portability (extending into more templates and more installers etc)
this seems like a nice idea.

For example: you want to have a minimal XFCE image with just one
definition, and you want to maintain deriviates of this minimal
XFCE image in the same file. Or more in the spirit of this thread:
You have one system install definition, but for hardware compability
reasons you need to change some tiny pieces in it and add 3 variants
for various hardware architecture. This would allow me to reduce

 infotropique/system/{core,$arch-core,$arch-$wm,…}
or something like infottropique/system/$arch/$wm
to a more clean library overview.

Just as a potential reference sneak-peak without
much context ;)

> > +beaglebone-black-installation-os
> 
> Now that sounds really awesome.  :-)
> 
> So you’ve successfully installed GuixSD on BBB using this image?
> 
> Is the BBB powerful enough to run ‘guix pull’?  (I have an Olimex A20
> with 1G of RAM that just dies.)
> 
> For small devices, it would also make sense to (cross-)build a
> ready-to-use GuixSD image and just write it to the device.
> 
> Thoughts?
> 
> Anyway, kudos on that one!  I think you owe us a blog post with pictures
> of the BBB running GuixSD.  :-)
> 
> Ludo’.

Definitely very exciting!

-- 
GnuPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588
GnuPG: https://c.n0.is/ng0_pubkeys/tree/keys
  WWW: https://n0.is

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#29409] [PATCH] system: Add BeagleBone Black installer.

[Mathieu Othacehe]

> I’d be tempted to just add it to (gnu system install) and then we’d do:
>
>   guix system disk-image -e '(@ (gnu system install) beaglebone-black-installation-os)'
>
> WDYT?

It seems better !

> Now that sounds really awesome.  :-)

Thanks :)

> So you’ve successfully installed GuixSD on BBB using this image?

I succeeded in booting the installer, I didn't try to install GuixSD on
eMMC for now.

> Is the BBB powerful enough to run ‘guix pull’?  (I have an Olimex A20
> with 1G of RAM that just dies.)

I really doubt that it will succeed :(

> For small devices, it would also make sense to (cross-)build a
> ready-to-use GuixSD image and just write it to the device.
>
> Thoughts?

Sure I see two viable use-cases for now :

* Build a functional system (mpd or prosody server for example) and
  write it on an external sdcard.

* Cross-build a functional system and write it on an sdcard too.
  It could be interesting because building a system image on a quite
  powerful ARMv7 board (I bought an Odroid XU4 for this purpose) takes
  between 3-4 hours.

Time is mostly spent in qemu machine copying files and in
guix-registering.

I don't think installing/guix pulling/guix system reconfiguring/ as we
do it on intel systems is really viable on those tiny ARM boards.

> Anyway, kudos on that one!  I think you owe us a blog post with pictures
> of the BBB running GuixSD.  :-)

Thanks again and yes, it's the top task on my todo-list :)

Mathieu

[bug#29409] [PATCH v2 1/4] vm: Adapt qemu command to ARM.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/build/vm.scm (load-in-linux-vm): New argument #:target-arm32.
Use it to adapt command for qemu-system-arm.  This implies to choose a
machine ("virt"), use the correct console port "ttyAMA0", disable KVM use
that is buggy on some ARM boards (Odroid XU4 for example) and use user mode
network stack instead of NIC. Gather all those options in a new variable
"arch-specific-flags".
* gnu/system/vm.scm (expression->derivation-in-linux-vm): Pass
to load-in-linux-vm "#:target-arm32?" argument.
---
 gnu/build/vm.scm  | 34 +++++++++++++++++++++++++++-------
 gnu/system/vm.scm |  4 ++++
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index ed84463..8f8ca60 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -77,6 +77,7 @@
                            linux initrd
                            make-disk-image?
                            single-file-output?
+                           target-arm32?
                            (disk-image-size (* 100 (expt 2 20)))
                            (disk-image-format "qcow2")
                            (references-graphs '()))
@@ -91,6 +92,31 @@ access it via /dev/hda.
 
 REFERENCES-GRAPHS can specify a list of reference-graph files as produced by
 the #:references-graphs parameter of 'derivation'."
+
+  (define arch-specific-flags
+    `(;; On ARM, a machine has to be specified. Use "virt" machine to avoid
+      ;; hardware limits imposed by other machines.
+      ,@(if target-arm32? '("-M" "virt") '())
+
+      ;; Only enable kvm if we see /dev/kvm exists.  This allows users without
+      ;; hardware virtualization to still use these commands.  KVM support is
+      ;; still buggy on some ARM32 boards. Do not use it even if available.
+      ,@(if (and (file-exists? "/dev/kvm")
+                 (not target-arm32?))
+            '("-enable-kvm")
+            '())
+      "-append"
+      ;; The serial port name differs between emulated architectures/machines.
+      ,@(if target-arm32?
+            `(,(string-append "console=ttyAMA0 --load=" builder))
+            `(,(string-append "console=ttyS0 --load=" builder)))
+      ;; NIC is not supported on ARM "virt" machine, so use a user mode
+      ;; network stack instead.
+      ,@(if target-arm32?
+            '("-device" "virtio-net-pci,netdev=mynet"
+              "-netdev" "user,id=mynet")
+            '("-net" "nic,model=virtio"))))
+
   (when make-disk-image?
     (format #t "creating ~a image of ~,2f MiB...~%"
             disk-image-format (/ disk-image-size (expt 2 20)))
@@ -113,7 +139,6 @@ the #:references-graphs parameter of 'derivation'."
   (unless (zero?
            (apply system* qemu "-nographic" "-no-reboot"
                   "-m" (number->string memory-size)
-                  "-net" "nic,model=virtio"
                   "-virtfs"
                   (string-append "local,id=store_dev,path="
                                  (%store-directory)
@@ -132,12 +157,7 @@ the #:references-graphs parameter of 'derivation'."
                                                   ",format=" disk-image-format
                                                   ",id=myhd"))
                        '())
-                   ;; Only enable kvm if we see /dev/kvm exists.
-                   ;; This allows users without hardware virtualization to still
-                   ;; use these commands.
-                   (if (file-exists? "/dev/kvm")
-                       '("-enable-kvm")
-                       '()))))
+                   arch-specific-flags)))
     (error "qemu failed" qemu))
 
   ;; When MAKE-DISK-IMAGE? is true, the image is in OUTPUT already.
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index d754ac7..b376337 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -175,6 +175,10 @@ made available under the /xchg CIFS share."
                                 #:memory-size #$memory-size
                                 #:make-disk-image? #$make-disk-image?
                                 #:single-file-output? #$single-file-output?
+                                ;; FIXME: ‘target-arm32?’ may not operate on
+                                ;; the right system/target values.  Rewrite
+                                ;; using ‘let-system’ when available.
+                                #:target-arm32? #$(target-arm32?)
                                 #:disk-image-format #$disk-image-format
                                 #:disk-image-size size
                                 #:references-graphs graphs)))))
-- 
2.7.4

[bug#29409] [PATCH v2 2/4] bootloader: Factorize write-file-on-device.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/bootloader/extlinux.scm (install-extlinux): Factorize bootloader
  writing in a new procedure ...
* gnu/bootloader.scm (write-file-on-device): ... defined and exported here.
---
 gnu/bootloader/extlinux.scm | 10 +++-------
 gnu/build/bootloader.scm    | 37 +++++++++++++++++++++++++++++++++++++
 gnu/local.mk                |  1 +
 gnu/system/vm.scm           |  6 ++++--
 guix/scripts/system.scm     |  6 ++++--
 5 files changed, 49 insertions(+), 11 deletions(-)
 create mode 100644 gnu/build/bootloader.scm

diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm
index 9b6e2c7..f7820a3 100644
--- a/gnu/bootloader/extlinux.scm
+++ b/gnu/bootloader/extlinux.scm
@@ -20,6 +20,7 @@
 (define-module (gnu bootloader extlinux)
   #:use-module (gnu bootloader)
   #:use-module (gnu system)
+  #:use-module (gnu build bootloader)
   #:use-module (gnu packages bootloaders)
   #:use-module (guix gexp)
   #:use-module (guix monads)
@@ -95,13 +96,8 @@ TIMEOUT ~a~%"
                   (find-files syslinux-dir "\\.c32$"))
         (unless
             (and (zero? (system* extlinux "--install" install-dir))
-                 (call-with-input-file (string-append syslinux-dir "/" #$mbr)
-                   (lambda (input)
-                     (let ((bv (get-bytevector-n input 440)))
-                       (call-with-output-file device
-                         (lambda (output)
-                           (put-bytevector output bv))
-                         #:binary #t)))))
+                 (write-file-on-device
+                  (string-append syslinux-dir "/" #$mbr) 440 device 0))
           (error "failed to install SYSLINUX")))))
 
 (define install-extlinux-mbr
diff --git a/gnu/build/bootloader.scm b/gnu/build/bootloader.scm
new file mode 100644
index 0000000..d00674d
--- /dev/null
+++ b/gnu/build/bootloader.scm
@@ -0,0 +1,37 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu build bootloader)
+  #:use-module (ice-9 binary-ports)
+  #:export (write-file-on-device))
+
+\f
+;;;
+;;; Writing utils.
+;;;
+
+(define (write-file-on-device file size device offset)
+  "Write SIZE bytes from FILE to DEVICE starting at OFFSET."
+  (call-with-input-file file
+    (lambda (input)
+      (let ((bv (get-bytevector-n input size)))
+        (call-with-output-file device
+          (lambda (output)
+            (seek output offset SEEK_SET)
+            (put-bytevector output bv))
+          #:binary #t)))))
diff --git a/gnu/local.mk b/gnu/local.mk
index 4682975..7a55efc 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -489,6 +489,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/system/vm.scm				\
 						\
   %D%/build/activation.scm			\
+  %D%/build/bootloader.scm			\
   %D%/build/cross-toolchain.scm			\
   %D%/build/file-systems.scm			\
   %D%/build/install.scm				\
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index b376337..6102d46 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -277,10 +277,12 @@ register INPUTS in the store database of the image so that Guix can be used in
 the image."
   (expression->derivation-in-linux-vm
    name
-   (with-imported-modules (source-module-closure '((gnu build vm)
+   (with-imported-modules (source-module-closure '((gnu build bootloader)
+                                                   (gnu build vm)
                                                    (guix build utils)))
      #~(begin
-         (use-modules (gnu build vm)
+         (use-modules (gnu build bootloader)
+                      (gnu build vm)
                       (guix build utils)
                       (srfi srfi-26)
                       (ice-9 binary-ports))
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index d0eacc5..cbf7e6c 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -674,9 +674,11 @@ any, are available.  Raise an error if they're not."
 and TARGET arguments."
   (with-monad %store-monad
     (gexp->file "bootloader-installer"
-                (with-imported-modules '((guix build utils))
+                (with-imported-modules '((gnu build bootloader)
+                                         (guix build utils))
                   #~(begin
-                      (use-modules (guix build utils)
+                      (use-modules (gnu build bootloader)
+                                   (guix build utils)
                                    (ice-9 binary-ports))
                       (#$installer #$bootloader #$device #$target))))))
 
-- 
2.7.4

[bug#29409] [PATCH v2 2/4] bootloader: Factorize write-file-on-device.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/bootloader/extlinux.scm (install-extlinux): Factorize bootloader
>   writing in a new procedure ...
> * gnu/bootloader.scm (write-file-on-device): ... defined and exported here.
> ---
>  gnu/bootloader/extlinux.scm | 10 +++-------
>  gnu/build/bootloader.scm    | 37 +++++++++++++++++++++++++++++++++++++
>  gnu/local.mk                |  1 +
>  gnu/system/vm.scm           |  6 ++++--
>  guix/scripts/system.scm     |  6 ++++--
>  5 files changed, 49 insertions(+), 11 deletions(-)
>  create mode 100644 gnu/build/bootloader.scm

Please mention all the files in the commit log.

Otherwise LGTM, thanks!

Ludo’.

[bug#29409] [PATCH v2 2/4] bootloader: Factorize write-file-on-device.

[Mathieu Othacehe]

> Please mention all the files in the commit log.
>
> Otherwise LGTM, thanks!

Thanks, fixed and pushed.

Mathieu

[bug#29409] [PATCH v2 3/4] scripts: system: Add --expression option.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* guix/scripts/system.scm (show-help): Add expression option.
(%options): Ditto.
(process-action): Read operating-system from expression or file.
* Makefile.am (release): Use expression instead of file to produce
disk-images.
* doc/guix.texi (Building the Installation Image): Adapt disk-image command to
  use an expression.
(Invoking guix system): Introduce the expression option.
---
 Makefile.am             |  2 +-
 doc/guix.texi           | 10 +++++++++-
 guix/scripts/system.scm | 24 +++++++++++++++++++-----
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 85b9ab3..dff1e5e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -648,7 +648,7 @@ release: dist
 	    guix system disk-image							\
 	    --file-system-type=iso9660							\
             --system=$$system								\
-	    gnu/system/install.scm` ;							\
+	    -e "(@ (gnu system install) installation-os)"` ;							\
 	  if [ ! -f "$$image" ] ; then							\
 	    echo "failed to produced GuixSD installation image for $$system" >&2 ;	\
 	    exit 1 ;									\
diff --git a/doc/guix.texi b/doc/guix.texi
index 92ac45b..9bdfdf7 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -8356,7 +8356,7 @@ The installation image described above was built using the @command{guix
 system} command, specifically:
 
 @example
-guix system disk-image gnu/system/install.scm
+guix system disk-image -e "(@@ (gnu system install) installation-os)"
 @end example
 
 Have a look at @file{gnu/system/install.scm} in the source tree,
@@ -18744,6 +18744,14 @@ Build Options}).  In addition, @var{options} can contain one of the
 following:
 
 @table @option
+@item --expression=@var{expr}
+@itemx -e @var{expr}
+Consider the operating-system @var{expr} evaluates to.
+This is an alternative to specifying a file which evaluates to an
+operating-system.
+This is used to generate the GuixSD installer @pxref{Building the
+Installation Image}).
+
 @item --system=@var{system}
 @itemx -s @var{system}
 Attempt to build for @var{system} instead of the host system type.
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index cbf7e6c..f710db9 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -858,6 +858,9 @@ Some ACTIONS support additional ARGS.\n"))
   (display (G_ "
   -d, --derivation       return the derivation of the given system"))
   (display (G_ "
+  -e, --expression=EXPR  consider the operating-system EXPR evaluates to
+                         instead of reading FILE, when applicable"))
+  (display (G_ "
       --on-error=STRATEGY
                          apply STRATEGY when an error occurs while reading FILE"))
   (display (G_ "
@@ -895,6 +898,9 @@ Some ACTIONS support additional ARGS.\n"))
          (option '(#\V "version") #f #f
                  (lambda args
                    (show-version-and-exit "guix system")))
+         (option '(#\e "expression") #t #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'expression arg result)))
          (option '(#\d "derivation") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'derivations-only? #t result)))
@@ -964,11 +970,19 @@ resulting from command-line parsing."
   (let* ((file        (match args
                         (() #f)
                         ((x . _) x)))
+         (expr        (assoc-ref opts 'expression))
          (system      (assoc-ref opts 'system))
-         (os          (if file
-                          (load* file %user-module
-                                 #:on-error (assoc-ref opts 'on-error))
-                          (leave (G_ "no configuration file specified~%"))))
+         (os          (cond
+                       ((and expr file)
+                        (leave
+                         (G_ "both file and expression cannot be specified~%")))
+                       (expr
+                        (read/eval expr))
+                       (file
+                        (load* file %user-module
+                                    #:on-error (assoc-ref opts 'on-error)))
+                       (else
+                        (leave (G_ "no configuration specified~%")))))
 
          (dry?        (assoc-ref opts 'dry-run?))
          (bootloader? (assoc-ref opts 'install-bootloader?))
@@ -1093,7 +1107,7 @@ argument list and OPTS is the option alist."
 
       (case action
         ((build container vm vm-image disk-image reconfigure)
-         (unless (= count 1)
+         (unless (or (= count 1) (= count 0))
            (fail)))
         ((init)
          (unless (= count 2)
-- 
2.7.4

[bug#29409] [PATCH v2 3/4] scripts: system: Add --expression option.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * guix/scripts/system.scm (show-help): Add expression option.
> (%options): Ditto.
> (process-action): Read operating-system from expression or file.
> * Makefile.am (release): Use expression instead of file to produce
> disk-images.
> * doc/guix.texi (Building the Installation Image): Adapt disk-image command to
>   use an expression.
> (Invoking guix system): Introduce the expression option.
> ---
>  Makefile.am             |  2 +-
>  doc/guix.texi           | 10 +++++++++-
>  guix/scripts/system.scm | 24 +++++++++++++++++++-----
>  3 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 85b9ab3..dff1e5e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -648,7 +648,7 @@ release: dist
>  	    guix system disk-image							\
>  	    --file-system-type=iso9660							\
>              --system=$$system								\
> -	    gnu/system/install.scm` ;							\
> +	    -e "(@ (gnu system install) installation-os)"` ;							\

[...]

>  @example
> -guix system disk-image gnu/system/install.scm
> +guix system disk-image -e "(@@ (gnu system install) installation-os)"
>  @end example

I’d suggest leaving these two things unchanged because it’s still more
convenient to type the file name.

>    (display (G_ "
> +  -e, --expression=EXPR  consider the operating-system EXPR evaluates to
                                                  ^
No hyphen.

[...]

>        (case action
>          ((build container vm vm-image disk-image reconfigure)
> -         (unless (= count 1)
> +         (unless (or (= count 1) (= count 0))
>             (fail)))

What happens if we run “guix system vm”?  Do we still get a proper
error?

The rest LGTM.

Thanks,
Ludo’.

[bug#29409] [PATCH v2 3/4] scripts: system: Add --expression option.

[Mathieu Othacehe]

> What happens if we run “guix system vm”?  Do we still get a proper
> error?

Nope, it's a mistake, would this be better ?

@@ -1079,7 +1093,8 @@ argument list and OPTS is the option alist."
     ;; Extract the plain arguments from OPTS.
     (let* ((args   (reverse (filter-map (match-pair 'argument) opts)))
            (count  (length args))
-           (action (assoc-ref opts 'action)))
+           (action (assoc-ref opts 'action))
+           (expr   (assoc-ref opts 'expression)))
       (define (fail)
         (leave (G_ "wrong number of arguments for action '~a'~%")
                action))
@@ -1093,7 +1108,8 @@ argument list and OPTS is the option alist."
 
       (case action
         ((build container vm vm-image disk-image reconfigure)
-         (unless (= count 1)
+         (unless (or (= count 1)
+                     (and expr (= count 0)))

Thanks,

Mathieu

[bug#29409] [PATCH v2 3/4] scripts: system: Add --expression option.

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> What happens if we run “guix system vm”?  Do we still get a proper
>> error?
>
> Nope, it's a mistake, would this be better ?
>
> @@ -1079,7 +1093,8 @@ argument list and OPTS is the option alist."
>      ;; Extract the plain arguments from OPTS.
>      (let* ((args   (reverse (filter-map (match-pair 'argument) opts)))
>             (count  (length args))
> -           (action (assoc-ref opts 'action)))
> +           (action (assoc-ref opts 'action))
> +           (expr   (assoc-ref opts 'expression)))
>        (define (fail)
>          (leave (G_ "wrong number of arguments for action '~a'~%")
>                 action))
> @@ -1093,7 +1108,8 @@ argument list and OPTS is the option alist."
>  
>        (case action
>          ((build container vm vm-image disk-image reconfigure)
> -         (unless (= count 1)
> +         (unless (or (= count 1)
> +                     (and expr (= count 0)))

It looks better, yes.  Thanks!

Ludo’.

[bug#29409] [PATCH v2 3/4] scripts: system: Add --expression option.

[Mathieu Othacehe]

> It looks better, yes.  Thanks!

I pushed the two remaining patches !

Thanks a lot for reviewing :)

Mathieu

[bug#29409] [PATCH v2 4/4] system: Add BeagleBone Black installer.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/bootloader/u-boot.scm (u-boot-beaglebone-black-bootloader): New exported
  bootloader.
* gnu/system/install.scm (beaglebone-black-installation-os): New exported variable.
---
 gnu/bootloader/u-boot.scm | 25 ++++++++++++++++++++++++-
 gnu/system/install.scm    | 28 +++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/gnu/bootloader/u-boot.scm b/gnu/bootloader/u-boot.scm
index 963b0d7..397eb81 100644
--- a/gnu/bootloader/u-boot.scm
+++ b/gnu/bootloader/u-boot.scm
@@ -21,18 +21,35 @@
   #:use-module (gnu bootloader extlinux)
   #:use-module (gnu bootloader)
   #:use-module (gnu system)
+  #:use-module (gnu build bootloader)
   #:use-module (gnu packages bootloaders)
   #:use-module (guix gexp)
   #:use-module (guix monads)
   #:use-module (guix records)
   #:use-module (guix utils)
-  #:export (u-boot-bootloader))
+  #:export (u-boot-bootloader
+            u-boot-beaglebone-black-bootloader))
 
 (define install-u-boot
   #~(lambda (bootloader device mount-point)
       (if bootloader
         (error "Failed to install U-Boot"))))
 
+(define install-beaglebone-black-u-boot
+  ;; http://wiki.beyondlogic.org/index.php?title=BeagleBoneBlack_Upgrading_uBoot
+  ;; This first stage bootloader called MLO (U-Boot SPL) is expected at
+  ;; 0x20000 by BBB ROM code. The second stage bootloader will be loaded by
+  ;; the MLO and is expected at 0x60000.  Write both first stage ("MLO") and
+  ;; second stage ("u-boot.img") images, read in BOOTLOADER directory, to the
+  ;; specified DEVICE.
+  #~(lambda (bootloader device mount-point)
+      (let ((mlo (string-append bootloader "/libexec/MLO"))
+            (u-boot (string-append bootloader "/libexec/u-boot.img")))
+        (write-file-on-device mlo (* 256 512)
+                              device (* 256 512))
+        (write-file-on-device u-boot (* 1024 512)
+                              device (* 768 512)))))
+
 \f
 
 ;;;
@@ -45,3 +62,9 @@
    (name 'u-boot)
    (package #f)
    (installer install-u-boot)))
+
+(define u-boot-beaglebone-black-bootloader
+  (bootloader
+   (inherit u-boot-bootloader)
+   (package u-boot-beagle-bone-black)
+   (installer install-beaglebone-black-u-boot)))
diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index c2f73f7..d98becd 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -22,6 +22,7 @@
 
 (define-module (gnu system install)
   #:use-module (gnu)
+  #:use-module (gnu bootloader u-boot)
   #:use-module (guix gexp)
   #:use-module (guix store)
   #:use-module (guix monads)
@@ -42,7 +43,8 @@
   #:use-module (gnu packages nvi)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-26)
-  #:export (installation-os))
+  #:export (installation-os
+            beaglebone-black-installation-os))
 
 ;;; Commentary:
 ;;;
@@ -372,7 +374,27 @@ You have been warned.  Thanks for being so brave.\x1b[0m
                      nvi                          ;:wq!
                      %base-packages))))
 
-;; Return it here so 'guix system' can consume it directly.
-installation-os
+(define beaglebone-black-installation-os
+  (operating-system
+    (inherit installation-os)
+    (bootloader (bootloader-configuration
+                 (bootloader u-boot-beaglebone-black-bootloader)
+                 (target "/dev/sda")))
+    (kernel linux-libre)
+    (initrd (lambda (fs . rest)
+              (apply base-initrd fs
+                     ;; This module is required to mount the sd card.
+                     #:extra-modules (list "omap_hsmmc")
+                     rest)))
+    (services (append
+               ;; mingetty does not work on serial lines.
+               ;; Use agetty with board-specific serial parameters.
+               (list (agetty-service
+                      (agetty-configuration
+                       (extra-options '("-L"))
+                       (baud-rate "115200")
+                       (term "vt100")
+                       (tty "ttyO0"))))
+               (operating-system-user-services installation-os)))))
 
 ;;; install.scm ends here
-- 
2.7.4

[bug#29409] [PATCH v2 4/4] system: Add BeagleBone Black installer.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/bootloader/u-boot.scm (u-boot-beaglebone-black-bootloader): New exported
>   bootloader.
> * gnu/system/install.scm (beaglebone-black-installation-os): New exported variable.

[...]

> -;; Return it here so 'guix system' can consume it directly.
> -installation-os

I’d leave these two lines, but otherwise LGTM.

Thank you!

Ludo’.

[bug#29409] [PATCH v2 1/4] vm: Adapt qemu command to ARM.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/build/vm.scm (load-in-linux-vm): New argument #:target-arm32.
> Use it to adapt command for qemu-system-arm.  This implies to choose a
> machine ("virt"), use the correct console port "ttyAMA0", disable KVM use
> that is buggy on some ARM boards (Odroid XU4 for example) and use user mode
> network stack instead of NIC. Gather all those options in a new variable
> "arch-specific-flags".
> * gnu/system/vm.scm (expression->derivation-in-linux-vm): Pass
> to load-in-linux-vm "#:target-arm32?" argument.

Perfect, thank you!

[bug#29409] [PATCH v2 1/4] vm: Adapt qemu command to ARM.

[Mathieu Othacehe]

> Perfect, thank you!

Thanks, pushed !

Mathieu

[bug#29409] [PATCH] system: examples: Add a template for BeagleBone Black.

[m.othacehe]

From: Mathieu Othacehe <m.othacehe@gmail.com>

* gnu/system/examples/beaglebone-black.tmpl: New file.
* Makefile.am (EXAMPLES): Add it.
* gnu/system/install.scm (/etc/configuration-files): Add it.
---
 Makefile.am                               |  1 +
 gnu/system/examples/beaglebone-black.tmpl | 59 +++++++++++++++++++++++++++++++
 gnu/system/install.scm                    |  2 ++
 3 files changed, 62 insertions(+)
 create mode 100644 gnu/system/examples/beaglebone-black.tmpl

diff --git a/Makefile.am b/Makefile.am
index 85b9ab3..7d7b1f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -253,6 +253,7 @@ AUX_FILES =						\
 # Templates, examples.
 EXAMPLES =					\
   gnu/system/examples/bare-bones.tmpl		\
+  gnu/system/examples/beaglebone-black.tmpl	\
   gnu/system/examples/desktop.tmpl		\
   gnu/system/examples/lightweight-desktop.tmpl	\
   gnu/system/examples/vm-image.tmpl
diff --git a/gnu/system/examples/beaglebone-black.tmpl b/gnu/system/examples/beaglebone-black.tmpl
new file mode 100644
index 0000000..8dc0c91
--- /dev/null
+++ b/gnu/system/examples/beaglebone-black.tmpl
@@ -0,0 +1,59 @@
+;; This is an operating system configuration template
+;; for a "bare bones" setup on BeagleBone Black board.
+
+(use-modules (gnu) (gnu bootloader u-boot))
+(use-service-modules networking ssh)
+(use-package-modules bootloaders screen ssh)
+
+(operating-system
+  (host-name "komputilo")
+  (timezone "Europe/Berlin")
+  (locale "en_US.utf8")
+
+  ;; Assuming /dev/mmcblk1 is the eMMC, and "my-root" is
+  ;; the label of the target root file system.
+  (bootloader (bootloader-configuration
+               (bootloader u-boot-beaglebone-black-bootloader)
+               (target "/dev/mmcblk1")))
+  (file-systems (cons (file-system
+                        (device "my-root")
+                        (title 'label)
+                        (mount-point "/")
+                        (type "ext4"))
+                      %base-file-systems))
+
+  ;; This is where user accounts are specified.  The "root"
+  ;; account is implicit, and is initially created with the
+  ;; empty password.
+  (users (cons (user-account
+                (name "alice")
+                (comment "Bob's sister")
+                (group "users")
+
+                ;; Adding the account to the "wheel" group
+                ;; makes it a sudoer.  Adding it to "audio"
+                ;; and "video" allows the user to play sound
+                ;; and access the webcam.
+                (supplementary-groups '("wheel"
+                                        "audio" "video"))
+                (home-directory "/home/alice"))
+               %base-user-accounts))
+
+  ;; Globally-installed packages.
+  (packages (cons* screen openssh %base-packages))
+
+  ;; Add services to the baseline: a DHCP client and
+  ;; an SSH server.
+  (services (cons* (dhcp-client-service)
+                   (service openssh-service-type
+                            (openssh-configuration
+                             (port-number 2222)))
+                   ;; mingetty does not work on serial lines.
+                   ;; Use agetty with board-specific serial parameters.
+                   (agetty-service
+                    (agetty-configuration
+                     (extra-options '("-L"))
+                     (baud-rate "115200")
+                     (term "vt100")
+                     (tty "ttyO0")))
+                   %base-services)))
diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index 8864415..1cc3db1 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -156,9 +156,11 @@ the user's target storage device rather than on the RAM disk."
                                                 (string-append #$output "/"
                                                                target)))
                                    '(#$(file "bare-bones.tmpl")
+                                     #$(file "beaglebone-black.tmpl")
                                      #$(file "desktop.tmpl")
                                      #$(file "lightweight-desktop.tmpl"))
                                    '("bare-bones.scm"
+                                     "beaglebone-black.scm"
                                      "desktop.scm"
                                      "lightweight-desktop.scm"))
                          #t))))
-- 
2.7.4

[bug#29409] [PATCH] system: examples: Add a template for BeagleBone Black.

[Ludovic Courtès]

m.othacehe@gmail.com skribis:

> From: Mathieu Othacehe <m.othacehe@gmail.com>
>
> * gnu/system/examples/beaglebone-black.tmpl: New file.
> * Makefile.am (EXAMPLES): Add it.
> * gnu/system/install.scm (/etc/configuration-files): Add it.

[...]

> +  ;; Add services to the baseline: a DHCP client and
> +  ;; an SSH server.
> +  (services (cons* (dhcp-client-service)
> +                   (service openssh-service-type
> +                            (openssh-configuration
> +                             (port-number 2222)))

I think you can remove (openssh-configuration …) altogether.

Otherwise LGTM, thank you!

Ludo’.

[bug#29409] [PATCH] system: examples: Add a template for BeagleBone Black.

[Mathieu Othacehe]

Hey Ludo,

> Otherwise LGTM, thank you!

That was fast, done and pushed, thanks !

Mathieu

[bug#29409] Remove hugetlb control group on ARM32.

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]


Hi Guix,

Mounting %control-groups fails on ARM32 platform because we build a
kernel without LPAE support which implies hugetlb control group cannot
be supported.

Like Debian we could have an ARMMP and ARMMP-LPAE kernel but the problem
would still exists for ARMMP.

I'm not sure what do about, a workaround could be the ugly hack
attached, WDYT ?

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-file-systems-Do-not-mount-hugetlb-cgroup-filesystem-.patch --]
[-- Type: text/x-patch, Size: 1606 bytes --]

From 2172a1897a9729b65767bb58883247a3c604109f Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Wed, 20 Dec 2017 20:02:33 +0100
Subject: [PATCH] file-systems: Do not mount hugetlb cgroup filesystem on
 arm32.

On ARM32 without LPAE support, hugetlb control group is not supported.

* gnu/system/file-systems.scm (%control-groups): Remove hugetlb from control
  groups on arm32 platforms.
---
 gnu/system/file-systems.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 27734e892..cbe6a725a 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -21,6 +21,7 @@
   #:use-module (rnrs bytevectors)
   #:use-module (srfi srfi-1)
   #:use-module (guix records)
+  #:use-module (guix utils)
   #:use-module (gnu system uuid)
   #:re-export (uuid                               ;backward compatibility
                string->uuid
@@ -278,8 +279,9 @@ TARGET in the other system."
                    ;; This must be mounted after, and unmounted before the
                    ;; parent directory.
                    (dependencies (list parent))))
-               '("cpuset" "cpu" "cpuacct" "memory" "devices" "freezer"
-                 "blkio" "perf_event" "hugetlb")))))
+               `("cpuset" "cpu" "cpuacct" "memory" "devices" "freezer"
+                 "blkio" "perf_event"
+                 ,@(if (not (target-arm32?)) '("hugetlb") '()))))))
 
 (define %elogind-file-systems
   ;; We don't use systemd, but these file systems are needed for elogind,
-- 
2.15.0

[bug#29409] Remove hugetlb control group on ARM32.

[Tobias Geerinckx-Rice]

Mathieu! (and others),

Mathieu Othacehe wrote on 20/12/17 at 20:15:
> Mounting %control-groups fails on ARM32 platform because we build a
> kernel without LPAE support which implies hugetlb control group cannot
> be supported.

What happens when the mount fails? At first glance, %control-groups
aren't needed-for-boot. Unfortunately, I'm not able to test it now: it's
all disgustingly hypothetical from here on.

> Like Debian we could have an ARMMP and ARMMP-LPAE kernel but the problem
> would still exists for ARMMP.
> 
> I'm not sure what do about, a workaround could be the ugly hack
> attached, WDYT ?

So to me your patch implies that mounting the hugetlb cgroup is entirely
optional, and that no other (known) services will actually break if it's
not mounted. %control-groups are mounted as part of %base-file-systems,
about which the manual:

  -- Scheme Variable: %base-file-systems
      These are essential file systems that are required on normal
      systems, such as %PSEUDO-TERMINAL-FILE-SYSTEM and %IMMUTABLE-STORE
      (see below.)  Operating system declarations should always contain
      at least these.

In practice, %base-file-systems depends on more mounts than it probably
should, since some of them aren't essential and some can't even exist on
some platforms. That keeps things simple, and isn't a problem *if*
mounting optional file systems like %control-groups simply logs the
error and continues normally with the next mount. A bit dirty, perhaps,
but there are more places in file-systems.scm that feel that way to me.

If the patch only serves to suppress such an error, I don't think it's
worth copying details of our kernel configurations around.

If it fixes a broken boot, there's something... off about the way we
handle mounts.

...or maybe I am. Thoughts?

> From 2172a1897a9729b65767bb58883247a3c604109f Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> Date: Wed, 20 Dec 2017 20:02:33 +0100
> Subject: [PATCH] file-systems: Do not mount hugetlb cgroup filesystem on
>  arm32.
> 
> On ARM32 without LPAE support, hugetlb control group is not supported.

Whenever I write an ‘extra’ line like this one, I usually realise it's
really a comment in disguise, more at home in the code than forgotten in
the commit log. I think that's the case here.

Kind regards,

T G-R

[bug#29409] Remove hugetlb control group on ARM32.

[Ludovic Courtès]

Hello!

Tobias Geerinckx-Rice <me@tobias.gr> skribis:

> Mathieu Othacehe wrote on 20/12/17 at 20:15:
>> Mounting %control-groups fails on ARM32 platform because we build a
>> kernel without LPAE support which implies hugetlb control group cannot
>> be supported.
>
> What happens when the mount fails? At first glance, %control-groups
> aren't needed-for-boot. Unfortunately, I'm not able to test it now: it's
> all disgustingly hypothetical from here on.

If ‘mount’ fails with EOPNOTSUPP, we could handle that specifically,
perhaps with an (optional? #t) flag on the file system or something.

>> Like Debian we could have an ARMMP and ARMMP-LPAE kernel but the problem
>> would still exists for ARMMP.
>> 
>> I'm not sure what do about, a workaround could be the ugly hack
>> attached, WDYT ?
>
> So to me your patch implies that mounting the hugetlb cgroup is entirely
> optional, and that no other (known) services will actually break if it's
> not mounted. %control-groups are mounted as part of %base-file-systems,
> about which the manual:
>
>   -- Scheme Variable: %base-file-systems
>       These are essential file systems that are required on normal
>       systems, such as %PSEUDO-TERMINAL-FILE-SYSTEM and %IMMUTABLE-STORE
>       (see below.)  Operating system declarations should always contain
>       at least these.
>
> In practice, %base-file-systems depends on more mounts than it probably
> should, since some of them aren't essential and some can't even exist on
> some platforms. That keeps things simple, and isn't a problem *if*
> mounting optional file systems like %control-groups simply logs the
> error and continues normally with the next mount. A bit dirty, perhaps,
> but there are more places in file-systems.scm that feel that way to me.
>
> If the patch only serves to suppress such an error, I don't think it's
> worth copying details of our kernel configurations around.
>
> If it fixes a broken boot, there's something... off about the way we
> handle mounts.
>
> ...or maybe I am. Thoughts?

‘%base-file-systems’ is loosely defined and it’s a moving target.

Probably ‘%control-groups’ should be part of ‘%elogind-file-systems’ and
not ‘%base-file-systems’, because I think it’s elogind that needs them.

That would solve bare-bones-style configuration that doesn’t use
elogind.  It wouldn’t help for all things desktop, though.

WDYT?

> --- a/gnu/system/file-systems.scm
> +++ b/gnu/system/file-systems.scm
> @@ -21,6 +21,7 @@
>    #:use-module (rnrs bytevectors)
>    #:use-module (srfi srfi-1)
>    #:use-module (guix records)
> +  #:use-module (guix utils)

This is not great because (gnu system file-systems) is also used on the
“build side”, but (guix utils) would pull in (guix config), which is
configuration-dependent (it contains data that can vary from system to
system):

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(guix modules)
scheme@(guile-user)> (source-module-closure '((guix utils)))
$3 = ((guix utils) (guix config) (guix memoization) (guix profiling) (guix build utils) (guix build syscalls))
scheme@(guile-user)> (source-module-closure '((gnu system file-systems)))
$4 = ((gnu system file-systems) (guix records) (gnu system uuid))
--8<---------------cut here---------------end--------------->8---

Thanks,
Ludo’.

[bug#29409] Remove hugetlb control group on ARM32.

[Danny Milosavljevic]

Hi Mathieu,

On Wed, 20 Dec 2017 20:15:07 +0100
Mathieu Othacehe <m.othacehe@gmail.com> wrote:

> I'm not sure what do about, a workaround could be the ugly hack
> attached, WDYT ?

I researched cgroups a bit and it seems that /sys/fs/cgroup/hugetlb is for configuration of hugetlb and that cgroups themselves don't require hugetlb - and neither does elogind.

There could be any number of reasons hugetlb doesn't work (custom kernel config etc).

We could use (file-exists? "/proc/sys/vm/nr_hugepages") in order to test for hugetlb support.  The proc filesystem is already mounted at this point.

Alternatively, it would be possible to use (needed-for-boot? #f) for the hugetlb cgroup file-system.  In that case, maybe failure isn't so bad.  I didn't test that, however.

If we want to make this ARM-specific we can do that too, but I think it's more general than that.

[bug#29409] Remove hugetlb control group on ARM32.

[Ludovic Courtès]

Hi,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Wed, 20 Dec 2017 20:15:07 +0100
> Mathieu Othacehe <m.othacehe@gmail.com> wrote:
>
>> I'm not sure what do about, a workaround could be the ugly hack
>> attached, WDYT ?
>
> I researched cgroups a bit and it seems that /sys/fs/cgroup/hugetlb is for configuration of hugetlb and that cgroups themselves don't require hugetlb - and neither does elogind.

Indeed.  So actually we can probably remove it altogether.

We could simply do that and keep the definition around for when we need
it.

Thoughts?

> We could use (file-exists? "/proc/sys/vm/nr_hugepages") in order to test for hugetlb support.  The proc filesystem is already mounted at this point.

Currently we can’t easily introduce special conditions for file system
mounts.

> Alternatively, it would be possible to use (needed-for-boot? #f) for the hugetlb cgroup file-system.  In that case, maybe failure isn't so bad.  I didn't test that, however.

‘need-for-boot?’ tells whether the file system should be mounted from
the initrd or after PID 1 has been started, so it doesn’t help in this
case (it’s already #f I think.)

Ludo’.

[bug#29409] Remove hugetlb control group on ARM32.

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]


Hi Tobias, Danny and Ludo,

Thanks for your answers!

> Indeed.  So actually we can probably remove it altogether.
>
> We could simply do that and keep the definition around for when we need
> it.
>
> Thoughts?

Seems ok to me.

I also proposed a patch to move %control-groups from %base-file-systems
to %elogind-file-systems.

WDYT ?

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-file-systems-Do-not-mount-hugetlb-cgroup-filesystem.patch --]
[-- Type: text/x-patch, Size: 1153 bytes --]

From a531af25b3cffeb70e5681ef70ced564378f2db7 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Fri, 22 Dec 2017 08:42:29 +0100
Subject: [PATCH 1/2] file-systems: Do not mount hugetlb cgroup filesystem.

On ARM32 without LPAE support, hugetlb control group is not supported.
As it is not needed by elogind, remove it for all platforms.

* gnu/system/file-systems.scm (%control-groups): Remove hugetlb from control
  groups platforms.
---
 gnu/system/file-systems.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 27734e892..9de465167 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -279,7 +279,7 @@ TARGET in the other system."
                    ;; parent directory.
                    (dependencies (list parent))))
                '("cpuset" "cpu" "cpuacct" "memory" "devices" "freezer"
-                 "blkio" "perf_event" "hugetlb")))))
+                 "blkio" "perf_event")))))
 
 (define %elogind-file-systems
   ;; We don't use systemd, but these file systems are needed for elogind,
-- 
2.15.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-file-systems-Move-control-groups-from-base-file-syst.patch --]
[-- Type: text/x-patch, Size: 3512 bytes --]

From 4a28602843a661989c687086af009481dab05eed Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Fri, 22 Dec 2017 08:43:24 +0100
Subject: [PATCH 2/2] file-systems: Move %control-groups from
 %base-file-systems to %elogind-file-systems.

* gnu/system/file-systems.scm (%base-file-systems): Move %control-groups from
here, to ...
(%elogind-file-systems): ... here.
---
 gnu/system/file-systems.scm | 63 +++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 9de465167..7f5afb00f 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -284,41 +284,42 @@ TARGET in the other system."
 (define %elogind-file-systems
   ;; We don't use systemd, but these file systems are needed for elogind,
   ;; which was extracted from systemd.
-  (list (file-system
-          (device "none")
-          (mount-point "/run/systemd")
-          (type "tmpfs")
-          (check? #f)
-          (flags '(no-suid no-dev no-exec))
-          (options "mode=0755")
-          (create-mount-point? #t))
-        (file-system
-          (device "none")
-          (mount-point "/run/user")
-          (type "tmpfs")
-          (check? #f)
-          (flags '(no-suid no-dev no-exec))
-          (options "mode=0755")
-          (create-mount-point? #t))
-        ;; Elogind uses cgroups to organize processes, allowing it to map PIDs
-        ;; to sessions.  Elogind's cgroup hierarchy isn't associated with any
-        ;; resource controller ("subsystem").
-        (file-system
-          (device "cgroup")
-          (mount-point "/sys/fs/cgroup/elogind")
-          (type "cgroup")
-          (check? #f)
-          (options "none,name=elogind")
-          (create-mount-point? #t)
-          (dependencies (list (car %control-groups))))))
+  (append
+   (list (file-system
+           (device "none")
+           (mount-point "/run/systemd")
+           (type "tmpfs")
+           (check? #f)
+           (flags '(no-suid no-dev no-exec))
+           (options "mode=0755")
+           (create-mount-point? #t))
+         (file-system
+           (device "none")
+           (mount-point "/run/user")
+           (type "tmpfs")
+           (check? #f)
+           (flags '(no-suid no-dev no-exec))
+           (options "mode=0755")
+           (create-mount-point? #t))
+         ;; Elogind uses cgroups to organize processes, allowing it to map PIDs
+         ;; to sessions.  Elogind's cgroup hierarchy isn't associated with any
+         ;; resource controller ("subsystem").
+         (file-system
+           (device "cgroup")
+           (mount-point "/sys/fs/cgroup/elogind")
+           (type "cgroup")
+           (check? #f)
+           (options "none,name=elogind")
+           (create-mount-point? #t)
+           (dependencies (list (car %control-groups)))))
+   %control-groups))
 
 (define %base-file-systems
   ;; List of basic file systems to be mounted.  Note that /proc and /sys are
   ;; currently mounted by the initrd.
-  (append (list %pseudo-terminal-file-system
-                %shared-memory-file-system
-                %immutable-store)
-          %control-groups))
+  (list %pseudo-terminal-file-system
+        %shared-memory-file-system
+        %immutable-store))
 
 ;; File systems for Linux containers differ from %base-file-systems in that
 ;; they impose additional restrictions such as no-exec or need different
-- 
2.15.0

[bug#29409] Remove hugetlb control group on ARM32.

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

> From a531af25b3cffeb70e5681ef70ced564378f2db7 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> Date: Fri, 22 Dec 2017 08:42:29 +0100
> Subject: [PATCH 1/2] file-systems: Do not mount hugetlb cgroup filesystem.
>
> On ARM32 without LPAE support, hugetlb control group is not supported.
> As it is not needed by elogind, remove it for all platforms.
>
> * gnu/system/file-systems.scm (%control-groups): Remove hugetlb from control
>   groups platforms.

LGTM, but please make sure that a gnu/system/examples/desktop.tmpl or
similar still works in a VM.

> From 4a28602843a661989c687086af009481dab05eed Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> Date: Fri, 22 Dec 2017 08:43:24 +0100
> Subject: [PATCH 2/2] file-systems: Move %control-groups from
>  %base-file-systems to %elogind-file-systems.
>
> * gnu/system/file-systems.scm (%base-file-systems): Move %control-groups from
> here, to ...
> (%elogind-file-systems): ... here.

LGTM, but please double-check that “make check-system TESTS=basic” and
bare-bones.tmpl or install.scm (which do not use elogind) still work as
expected.

Thanks!

Ludo’.

[bug#29409] Remove hugetlb control group on ARM32.

[Mathieu Othacehe]

> LGTM, but please double-check that “make check-system TESTS=basic” and
> bare-bones.tmpl or install.scm (which do not use elogind) still work as
> expected.

I ran the tests you suggested, everything seems ok so I pushed both
patches.

Thanks,

Mathieu