[bug#37305] [PATCH v3] Allow booting from a Btrfs subvolume

[Ludovic Courtès]

Hi Maxim,

Sorry for dropping the ball for sooo long.

Maxim Cournoyer <address@hidden> skribis:

>>From b03a574ad565b34bbe8a7d3d0322591850984dc6 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <address@hidden>
> Date: Tue, 11 Feb 2020 14:00:06 -0500
> Subject: [PATCH 2/4] linux-boot: Refactor boot-system.
>
> The --root option can now be omitted, and inferred from the root file system
> declaration instead.
>
> * gnu/build/file-systems.scm (canonicalize-device-spec): Extend to support NFS
> directly, and...
> * gnu/build/linux-boot.scm (boot-system): ...remove NFS special casing from
> here.  Remove nested definitions for root-fs-type, root-fs-flags and
> root-fs-options, and bind those inside the let* instead.  Make "--root" take
> precedence over the device field string representation of the root file
> system.
> * doc/guix.texi (Initial RAM Disk): Document that "--root" can be left

[...]

> +++ b/gnu/build/linux-boot.scm
> @@ -498,25 +498,13 @@ upon error."
>    (define (root-mount-point? fs)
>      (string=? (file-system-mount-point fs) "/"))
>  
> -  (define root-fs-type
> -    (or (any (lambda (fs)
> -               (and (root-mount-point? fs)
> -                    (file-system-type fs)))
> -             mounts)
> -        "ext4"))
> -
> -  (define root-fs-flags
> -    (mount-flags->bit-mask (or (any (lambda (fs)
> -                                      (and (root-mount-point? fs)
> -                                           (file-system-flags fs)))
> -                                    mounts)
> -                               '())))
> -
> -  (define root-fs-options
> -    (any (lambda (fs)
> -           (and (root-mount-point? fs)
> -                (file-system-options fs)))
> -         mounts))

[...]

> +             (root-fs (find root-mount-point? mounts))
> +             (root-fs-type (or (and=> root-fs file-system-type)
> +                               "ext4"))
> +             (root-fs-device (and=> root-fs file-system-device))
> +             (root-fs-flags (mount-flags->bit-mask
> +                             (or (and=> root-fs file-system-flags)
> +                                 '())))
> +             (root-options (if root-fs
> +                               (file-system-options root-fs)
> +                               #f))

I would tend to keep these as defines to make the ‘let*’ less
intimidating, but it’s a detail.

> +        ;; XXX: Importing (guix utils) and using &fix-hint causes the
> +        ;; following error when booting the init RAM disk: "ERROR: In
> +        ;; procedure dynamic-func:\nIn procedure dynamic-pointer: Symbol not
> +        ;; found: strverscmp", so we just embed the hint in the message.

I think it should just be “FIXME: Use &fix-hint once it no longer pulls
in (guix utils).”

>>From 082934db68964890ebd2a2118fb44d66911844d3 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <address@hidden>
> Date: Sun, 14 Jul 2019 20:50:23 +0900
> Subject: [PATCH 4/4] bootloader: grub: Allow booting from a Btrfs subvolume.
>
> * gnu/bootloader/grub.scm (strip-mount-point): Remove procedure.
> (normalize-file): Add procedure.
> (grub-configuration-file): New BTRFS-SUBVOLUME-FILE-NAME parameter.  When
> defined, prepend its value to the kernel and initrd file names, using the
> NORMALIZE-FILE procedure.  Adjust the call to EYE-CANDY to pass the
> BTRFS-SUBVOLUME-FILE-NAME argument.  Normalize the KEYMAP file as well.
> (eye-candy): Add a BTRFS-SUBVOLUME-FILE-NAME parameter, and use it, along with
> the NORMALIZE-FILE procedure, to normalize the FONT-FILE and IMAGE nested
> variables.  Adjust doc.
> * gnu/bootloader/depthcharge.scm (depthcharge-configuration-file): Adapt.
> * gnu/bootloader/extlinux.scm (extlinux-configuration-file): Likewise.
> * gnu/system/file-systems.scm (btrfs-subvolume?)
> (btrfs-store-subvolume-file-name): New procedures.
> * gnu/system.scm (operating-system-bootcfg): Specify the Btrfs
> subvolume file name the store resides on to the
> `operating-system-bootcfg' procedure, using the new
> BTRFS-SUBVOLUME-FILE-NAME argument.
> * doc/guix.texi (File Systems): Add a Btrfs subsection to document the use of
> subvolumes.
> * gnu/tests/install.scm: Add test "btrfs-root-on-subvolume-os".

Please list the entities added to ‘install.scm’.

>  (define* (eye-candy config store-device store-mount-point
> +                    btrfs-store-subvolume-file-name
>                      #:key system port)

I think ‘btrfs-store-subvolume-file-name’ should be a keyword argument.

>  (define* (grub-configuration-file config entries
>                                    #:key
>                                    (system (%current-system))
> -                                  (old-entries '()))
> +                                  (old-entries '())
> +                                  btrfs-subvolume-file-name)

I wonder if we should just call it ‘store-directory-prefix’ or similar
since, after all, it’s just about prepending a prefix, which could
perhaps be useful for file systems other than Btrfs.

Thoughts?

Anyway, that’s great work, so I’ll be happy to finally see it committed
in the coming days!

Ludo’.