[bug#27705] [PATCH 0/4] Make both EFI and non-EFI systems boot our disk images.

[bug#27705] [PATCH 0/4] Make both EFI and non-EFI systems boot our disk images.

[Danny Milosavljevic]

This is an alternative implementation of bug# 27695 without extra Guix-side
bootloader.  It just extends grub-efi with a few files.

I've also added all the patches needed for it to actually work into the same
patchset.

I've tested it again with disk-image, disk-image -t iso9660, qemu with UEFI
BIOS and without UEFI BIOS.  Furthermore, I tested the ISO-9660 image both
as CD and as HDD (in qemu).  It all worked.

Danny Milosavljevic (4):
  gnu: grub-efi: Add mtools input.
  build: Allow mounting of entire disks.
  gnu: grub-efi: Add grub.
  install: Use grub-efi.

 gnu/build/file-systems.scm   | 13 ++++++++-----
 gnu/packages/bootloaders.scm | 25 +++++++++++++++++++++++++
 gnu/system/install.scm       |  3 ++-
 3 files changed, 35 insertions(+), 6 deletions(-)

[bug#27705] [PATCH 1/4] gnu: grub-efi: Add mtools input.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm: Add (gnu packages mtools).
(grub-efi)[inputs]: Add mtools.
[arguments]: Add phase "use-absolute-mtools-path".
---
 gnu/packages/bootloaders.scm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 9ae617528..b6833dff6 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -37,6 +37,7 @@
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages man)
+  #:use-module (gnu packages mtools)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages python)
@@ -149,6 +150,7 @@ menu to select one of the installed operating systems.")
     (synopsis "GRand Unified Boot loader (UEFI version)")
     (inputs
      `(("efibootmgr" ,efibootmgr)
+       ("mtools", mtools)
        ,@(package-inputs grub)))
     (arguments
      `(;; TODO: Tests need a UEFI firmware for qemu. There is one at
@@ -166,7 +168,19 @@ menu to select one of the installed operating systems.")
                      (("efibootmgr")
                       (string-append (assoc-ref inputs "efibootmgr")
                                      "/sbin/efibootmgr")))
-                   #t)))))))))
+                   #t))
+               (add-after 'patch-stuff 'use-absolute-mtools-path
+                 (lambda* (#:key inputs #:allow-other-keys)
+                   (let ((mtools (assoc-ref inputs "mtools")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mformat\"")
+                        (string-append "\"" mtools
+                                       "/bin/mformat\"")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mcopy\"")
+                        (string-append "\"" mtools
+                                       "/bin/mcopy\"")))
+                     #t))))))))))
 
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))

[bug#27705] [PATCH 2/4] build: Allow mounting of entire disks.

[Danny Milosavljevic]

* gnu/build/file-systems.scm (disk-partitions): Also return entire drives.
---
 gnu/build/file-systems.scm | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index b6930497d..462ed9b7f 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -377,11 +377,14 @@ not valid header was found."
     (string-ref str (- (string-length str) 1)))
 
   (define (partition? name major minor)
-    ;; Select device names that end in a digit, like libblkid's 'probe_all'
-    ;; function does.  Checking for "/sys/dev/block/MAJOR:MINOR/partition"
-    ;; doesn't work for partitions coming from mapped devices.
-    (and (char-set-contains? char-set:digit (last-character name))
-         (> major 2)))                      ;ignore RAM disks and floppy disks
+    ;; grub-mkrescue does some funny things for EFI support which
+    ;; makes it a lot more difficult than one would expect to support
+    ;; booting an ISO-9660 image from an USB flash drive.
+    ;; For example there's a buggy (too small) hidden partition in it
+    ;; which Linux rightfully refuses to mount.
+    ;; In any case, partition tables are supposed to be optional so
+    ;; here we allow checking entire disks for file systems, too.
+    (> major 2))                      ;ignore RAM disks and floppy disks
 
   (call-with-input-file "/proc/partitions"
     (lambda (port)

[bug#27705] [PATCH 3/4] gnu: grub-efi: Add grub.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm (grub-efi)[native-inputs]: Add grub.
[arguments]: Add phase "install-non-efi".
---
 gnu/packages/bootloaders.scm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index b6833dff6..f18402e21 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -152,6 +152,9 @@ menu to select one of the installed operating systems.")
      `(("efibootmgr" ,efibootmgr)
        ("mtools", mtools)
        ,@(package-inputs grub)))
+    (native-inputs
+     `(("grub" ,grub)
+       ,@(package-native-inputs grub)))
     (arguments
      `(;; TODO: Tests need a UEFI firmware for qemu. There is one at
        ;; https://github.com/tianocore/edk2/tree/master/OvmfPkg .
@@ -180,7 +183,15 @@ menu to select one of the installed operating systems.")
                        (("\"mcopy\"")
                         (string-append "\"" mtools
                                        "/bin/mcopy\"")))
-                     #t))))))))))
+                     #t)))
+               (add-after 'install 'install-non-efi
+                 (lambda* (#:key inputs outputs #:allow-other-keys)
+                   ;; grub-mkresuce can use it for making a hybrid image.
+                   (copy-recursively (string-append (assoc-ref inputs "grub")
+                                                    "/lib/grub/i386-pc")
+                                     (string-append (assoc-ref outputs "out")
+                                                    "/lib/grub/i386-pc"))
+                   #t)))))))))
 
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))

[bug#27705] [PATCH 3/4] gnu: grub-efi: Add grub.

[Danny Milosavljevic]

> +               (add-after 'install 'install-non-efi
> +                 (lambda* (#:key inputs outputs #:allow-other-keys)
> +                   ;; grub-mkresuce can use it for making a hybrid image.
> +                   (copy-recursively (string-append (assoc-ref inputs "grub")
> +                                                    "/lib/grub/i386-pc")
> +                                     (string-append (assoc-ref outputs "out")
> +                                                    "/lib/grub/i386-pc"))
> +                   #t)))))))))

Hmm, what happens on non-x86 architectures that nonetheless use grub-efi (there are apparently such).

[bug#27705] [PATCH 3/4] gnu: grub-efi: Add grub.

[Marius Bakke]

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

Danny Milosavljevic <dannym@scratchpost.org> writes:

> * gnu/packages/bootloaders.scm (grub-efi)[native-inputs]: Add grub.
> [arguments]: Add phase "install-non-efi".

[...]

> +    (native-inputs
> +     `(("grub" ,grub)
> +       ,@(package-native-inputs grub)))

Since we take machine-specific code from it, I think it must be a
regular input.

However...

>      (arguments
>       `(;; TODO: Tests need a UEFI firmware for qemu. There is one at
>         ;; https://github.com/tianocore/edk2/tree/master/OvmfPkg .
> @@ -180,7 +183,15 @@ menu to select one of the installed operating systems.")
>                         (("\"mcopy\"")
>                          (string-append "\"" mtools
>                                         "/bin/mcopy\"")))
> -                     #t))))))))))
> +                     #t)))
> +               (add-after 'install 'install-non-efi
> +                 (lambda* (#:key inputs outputs #:allow-other-keys)
> +                   ;; grub-mkresuce can use it for making a hybrid image.
> +                   (copy-recursively (string-append (assoc-ref inputs "grub")
> +                                                    "/lib/grub/i386-pc")
> +                                     (string-append (assoc-ref outputs "out")
> +                                                    "/lib/grub/i386-pc"))
> +                   #t)))))))))

I don't think we should do this in 'grub-efi'. Now users can not be
certain whether they have EFI boot or not, since grub will happily "fall
back" to i386-pc if it does not detect a UEFI system. It's a regression
of sorts.

However... The 'grub-hybrid' approach seems okay to me (although
building grub-efi again is unnecessary :)). If it works well, maybe we
could deprecate the other two GRUB packages.

FWIW Gentoo has taken the 'hybrid' approach for many years (building
once for each target platform and consolidating out/lib/grub). But it
would be good to know whether this configuration is supported upstream.

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

[bug#27705] [PATCH 3/4] gnu: grub-efi: Add grub.

[Danny Milosavljevic]

Hi Marius,

On Mon, 17 Jul 2017 23:26:06 +0200
Marius Bakke <mbakke@fastmail.com> wrote:

>However... The 'grub-hybrid' approach seems okay to me (although
>building grub-efi again is unnecessary :)). 

No, it's not unnecessary.  When you do not use this approach (for example when you create a new package which has "grub" and "grub-efi" as input) then the resulting executables (for example grub-efi's grub-mkrescue) will not find the platform support files - because the search path for multiple platforms is being hardcoded at build time.

Doesn't work:

+(define-public grub-hybrid
+  (package
+    (name "grub-hybrid")
+    (version (package-version grub-efi))
+    (inputs
+     `(("grub-efi" ,grub-efi) ; keep as first (preferred) entry
+       ("grub" ,grub)))
+    (build-system trivial-build-system)
+    (arguments
+     '(#:modules ((guix build union))
+       #:builder (begin
+                   (use-modules (ice-9 match)
+                                (guix build union))
+                   (match %build-inputs
+                     (((names . directories) ...)
+                      ;; Take "lib/grub/i386-pc" from grub, the remainder
+                      ;; from grub-efi.
+                      (union-build (assoc-ref %outputs "out")
+                                   directories))))))
+    (home-page "https://www.gnu.org/software/grub/")
+    (synopsis "GRand Unified Boot loader (Hybrid version)")
+    (description "This version of GRUB tries the @code{grub-efi} and
+then the traditional @code{grub}.")))
+

> FWIW Gentoo has taken the 'hybrid' approach for many years (building
> once for each target platform and consolidating out/lib/grub). But it
> would be good to know whether this configuration is supported upstream.

I'm using it for grub-mkrescue and, there, it's definitely supported in the sense of there's explicit case analysis in the source code for it and also big structures to enable this kind of using multiple platforms.

For other uses I don't know.

[bug#27705] [PATCH 4/4] install: Use grub-efi.

[Danny Milosavljevic]

* gnu/system/install.scm (installation-os): Use grub-efi.
---
 gnu/system/install.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index f9aa7f673..97cc0c9f6 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -299,7 +299,8 @@ Use Alt-F2 for documentation.
     (host-name "gnu")
     (timezone "Europe/Paris")
     (locale "en_US.utf8")
-    (bootloader (grub-configuration
+    (bootloader (bootloader-configuration
+                 (bootloader grub-efi-bootloader)
                  (device "/dev/sda")))
     (file-systems
      ;; Note: the disk image build code overrides this root file system with

[bug#27705] [PATCH 4/4] install: Use grub-efi.

[Danny Milosavljevic]

This makes the system disk-image use grub-efi all the time, also for non-ISO9660.

But I've successfully tested booting this non-ISO9660 image (which uses grub-efi) as well on a non-UEFI non-Libreboot x86.

[bug#27705] [PATCH v2 0/4] Make both EFI and non-EFI systems boot our ISO9660 disk images.

[Danny Milosavljevic]

I've successfully tested it using:

$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -cdrom Z -serial stdio
$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -hda Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -cdrom Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -hda Z -serial stdio

Danny Milosavljevic (4):
  gnu: grub-efi: Add mtools input.
  build: Allow mounting of entire disks.
  gnu: Add grub-hybrid.
  vm: Use grub-hybrid's grub-mkrescue.

 gnu/build/file-systems.scm   | 13 ++++++++-----
 gnu/packages/bootloaders.scm | 36 +++++++++++++++++++++++++++++++++++-
 gnu/system/vm.scm            |  2 +-
 3 files changed, 44 insertions(+), 7 deletions(-)

[bug#27705] [PATCH v2 1/4] gnu: grub-efi: Add mtools input.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm: Add (gnu packages mtools).
(grub-efi)[inputs]: Add mtools.
[arguments]: Add phase "use-absolute-mtools-path".
---
 gnu/packages/bootloaders.scm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 802e8ab8d..7a91e32d9 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -37,6 +37,7 @@
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages man)
+  #:use-module (gnu packages mtools)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages python)
@@ -149,6 +150,7 @@ menu to select one of the installed operating systems.")
     (synopsis "GRand Unified Boot loader (UEFI version)")
     (inputs
      `(("efibootmgr" ,efibootmgr)
+       ("mtools", mtools)
        ,@(package-inputs grub)))
     (arguments
      `(;; TODO: Tests need a UEFI firmware for qemu. There is one at
@@ -166,7 +168,19 @@ menu to select one of the installed operating systems.")
                      (("efibootmgr")
                       (string-append (assoc-ref inputs "efibootmgr")
                                      "/sbin/efibootmgr")))
-                   #t)))))))))
+                   #t))
+               (add-after 'patch-stuff 'use-absolute-mtools-path
+                 (lambda* (#:key inputs #:allow-other-keys)
+                   (let ((mtools (assoc-ref inputs "mtools")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mformat\"")
+                        (string-append "\"" mtools
+                                       "/bin/mformat\"")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mcopy\"")
+                        (string-append "\"" mtools
+                                       "/bin/mcopy\"")))
+                     #t))))))))))
 
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))

[bug#27705] [PATCH v2 2/4] build: Allow mounting of entire disks.

[Danny Milosavljevic]

* gnu/build/file-systems.scm (disk-partitions): Also return entire drives.
---
 gnu/build/file-systems.scm | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index b6930497d..462ed9b7f 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -377,11 +377,14 @@ not valid header was found."
     (string-ref str (- (string-length str) 1)))
 
   (define (partition? name major minor)
-    ;; Select device names that end in a digit, like libblkid's 'probe_all'
-    ;; function does.  Checking for "/sys/dev/block/MAJOR:MINOR/partition"
-    ;; doesn't work for partitions coming from mapped devices.
-    (and (char-set-contains? char-set:digit (last-character name))
-         (> major 2)))                      ;ignore RAM disks and floppy disks
+    ;; grub-mkrescue does some funny things for EFI support which
+    ;; makes it a lot more difficult than one would expect to support
+    ;; booting an ISO-9660 image from an USB flash drive.
+    ;; For example there's a buggy (too small) hidden partition in it
+    ;; which Linux rightfully refuses to mount.
+    ;; In any case, partition tables are supposed to be optional so
+    ;; here we allow checking entire disks for file systems, too.
+    (> major 2))                      ;ignore RAM disks and floppy disks
 
   (call-with-input-file "/proc/partitions"
     (lambda (port)

[bug#27705] [PATCH v2 3/4] gnu: Add grub-hybrid.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm (grub-hybrid): New variable.
---
 gnu/packages/bootloaders.scm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 7a91e32d9..a7a4f3625 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -182,6 +182,26 @@ menu to select one of the installed operating systems.")
                                        "/bin/mcopy\"")))
                      #t))))))))))
 
+(define-public grub-hybrid
+  (package
+    (inherit grub-efi)
+    (name "grub-hybrid")
+    (synopsis "GRand Unified Boot loader (hybrid version)")
+    (native-inputs
+     `(("grub" ,grub)
+       ,@(package-native-inputs grub-efi)))
+    (arguments
+     (substitute-keyword-arguments (package-arguments grub-efi)
+      ((#:phases phases)
+      `(modify-phases ,phases
+         (add-after 'install 'install-non-efi
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (copy-recursively (string-append (assoc-ref inputs "grub")
+                                              "/lib/grub/i386-pc")
+                               (string-append (assoc-ref outputs "out")
+                                              "/lib/grub/i386-pc"))
+             #t))))))))
+
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))
     (package

[bug#27705] [PATCH v2 4/4] vm: Use grub-hybrid's grub-mkrescue.

[Danny Milosavljevic]

* gnu/system/vm.scm (iso9660-image): Use grub-hybrid's grub-mkrescue.
---
 gnu/system/vm.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 90d29b078..f9967254c 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -217,7 +217,7 @@ INPUTS is a list of inputs (as for packages)."
                         inputs)))
 
            (set-path-environment-variable "PATH" '("bin" "sbin") inputs)
-           (make-iso9660-image #$(bootloader-package bootloader)
+           (make-iso9660-image #$grub-hybrid
                                #$bootcfg-drv
                                #$os-drv
                                "/xchg/guixsd.iso"

[bug#27705] [PATCH v3 0/4] Make both EFI and non-EFI systems boot our ISO9660 disk images.

[Danny Milosavljevic]

I've successfully tested it using:

$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -cdrom Z -serial stdio
$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -hda Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -cdrom Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -hda Z -serial stdio

Danny Milosavljevic (4):
  gnu: grub-efi: Add mtools input.
  build: Allow mounting of entire disks.
  gnu: Add grub-hybrid.
  vm: Use grub-hybrid's grub-mkrescue.

 gnu/build/file-systems.scm   | 13 ++++++++-----
 gnu/packages/bootloaders.scm | 40 +++++++++++++++++++++++++++++++++++++++-
 gnu/system/vm.scm            |  2 +-
 3 files changed, 48 insertions(+), 7 deletions(-)

[bug#27705] [PATCH v3 1/4] gnu: grub-efi: Add mtools input.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm: Add (gnu packages mtools).
(grub-efi)[inputs]: Add mtools.
[arguments]: Add phase "use-absolute-mtools-path".
---
 gnu/packages/bootloaders.scm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 802e8ab8d..7a91e32d9 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -37,6 +37,7 @@
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages man)
+  #:use-module (gnu packages mtools)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages python)
@@ -149,6 +150,7 @@ menu to select one of the installed operating systems.")
     (synopsis "GRand Unified Boot loader (UEFI version)")
     (inputs
      `(("efibootmgr" ,efibootmgr)
+       ("mtools", mtools)
        ,@(package-inputs grub)))
     (arguments
      `(;; TODO: Tests need a UEFI firmware for qemu. There is one at
@@ -166,7 +168,19 @@ menu to select one of the installed operating systems.")
                      (("efibootmgr")
                       (string-append (assoc-ref inputs "efibootmgr")
                                      "/sbin/efibootmgr")))
-                   #t)))))))))
+                   #t))
+               (add-after 'patch-stuff 'use-absolute-mtools-path
+                 (lambda* (#:key inputs #:allow-other-keys)
+                   (let ((mtools (assoc-ref inputs "mtools")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mformat\"")
+                        (string-append "\"" mtools
+                                       "/bin/mformat\"")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mcopy\"")
+                        (string-append "\"" mtools
+                                       "/bin/mcopy\"")))
+                     #t))))))))))
 
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))

[bug#27705] [PATCH v3 2/4] build: Allow mounting of entire disks.

[Danny Milosavljevic]

* gnu/build/file-systems.scm (disk-partitions): Also return entire drives.
---
 gnu/build/file-systems.scm | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index b6930497d..462ed9b7f 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -377,11 +377,14 @@ not valid header was found."
     (string-ref str (- (string-length str) 1)))
 
   (define (partition? name major minor)
-    ;; Select device names that end in a digit, like libblkid's 'probe_all'
-    ;; function does.  Checking for "/sys/dev/block/MAJOR:MINOR/partition"
-    ;; doesn't work for partitions coming from mapped devices.
-    (and (char-set-contains? char-set:digit (last-character name))
-         (> major 2)))                      ;ignore RAM disks and floppy disks
+    ;; grub-mkrescue does some funny things for EFI support which
+    ;; makes it a lot more difficult than one would expect to support
+    ;; booting an ISO-9660 image from an USB flash drive.
+    ;; For example there's a buggy (too small) hidden partition in it
+    ;; which Linux rightfully refuses to mount.
+    ;; In any case, partition tables are supposed to be optional so
+    ;; here we allow checking entire disks for file systems, too.
+    (> major 2))                      ;ignore RAM disks and floppy disks
 
   (call-with-input-file "/proc/partitions"
     (lambda (port)

[bug#27705] [PATCH v3 3/4] gnu: Add grub-hybrid.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm (grub-hybrid): New variable.
---
 gnu/packages/bootloaders.scm | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 7a91e32d9..9d18e7d9a 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -182,6 +182,30 @@ menu to select one of the installed operating systems.")
                                        "/bin/mcopy\"")))
                      #t))))))))))
 
+(define-public grub-hybrid
+  (package
+    (inherit grub-efi)
+    (name "grub-hybrid")
+    (synopsis "GRand Unified Boot loader (hybrid version)")
+    (inputs
+     `(("grub" ,grub)
+       ,@(package-inputs grub-efi)))
+    (arguments
+     (substitute-keyword-arguments (package-arguments grub-efi)
+      ((#:phases phases)
+      `(modify-phases ,phases
+         (add-after 'install 'install-non-efi
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((input-dir (string-append (assoc-ref inputs "grub") "/lib/grub"))
+                   (output-dir (string-append (assoc-ref outputs "out") "/lib/grub")))
+               (for-each
+                 (lambda (basename)
+                   (if (not (string-prefix? "." basename))
+                       (symlink (string-append input-dir "/" basename)
+                                (string-append output-dir "/" basename))))
+                 ((@@ (ice-9 ftw) scandir) input-dir))
+               #t)))))))))
+
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))
     (package

[bug#27705] [PATCH v3 4/4] vm: Use grub-hybrid's grub-mkrescue.

[Danny Milosavljevic]

* gnu/system/vm.scm (iso9660-image): Use grub-hybrid's grub-mkrescue.
---
 gnu/system/vm.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 90d29b078..f9967254c 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -217,7 +217,7 @@ INPUTS is a list of inputs (as for packages)."
                         inputs)))
 
            (set-path-environment-variable "PATH" '("bin" "sbin") inputs)
-           (make-iso9660-image #$(bootloader-package bootloader)
+           (make-iso9660-image #$grub-hybrid
                                #$bootcfg-drv
                                #$os-drv
                                "/xchg/guixsd.iso"

[bug#27705] [PATCH v4 0/4] Make both EFI and non-EFI systems boot our ISO9660 disk images.

[Danny Milosavljevic]

I've successfully tested it using:

$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -cdrom Z -serial stdio
$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -hda Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -cdrom Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -hda Z -serial stdio

Danny Milosavljevic (4):
  gnu: grub-efi: Add mtools input.
  build: Allow mounting of entire disks.
  gnu: Add grub-hybrid.
  vm: Use grub-hybrid's grub-mkrescue.

 gnu/bootloader/grub.scm      |  6 ++++++
 gnu/build/file-systems.scm   | 13 ++++++++-----
 gnu/packages/bootloaders.scm | 40 +++++++++++++++++++++++++++++++++++++++-
 gnu/system/vm.scm            |  7 +++++++
 4 files changed, 60 insertions(+), 6 deletions(-)

[bug#27705] [PATCH v4 1/4] gnu: grub-efi: Add mtools input.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm: Add (gnu packages mtools).
(grub-efi)[inputs]: Add mtools.
[arguments]: Add phase "use-absolute-mtools-path".
---
 gnu/packages/bootloaders.scm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 802e8ab8d..7a91e32d9 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -37,6 +37,7 @@
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages man)
+  #:use-module (gnu packages mtools)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages python)
@@ -149,6 +150,7 @@ menu to select one of the installed operating systems.")
     (synopsis "GRand Unified Boot loader (UEFI version)")
     (inputs
      `(("efibootmgr" ,efibootmgr)
+       ("mtools", mtools)
        ,@(package-inputs grub)))
     (arguments
      `(;; TODO: Tests need a UEFI firmware for qemu. There is one at
@@ -166,7 +168,19 @@ menu to select one of the installed operating systems.")
                      (("efibootmgr")
                       (string-append (assoc-ref inputs "efibootmgr")
                                      "/sbin/efibootmgr")))
-                   #t)))))))))
+                   #t))
+               (add-after 'patch-stuff 'use-absolute-mtools-path
+                 (lambda* (#:key inputs #:allow-other-keys)
+                   (let ((mtools (assoc-ref inputs "mtools")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mformat\"")
+                        (string-append "\"" mtools
+                                       "/bin/mformat\"")))
+                     (substitute* "util/grub-mkrescue.c"
+                       (("\"mcopy\"")
+                        (string-append "\"" mtools
+                                       "/bin/mcopy\"")))
+                     #t))))))))))
 
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))

[bug#27705] [PATCH v4 1/4] gnu: grub-efi: Add mtools input.

[Danny Milosavljevic]

Applied only this patch to master as 444f9dccc24138acf833f6fea17a8b47a776ba2b.

[bug#27705] [PATCH v4 2/4] build: Allow mounting of entire disks.

[Danny Milosavljevic]

* gnu/build/file-systems.scm (disk-partitions): Also return entire drives.
---
 gnu/build/file-systems.scm | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index b6930497d..462ed9b7f 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -377,11 +377,14 @@ not valid header was found."
     (string-ref str (- (string-length str) 1)))
 
   (define (partition? name major minor)
-    ;; Select device names that end in a digit, like libblkid's 'probe_all'
-    ;; function does.  Checking for "/sys/dev/block/MAJOR:MINOR/partition"
-    ;; doesn't work for partitions coming from mapped devices.
-    (and (char-set-contains? char-set:digit (last-character name))
-         (> major 2)))                      ;ignore RAM disks and floppy disks
+    ;; grub-mkrescue does some funny things for EFI support which
+    ;; makes it a lot more difficult than one would expect to support
+    ;; booting an ISO-9660 image from an USB flash drive.
+    ;; For example there's a buggy (too small) hidden partition in it
+    ;; which Linux rightfully refuses to mount.
+    ;; In any case, partition tables are supposed to be optional so
+    ;; here we allow checking entire disks for file systems, too.
+    (> major 2))                      ;ignore RAM disks and floppy disks
 
   (call-with-input-file "/proc/partitions"
     (lambda (port)

[bug#27705] [PATCH v4 2/4] build: Allow mounting of entire disks.

[Danny Milosavljevic]

Okay I'm risking it.  A big reason for magic numbers in all the common superblock types and also in the partition table is that this usecase of optional partition tables works.

I've also excessively tested it.

So I think the risk of breakage is low enough and I've pushed a variant of this patch to master as 9833bcfc08ef009b9e8b4398baa481ef65c80ad7.

I'll be obsessively checking the Guix mailing lists through the next few days - and if it causes problems I'll revert.  Should I be too slow, feel free to revert 9833bcfc08ef009b9e8b4398baa481ef65c80ad7 (only; the other patches of the patchset are independent and aren't a cause of concern).

Since the modified code is part of the system generation one can select the older generation in the Grub boot menu should the new one not work.

[bug#27705] [PATCH v4 2/4] build: Allow mounting of entire disks.

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Okay I'm risking it.  A big reason for magic numbers in all the common
> superblock types and also in the partition table is that this usecase
> of optional partition tables works.
>
> I've also excessively tested it.

Thanks for moving forward on this front!

Ludo’.

[bug#27705] [PATCH v4 3/4] gnu: Add grub-hybrid.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm (grub-hybrid): New variable.
---
 gnu/packages/bootloaders.scm | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 7a91e32d9..9d18e7d9a 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -182,6 +182,30 @@ menu to select one of the installed operating systems.")
                                        "/bin/mcopy\"")))
                      #t))))))))))
 
+(define-public grub-hybrid
+  (package
+    (inherit grub-efi)
+    (name "grub-hybrid")
+    (synopsis "GRand Unified Boot loader (hybrid version)")
+    (inputs
+     `(("grub" ,grub)
+       ,@(package-inputs grub-efi)))
+    (arguments
+     (substitute-keyword-arguments (package-arguments grub-efi)
+      ((#:phases phases)
+      `(modify-phases ,phases
+         (add-after 'install 'install-non-efi
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((input-dir (string-append (assoc-ref inputs "grub") "/lib/grub"))
+                   (output-dir (string-append (assoc-ref outputs "out") "/lib/grub")))
+               (for-each
+                 (lambda (basename)
+                   (if (not (string-prefix? "." basename))
+                       (symlink (string-append input-dir "/" basename)
+                                (string-append output-dir "/" basename))))
+                 ((@@ (ice-9 ftw) scandir) input-dir))
+               #t)))))))))
+
 (define-public syslinux
   (let ((commit "bb41e935cc83c6242de24d2271e067d76af3585c"))
     (package

[bug#27705] [PATCH v4 3/4] gnu: Add grub-hybrid.

[Danny Milosavljevic]

Pushed this patch (with minimal changes) to master as dd4b7476ec2c8c1d790f1366173a15bd6042eeee.

[bug#27705] [PATCH v4 4/4] vm: Use grub-hybrid's grub-mkrescue.

[Danny Milosavljevic]

* gnu/system/vm.scm (system-disk-image): Use grub-hybrid's grub-mkrescue.
* gnu/bootlader/grub.scm (grub-mkrescue-bootloader): New variable.
---
 gnu/bootloader/grub.scm | 6 ++++++
 gnu/system/vm.scm       | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 880491c98..e2d31c4ac 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -55,6 +55,7 @@
 
             grub-bootloader
             grub-efi-bootloader
+            grub-mkrescue-bootloader
 
             grub-configuration))
 
@@ -413,6 +414,11 @@ submenu \"GNU system, old configurations...\" {~%")
    (name 'grub-efi)
    (package grub-efi)))
 
+(define* grub-mkrescue-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (package grub-hybrid)))
+
 \f
 ;;;
 ;;; Compatibility macros.
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 90d29b078..4494af003 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -49,6 +49,7 @@
   #:use-module (gnu packages admin)
 
   #:use-module (gnu bootloader)
+  #:use-module ((gnu bootloader grub) #:select (grub-mkrescue-bootloader))
   #:use-module (gnu system shadow)
   #:use-module (gnu system pam)
   #:use-module (gnu system linux-initrd)
@@ -369,6 +370,12 @@ to USB sticks meant to be read-only."
                                #:volatile-root? #t
                                rest)))
 
+              (bootloader (if (string=? "iso9660" file-system-type)
+                              (bootloader-configuration
+                                (inherit (operating-system-bootloader os))
+                                (bootloader grub-mkrescue-bootloader))
+                              (operating-system-bootloader os)))
+
               ;; Force our own root file system.
               (file-systems (cons (file-system
                                     (mount-point "/")

[bug#27705] [PATCH v4 4/4] vm: Use grub-hybrid's grub-mkrescue.

[Danny Milosavljevic]

Pushed this patch to master as ba4de3b3ef07a95f8a610cdc5354de554b7a4a6b.

This means that these work now:

$ cp `guix system disk-image -t iso9660 gnu/system/install.scm` Z
$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -cdrom Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -cdrom Z -serial stdio

And these don't work (as in: guest kernel crashes because it can't find the root):

$ cp `guix system disk-image -t iso9660 gnu/system/install.scm` Z
$ chmod +w Z
$ qemu-system-x86_64 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -m 1G -enable-kvm -hda Z -serial stdio
$ qemu-system-x86_64 -m 1G -enable-kvm -hda Z -serial stdio

That's because "[PATCH v4 2/4] build: Allow mounting of entire disks" is not applied.  What do we do about it?  Should we apply it, too?