bug#54528: STORE-DIRECTORY-PREFIX is global, per generation, to all bootloader menu entries

bug#54528: STORE-DIRECTORY-PREFIX is global, per generation, to all bootloader menu entries

[Maxim Cournoyer]

Hello Guix,

Recently I noticed after reconfiguring with the following
operating system definition:

--8<---------------cut here---------------start------------->8---
(use-modules (gnu bootloader)
             (gnu bootloader grub)
             (gnu packages linux)
             (gnu system file-systems)
             (guix gexp)
             (sysadmin build-machines))

;;; XXX: Copied from berlin-nodes.scm.
(define %authorized-guix-keys
  ;; List of authorized 'guix archive' keys.
  (list (local-file "keys/guix/berlin.guixsd.org-export.pub")))

(define %btrfs-raid-uuid "64a837b7-b9dc-4b64-ba95-712ba4032c71")

(define %common-btrfs-options '(("compress-force" . "zstd")
                                ("space_cache" . "v2")
                                "degraded"))

;;; Top-level Btrfs subvolume.
(define %btrfs-pool
  (file-system
    (device (uuid %btrfs-raid-uuid))
    (mount-point "/mnt/btrfs-pool")
    (create-mount-point? #t)
    (type "btrfs")
    (options (alist->file-system-options
              (cons '("subvolid" . "5")
                    %common-btrfs-options)))))

(define (btrfs-subvolume-mount name mount-point)
  "Return a file system to mount the Btrfs subvolume NAME at MOUNT-POINT."
  (file-system
    (device (uuid %btrfs-raid-uuid))
    (mount-point mount-point)
    (create-mount-point? #t)
    (type "btrfs")
    (options (alist->file-system-options
              (cons (cons "subvol" name)
                    %common-btrfs-options)))))

\f
(define node-129-os
  (let ((base-os (berlin-new-build-machine-os
                  129 #:authorized-guix-keys %authorized-guix-keys)))
    (operating-system
      (inherit base-os)
      (bootloader
       (bootloader-configuration
        (inherit (operating-system-bootloader base-os))
        (bootloader grub-bootloader)
        (targets (list "/dev/sdb" "/dev/sdc" "/dev/sdd"))
        (menu-entries
         (list (menu-entry
                (label "Previous system -- 5.15.19 (#91, 2022-02-18 22:25)")
                (linux "/gnu/store/8w9v4dka10cv0r5fyw9f0pc14fszbl03-linux-libre-5.15.19/bzImage")
                (linux-arguments
                 '("--root=my-root"
                   "--system=/var/guix/profiles/system-92-link"
                   "--load=/var/guix/profiles/system-92-link/boot"
                   "console=tty0"
                   "console=ttyS0,57600n8"))
                (initrd "/gnu/store/in2bcjh03kyv793v8bd3fizswyx1q0rq-raw-initrd/initrd.cpio.gz"))))))
      (file-systems (cons*
                     (btrfs-subvolume-mount "@root"       "/")
                     (btrfs-subvolume-mount "@etc"        "/etc")
                     (btrfs-subvolume-mount "@home"       "/home")
                     (btrfs-subvolume-mount "@cache"      "/var/cache")
                     (btrfs-subvolume-mount "@log"        "/var/log")
                     (btrfs-subvolume-mount "@secrets"    "/secrets")
                     (btrfs-subvolume-mount "@srv"        "/srv")
                     %btrfs-pool
                     %base-file-systems))
      (packages (cons btrfs-progs (operating-system-packages base-os)))
      ;; FIXME: fix swap field.
      )))

node-129-os
--8<---------------cut here---------------end--------------->8---

That the custom menu-entry object specified in the bootloader
configuration would also result in a grub.cfg entry where the linux and
initrd items would be prefixed with '/@root/' (the store directory
prefix), which is not desired (this entry corresponds to another,
previous system generation that didn't even use Btrfs).

The problem is that the store-directory-prefix is globally applied to
all menu entries corresponding to a specific generation (e.g., via the
boot-parameters file); it seems like it should rather be preserved per
menu-entry.

Thanks,

Maxim

bug#54528: [PATCH] fix store-prefix for menu-entries

[Stefan Karrmann]

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

Dear Maxim Cournoyer,

here is a patch, that fixes the global store-prefix for grub menu-entries

on commit 58a95d599ee5d0dc6419d038b7317e1b77b11519

git apply fix-store-prefix-for-menu-entry.patch

Kind regards,
--
Dr. Stefan Karrmann

[-- Attachment #2: fix-store-prefix-for-menu-entry.patch --]
[-- Type: text/x-diff, Size: 2234 bytes --]

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index ecd44e7f3c..048b9ff8fd 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -378,22 +378,25 @@ (define (menu-entry->gexp entry)
           (device-mount-point (menu-entry-device-mount-point entry))
           (multiboot-kernel (menu-entry-multiboot-kernel entry))
           (chain-loader (menu-entry-chain-loader entry)))
+      ;; Here DEVICE is the store and DEVICE-MOUNT-POINT is its mount point.
+      ;; Use the right file names for LINUX and INITRD in case
+      ;; DEVICE-MOUNT-POINT is not "/", meaning that the store is on a
+      ;; separate partition. Then we don't use STORE-DIRECTORY-PREFIX.
+      ;; When STORE-DIRECTORY-PREFIX is defined, prepend it the linux and
+      ;; initrd paths, to allow booting from a Btrfs subvolume.
+      (define (normalize-or-grub path)
+        "Normalize PATH if and only if DEVICE-MOUNT-POINT is \"/\"."
+        (if (and (string? device-mount-point)
+                 (string= "/" device-mount-point))
+            (normalize-file path
+                            device-mount-point
+                            store-directory-prefix)
+            path))
       (cond
        (linux
         (let ((arguments (menu-entry-linux-arguments entry))
-              (linux (normalize-file linux
-                                     device-mount-point
-                                     store-directory-prefix))
-              (initrd (normalize-file (menu-entry-initrd entry)
-                                      device-mount-point
-                                      store-directory-prefix)))
-          ;; Here DEVICE is the store and DEVICE-MOUNT-POINT is its mount point.
-          ;; Use the right file names for LINUX and INITRD in case
-          ;; DEVICE-MOUNT-POINT is not "/", meaning that the store is on a
-          ;; separate partition.
-
-          ;; When STORE-DIRECTORY-PREFIX is defined, prepend it the linux and
-          ;; initrd paths, to allow booting from a Btrfs subvolume.
+              (linux (normalize-or-grub linux))
+              (initrd (normalize-or-grub (menu-entry-initrd entry))))
           #~(format port "menuentry ~s {
   ~a
   linux ~a ~a