Message ID | 87sgpby4p9.fsf@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [bug#37305] Allow booting from a Btrfs subvolume. | expand |
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > Hello! > > I'm sending this patch series to add support for booting off Btrfs > subvolumes. There was some interested shown on #guix, so hopefully > someone can test it on their system :-) > > Before this change, it wasn't possible to pass the required options to > the Linux kernel as our init script would ignore them. > > I'm not including system tests yet, as this will take a bit more time > and is starting to be a big change in itself. Hey, I haven't got around to looking at this yet, but I did write a system test when I reported this bug [1], it's in the 0001-WIP-Btrfs-store-subvolume-test.patch file in the initial message [2]. Maybe you could see if that passes with your changes here? 1: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33517 2: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33517#5 Chris
Hi! Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > I'm sending this patch series to add support for booting off Btrfs > subvolumes. There was some interested shown on #guix, so hopefully > someone can test it on their system :-) > > Before this change, it wasn't possible to pass the required options to > the Linux kernel as our init script would ignore them. > > I'm not including system tests yet, as this will take a bit more time > and is starting to be a big change in itself. Did you have a chance to look at the system test that Chris mentioned? > From 6858efa540d89c54ce377bfa6a6882e551cd2e56 Mon Sep 17 00:00:00 2001 > From: Maxim Cournoyer <maxim.cournoyer@gmail.com> > Date: Sun, 14 Jul 2019 20:50:23 +0900 > Subject: [PATCH 1/4] bootloader: grub: Allow booting from a Btrfs subvolume. > > * gnu/bootloader/grub.scm (prepend-subvol, arguments->subvol): New procedures. > (grub-configuration-file): Use ARGUMENTS->SUBVOL to extract the subvolume name > from the kernel arguments, and prepend it to the kernel and initrd paths using > the PREPEND-SUBVOL procedure. > * tests/grub.scm: Add tests for the "arguments->subvol" procedure. > * doc/guix.texi (File Systems, (Bootloader Configuration): Document the use of > Btrfs subvolumes. [...] > +@cindex rootflags, Grub > +@cindex Btrfs root subvolume, Grub > +To allow using a Btrfs @emph{subvolume} for the root partition, the Grub-based Nitpick: s/Grub/GRUB/ > --- a/gnu/bootloader/grub.scm > +++ b/gnu/bootloader/grub.scm > @@ -3,6 +3,7 @@ > ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com> > ;;; Copyright © 2017 Leo Famulari <leo@famulari.name> > ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com> > +;;; Copyright © 2019 Maxim Cournoyer <maxim.cournoyer@gmail.com> > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -25,6 +26,8 @@ > #:use-module (guix gexp) > #:use-module (gnu artwork) > #:use-module (gnu bootloader) > + #:use-module ((gnu build linux-modules) #:select (%not-comma)) ‘%not-comma’ is not a great API and not quite related to linux-modules :-), so it’s one of the rare cases where I’d rather keep it private and duplicate it if needed. > +(define (arguments->subvol arguments) > + "Return any \"subvol\" value given as an option to the \"rootflags\" > +argument, or #f on failure." > + (let* ((rootflags (find-long-option "rootflags" arguments)) > + (rootflags-options (and=> rootflags (cut string-tokenize <> %not-comma)))) > + (and=> rootflags-options (cut find-long-option "subvol" <>)))) Maybe rename ‘arguments->subvol’ to ‘linux-arguments-btrfs-subvolume’ or similar? Is “rootflags” the commonly-used name here? > + (let* ((device (menu-entry-device entry)) > + (device-mount-point (menu-entry-device-mount-point entry)) > + (label (menu-entry-label entry)) > + (kernel (menu-entry-linux entry)) > + (arguments (menu-entry-linux-arguments entry)) > + (subvol (arguments->subvol arguments)) > + (initrd (menu-entry-initrd entry))) > ;; Here DEVICE is the store and DEVICE-MOUNT-POINT is its mount point. > ;; Use the right file names for KERNEL and INITRD in case > ;; DEVICE-MOUNT-POINT is not "/", meaning that the store is on a > ;; separate partition. > - (let ((kernel (strip-mount-point device-mount-point kernel)) > - (initrd (strip-mount-point device-mount-point initrd))) > + > + ;; Also, in case a subvolume name could be extracted from the "subvol" > + ;; option given to the "rootflags" argument of the kernel, it is > + ;; prepended to the kernel and initrd paths, to allow booting from > + ;; a Btrfs subvolume. > + (let ((kernel (prepend-subvol subvol (strip-mount-point > + device-mount-point kernel))) > + (initrd (prepend-subvol subvol (strip-mount-point > + device-mount-point initrd)))) It might be clearer to have an explicit: (if subvolume #~(string-append #$subvolume …) (strip-mount-point …)) than to hide the ‘if’ in ‘prepend-subvol’. Regarding the interface, it’s the only time where we extract info from the user-provided ‘kernel-arguments’ list. Usually it’s the other way around: we have ‘file-systems’, and from that we build up the list of kernel arguments. Do you think it could be made to work similarly? (I’m not familiar with Btrfs though.) That way, we wouldn’t have to parse the kernel arguments, the user wouldn’t have to fiddle explicitly with kernel arguments, and the end result might more easily work with all the bootloaders. > (define (find-long-option option arguments) > "Find OPTION among ARGUMENTS, where OPTION is something like \"--load\". > -Return the value associated with OPTION, or #f on failure." > +Return the value associated with OPTION, or #f on failure. Any non-string > +arguments are ignored." > (let ((opt (string-append option "="))) > - (and=> (find (cut string-prefix? opt <>) > + (and=> (find (lambda (arg) > + (and (string? arg) > + (string-prefix? opt arg))) > arguments) > (lambda (arg) > (substring arg (+ 1 (string-index arg #\=))))))) Ignoring non-strings makes for a weird API IMO. :-) I understand this is because, when using it on the host side, you may now pass it gexps instead of strings. But perhaps that calls for a new procedure? > From 3a628d1e731b2857a4c964484213cce980cb596f Mon Sep 17 00:00:00 2001 > From: Maxim Cournoyer <maxim.cournoyer@gmail.com> > Date: Tue, 16 Jul 2019 18:09:38 +0900 > Subject: [PATCH 2/4] build: initrd: Fix "write-cpio-archive" return value. > > * gnu/build/linux-initrd.scm (write-cpio-archive): Really return OUTPUT on > success, even when compression is disabled. Good catch, go for it! > From 49ffe9a2645252bb708995169a9f1749f3982385 Mon Sep 17 00:00:00 2001 > From: Maxim Cournoyer <maxim.cournoyer@gmail.com> > Date: Thu, 18 Jul 2019 07:23:48 +0900 > Subject: [PATCH 3/4] linux-boot: Fix typo. > > * gnu/build/linux-boot.scm (mount-root-file-system): Fix typo. LGTM! > From b56aea9c62b015c8a8b48827f9587b1578c83af3 Mon Sep 17 00:00:00 2001 > From: Maxim Cournoyer <maxim.cournoyer@gmail.com> > Date: Thu, 18 Jul 2019 04:59:25 +0900 > Subject: [PATCH 4/4] linux-boot: Honor "rootflags" kernel argument. > > * gnu/build/linux-boot.scm (mount-root-file-system): Add the optional FLAGS > and OPTIONS arguments; and document them. Pass those to the `mount' calls. > (boot-system): Parse the "rootflags" kernel argument, and use it when calling > `mount-root-file-system'. > * doc/guix.texi (Initial RAM Disk): Document the use of the "rootflags" > argument. We’ll have to wait for patch #1, but in the final patch set, it should probably come before patch #1, no? Thank you! Ludo’.
From b56aea9c62b015c8a8b48827f9587b1578c83af3 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer <maxim.cournoyer@gmail.com> Date: Thu, 18 Jul 2019 04:59:25 +0900 Subject: [PATCH 4/4] linux-boot: Honor "rootflags" kernel argument. * gnu/build/linux-boot.scm (mount-root-file-system): Add the optional FLAGS and OPTIONS arguments; and document them. Pass those to the `mount' calls. (boot-system): Parse the "rootflags" kernel argument, and use it when calling `mount-root-file-system'. * doc/guix.texi (Initial RAM Disk): Document the use of the "rootflags" argument. --- doc/guix.texi | 19 +++++++++++++++++++ gnu/build/linux-boot.scm | 22 +++++++++++++--------- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index cc7c91ac92..1e093b38a0 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -24761,6 +24761,25 @@ Instruct the initial RAM disk as well as the @command{modprobe} command must be a comma-separated list of module names---e.g., @code{usbkbd,9pnet}. +@item rootflags=@var{options}@dots{} +@cindex mount options, passed to initrd +@cindex rootflags, initrd +This argument allows passing one or multiple file system specific mount +options to the @code{mount} procedure used by the init script. @var{options} +must be a comma-separated list of option names or option-value pairs. The +following example instructs the initial RAM disk to mount the Btrfs subvolume +named ``rootfs'' as the root file system, and to enable automatic file +defragmentation: + +@example +rootflags=subvol=rootfs,autodefrag +@end example + +Specifying the subvolume to mount by its name, as shown above, is also used in +Guix to produce a working Grub configuration for the Grub-based bootloaders +when using a Btrfs subvolume for the root file system (@xref{Bootloader +Configuration}). + @item --repl Start a read-eval-print loop (REPL) from the initial RAM disk before it tries to load kernel modules and to mount the root file system. Our diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm index b4e6421b27..b2d8f74a71 100644 --- a/gnu/build/linux-boot.scm +++ b/gnu/build/linux-boot.scm @@ -1,6 +1,7 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com> +;;; Copyright © 2019 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -360,15 +361,16 @@ the last argument of `mknod'." (filter-map string->number (scandir "/proc"))))) (define* (mount-root-file-system root type + #:optional (flags 0) options #:key volatile-root?) - "Mount the root file system of type TYPE at device ROOT. If VOLATILE-ROOT? -is true, mount ROOT read-only and make it an overlay with a writable tmpfs -using the kernel built-in overlayfs." - + "Mount the root file system of type TYPE at device ROOT. The optional FLAGS +and OPTIONS arguments behave the same as for the `mount' procedure. If +VOLATILE-ROOT? is true, mount ROOT read-only and make it an overlay with a +writable tmpfs using the kernel built-in overlayfs." (if volatile-root? (begin (mkdir-p "/real-root") - (mount root "/real-root" type MS_RDONLY) + (mount root "/real-root" type (logior MS_RDONLY flags) options) (mkdir-p "/rw-root") (mount "none" "/rw-root" "tmpfs") @@ -385,11 +387,11 @@ using the kernel built-in overlayfs." "lowerdir=/real-root,upperdir=/rw-root/upper,workdir=/rw-root/work")) (begin (check-file-system root type) - (mount root "/root" type))) + (mount root "/root" type flags options))) ;; Make sure /root/etc/mtab is a symlink to /proc/self/mounts. (false-if-exception - (delete-file "/root/etc/mtab")) + (delete-file "/root/etc/mtab")) (mkdir-p "/root/etc") (symlink "/proc/self/mounts" "/root/etc/mtab")) @@ -483,7 +485,8 @@ upon error." (mount-essential-file-systems) (let* ((args (linux-command-line)) (to-load (find-long-option "--load" args)) - (root (find-long-option "--root" args))) + (root (find-long-option "--root" args)) + (rootflags (find-long-option "rootflags" args))) (when (member "--repl" args) (start-repl)) @@ -526,7 +529,8 @@ upon error." ((uuid root) => identity) (else (file-system-label root))))) (mount-root-file-system (canonicalize-device-spec root) - root-fs-type + root-fs-type 0 + rootflags #:volatile-root? volatile-root?)) (mount "none" "/root" "tmpfs")) -- 2.23.0