From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:34939) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iC9eF-0006YT-IO for guix-patches@gnu.org; Sun, 22 Sep 2019 17:44:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iC9eD-0005sk-Qv for guix-patches@gnu.org; Sun, 22 Sep 2019 17:44:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:53998) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iC9eD-0005sB-MW for guix-patches@gnu.org; Sun, 22 Sep 2019 17:44:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1iC9eD-0007S3-JX for guix-patches@gnu.org; Sun, 22 Sep 2019 17:44:01 -0400 Subject: [bug#37305] [PATCH] Allow booting from a Btrfs subvolume. Resent-Message-ID: From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <87sgpby4p9.fsf@gmail.com> Date: Sun, 22 Sep 2019 23:43:03 +0200 In-Reply-To: <87sgpby4p9.fsf@gmail.com> (Maxim Cournoyer's message of "Thu, 05 Sep 2019 09:20:02 +0900") Message-ID: <87y2yg3t3s.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Maxim Cournoyer Cc: 37305@debbugs.gnu.org Hi! Maxim Cournoyer 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 > Date: Sun, 14 Jul 2019 20:50:23 +0900 > Subject: [PATCH 1/4] bootloader: grub: Allow booting from a Btrfs subvolu= me. > > * gnu/bootloader/grub.scm (prepend-subvol, arguments->subvol): New proced= ures. > (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 u= se 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 =C2=A9 2016 Chris Marusich > ;;; Copyright =C2=A9 2017 Leo Famulari > ;;; Copyright =C2=A9 2017 Mathieu Othacehe > +;;; Copyright =C2=A9 2019 Maxim Cournoyer > ;;; > ;;; 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)) =E2=80=98%not-comma=E2=80=99 is not a great API and not quite related to li= nux-modules :-), so it=E2=80=99s one of the rare cases where I=E2=80=99d 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=3D> rootflags (cut string-tokenize <> %= not-comma)))) > + (and=3D> rootflags-options (cut find-long-option "subvol" <>)))) Maybe rename =E2=80=98arguments->subvol=E2=80=99 to =E2=80=98linux-argument= s-btrfs-subvolume=E2=80=99 or similar? Is =E2=80=9Crootflags=E2=80=9D 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 po= int. > ;; 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 "sub= vol" > + ;; 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 =E2=80=A6) (strip-mount-point =E2=80=A6)) than to hide the =E2=80=98if=E2=80=99 in =E2=80=98prepend-subvol=E2=80=99. Regarding the interface, it=E2=80=99s the only time where we extract info f= rom the user-provided =E2=80=98kernel-arguments=E2=80=99 list. Usually it=E2= =80=99s the other way around: we have =E2=80=98file-systems=E2=80=99, and from that we build up t= he list of kernel arguments. Do you think it could be made to work similarly? (I=E2=80=99m not familiar= with Btrfs though.) That way, we wouldn=E2=80=99t have to parse the kernel arguments, the user wouldn=E2=80=99t 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-stri= ng > +arguments are ignored." > (let ((opt (string-append option "=3D"))) > - (and=3D> (find (cut string-prefix? opt <>) > + (and=3D> (find (lambda (arg) > + (and (string? arg) > + (string-prefix? opt arg))) > arguments) > (lambda (arg) > (substring arg (+ 1 (string-index arg #\=3D))))))) 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 > 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 > 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 > 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 FLA= GS > and OPTIONS arguments; and document them. Pass those to the `mount' call= s. > (boot-system): Parse the "rootflags" kernel argument, and use it when cal= ling > `mount-root-file-system'. > * doc/guix.texi (Initial RAM Disk): Document the use of the "rootflags" > argument. We=E2=80=99ll have to wait for patch #1, but in the final patch set, it sho= uld probably come before patch #1, no? Thank you! Ludo=E2=80=99.