emacs-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#37305: closed ([PATCH] Allow booting from a Btrfs subvolume.)


From: GNU bug Tracking System
Subject: bug#37305: closed ([PATCH] Allow booting from a Btrfs subvolume.)
Date: Wed, 20 May 2020 12:45:02 +0000

Your message dated Wed, 20 May 2020 08:44:13 -0400
with message-id <address@hidden>
and subject line Re: [bug#37305] [PATCH v3] Allow booting from a Btrfs subvolume
has caused the debbugs.gnu.org bug report #37305,
regarding [PATCH] Allow booting from a Btrfs subvolume.
to be marked as done.

(If you believe you have received this mail in error, please contact
address@hidden.)


-- 
37305: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=37305
GNU Bug Tracking System
Contact address@hidden with problems
--- Begin Message --- Subject: [PATCH] Allow booting from a Btrfs subvolume. Date: Thu, 05 Sep 2019 09:20:02 +0900 User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
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.

Attachment: 0001-bootloader-grub-Allow-booting-from-a-Btrfs-subvolume.patch
Description: Text Data

Attachment: 0002-build-initrd-Fix-write-cpio-archive-return-value.patch
Description: Text Data

Attachment: 0003-linux-boot-Fix-typo.patch
Description: Text Data

Attachment: 0004-linux-boot-Honor-rootflags-kernel-argument.patch
Description: Text Data

Attachment: signature.asc
Description: PGP signature


--- End Message ---
--- Begin Message --- Subject: Re: [bug#37305] [PATCH v3] Allow booting from a Btrfs subvolume Date: Wed, 20 May 2020 08:44:13 -0400 User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
Hi Ludovic!

Ludovic Courtès <address@hidden> writes:

> Hi Maxim,
>
> Sorry for dropping the ball for sooo long.

No worries :-)

> Maxim Cournoyer <address@hidden> skribis:

[...]

>> +++ 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.

It would only *appear* less intimidating ;-).  I personally prefer the
let* versions as the logic is more succinctly expressed (there is no
need for 'any' + lambdas, for example).

>> +        ;; 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).”

Done!

>>>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’.

Done!

>>  (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.

Done!

>>  (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?

Perhaps, but given it's not yet clear whether another file system will
require similar support from GRUB, I'd rather keep things as explicit as
possible for now.

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

Thanks for the great words and for having a last look :-).

I've added a news entry and pushed to master as:

--8<---------------cut here---------------start------------->8---
489699c456 allow-booting-from-btrfs-subvolume news: Add entry for Btrfs 
subvolume boot support.
b460ba7992 bootloader: grub: Allow booting from a Btrfs subvolume.
fa35fb58c8 file-systems: Add helpers for parsing the options string into an 
alist.
281d80d8e5 linux-boot: Refactor boot-system.
--8<---------------cut here---------------end--------------->8---

Closing!  Thanks to Pierre and Ludovic for testing and reviewing.

Maxim


--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]