bug#33517: Problem booting when using btrfs subvolume for /gnu/store

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Christopher Baines]

[-- Attachment #1.1: Type: text/plain, Size: 500 bytes --]

I'm loosing track of this issue a bit, as I've been dealing with it for
a while. I have a machine that I've setup where /gnu/store is a btrfs
subvolume. I do this so that I can make flexible use of the space on
that btrfs filesystem.

Unfortunately, the grub configuration generated for this doesn't seem to
account for this, and so it requires some tweaking to get it to boot.

A long while back, I discovered I could make the following change, then
the generated grub configuration would be fine.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Type: text/x-patch, Size: 761 bytes --]


---
 gnu/bootloader/grub.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 06856dd58c..c3ddc3e128 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -320,8 +320,8 @@ entries corresponding to old generations of the system."
       ;; 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)))
+      (let ((kernel kernel)
+            (initrd initrd))
         #~(format port "menuentry ~s {
   ~a
   linux ~a ~a
--
2.19.2

[-- Attachment #1.3: Type: text/plain, Size: 307 bytes --]



Unfortunately, it's not a proper solution, as it obviously breaks when
you actually want to strip the mount point off so that grub can find the
right files.

I'm creating a bug for this, as I think it would be good to track the
issue. I've also written a system test that I believe reproduced the
issue.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: 0001-WIP-Btrfs-store-subvolume-test.patch --]
[-- Type: text/x-patch, Size: 4362 bytes --]

From 7eee5685f95d0b6baeb97f5fdd947fe5223a61c9 Mon Sep 17 00:00:00 2001
From: Christopher Baines <mail@cbaines.net>
Date: Fri, 26 Oct 2018 18:48:32 +0100
Subject: [PATCH] WIP Btrfs store subvolume test

---
 gnu/tests/install.scm | 91 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 1 deletion(-)

diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 4764ffffde..cfa071187c 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -43,7 +43,8 @@
             %test-separate-home-os
             %test-raid-root-os
             %test-encrypted-os
-            %test-btrfs-root-os))
+            %test-btrfs-root-os
+            %test-btrfs-root-with-store-subvolume-os))
 
 ;;; Commentary:
 ;;;
@@ -826,4 +827,92 @@ build (current-guix) and then store a couple of full system images.")
                          (command (qemu-command/writable-image image)))
       (run-basic-test %btrfs-root-os command "btrfs-root-os")))))
 
+\f
+;;;
+;;; Btrfs root file system with store subvolume.
+;;;
+
+(define-os-with-source (%btrfs-root-with-store-subvolume-os
+                        %btrfs-root-with-store-subvolume-os-source)
+  ;; The OS we want to install.
+  (use-modules (gnu) (gnu tests) (srfi srfi-1))
+
+  (operating-system
+    (host-name "liberigilo")
+    (timezone "Europe/Paris")
+    (locale "en_US.UTF-8")
+
+    (bootloader (bootloader-configuration
+                 (bootloader grub-bootloader)
+                 (target "/dev/vdb")))
+    (kernel-arguments '("console=ttyS0"))
+    (file-systems (cons* (file-system
+                           (device (file-system-label "my-root"))
+                           (mount-point "/")
+                           (type "btrfs"))
+                         (file-system
+                           (device (file-system-label "my-root"))
+                           (mount-point "/gnu/store")
+                           (type "btrfs")
+                           (options "subvol=/gnu/store"))
+                         %base-file-systems))
+    (users (cons (user-account
+                  (name "charlie")
+                  (group "users")
+                  (home-directory "/home/charlie")
+                  (supplementary-groups '("wheel" "audio" "video")))
+                 %base-user-accounts))
+    (services (cons (service marionette-service-type
+                             (marionette-configuration
+                              (imported-modules '((gnu services herd)
+                                                  (guix combinators)))))
+                    %base-services))))
+
+(define %btrfs-root-with-store-subvolume-installation-script
+  ;; Shell script of a simple installation.
+  "\
+. /etc/profile
+set -e -x
+guix --version
+
+export GUIX_BUILD_OPTIONS=--no-grafts
+ls -l /run/current-system/gc-roots
+parted --script /dev/vdb mklabel gpt \\
+  mkpart primary ext2 1M 3M \\
+  mkpart primary ext2 3M 2G \\
+  set 1 boot on \\
+  set 1 bios_grub on
+mkfs.btrfs -L my-root /dev/vdb2
+mount /dev/vdb2 /mnt
+btrfs subvolume create /mnt/home
+mkdir /mnt/gnu
+btrfs subvolume create /mnt/gnu/store
+herd start cow-store /mnt
+mkdir /mnt/etc
+cp /etc/target-config.scm /mnt/etc/config.scm
+guix system build /mnt/etc/config.scm
+guix system init /mnt/etc/config.scm /mnt --no-substitutes
+sync
+reboot\n")
+
+(define %test-btrfs-root-with-store-subvolume-os
+  (system-test
+   (name "btrfs-root-with-store-subvolume-os")
+   (description
+    "Test basic functionality of an OS installed like one would do by hand.
+This test is expensive in terms of CPU and storage usage since we need to
+build (current-guix) and then store a couple of full system images.")
+   (value
+    (mlet* %store-monad
+        ((image   (run-install
+                   %btrfs-root-with-store-subvolume-os
+                   %btrfs-root-with-store-subvolume-os-source
+                   #:script
+                   %btrfs-root-with-store-subvolume-installation-script))
+
+         (command (qemu-command/writable-image image)))
+      (run-basic-test %btrfs-root-with-store-subvolume-os
+                      command
+                      "btrfs-root-with-store-subvolume-os")))))
+
 ;;; install.scm ends here
-- 
2.19.2


[-- Attachment #1.5: Type: text/plain, Size: 297 bytes --]


It would be good if a way could be found to make this "just work". I
just don't know whether the best way to do that is tweaking the way Guix
generates the grub configuration to be aware of the odd mounting, or
somehow making grub find things in the subvolume, or something else.

Thanks,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Ludovic Courtès]

Hello,

Christopher Baines <mail@cbaines.net> skribis:

> I'm loosing track of this issue a bit, as I've been dealing with it for
> a while. I have a machine that I've setup where /gnu/store is a btrfs
> subvolume. I do this so that I can make flexible use of the space on
> that btrfs filesystem.
>
> Unfortunately, the grub configuration generated for this doesn't seem to
> account for this, and so it requires some tweaking to get it to boot.
>
> A long while back, I discovered I could make the following change, then
> the generated grub configuration would be fine.
>
>
> ---
>  gnu/bootloader/grub.scm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
> index 06856dd58c..c3ddc3e128 100644
> --- a/gnu/bootloader/grub.scm
> +++ b/gnu/bootloader/grub.scm
> @@ -320,8 +320,8 @@ entries corresponding to old generations of the system."
>        ;; 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)))
> +      (let ((kernel kernel)
> +            (initrd initrd))
>          #~(format port "menuentry ~s {
>    ~a
>    linux ~a ~a
> --
> 2.19.2
>
>
>
> Unfortunately, it's not a proper solution, as it obviously breaks when
> you actually want to strip the mount point off so that grub can find the
> right files.

Is there a way ‘strip-mount-point’ or some higher-level code could
determine whether we actually need to strip the mount point?

Thanks,
Ludo’.

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Christopher Baines]

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


Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> Unfortunately, it's not a proper solution, as it obviously breaks when
>> you actually want to strip the mount point off so that grub can find the
>> right files.
>
> Is there a way ‘strip-mount-point’ or some higher-level code could
> determine whether we actually need to strip the mount point?

So, this is the file-system value that I'm using currently for the
store. The information about subvolume is in the options value.

(file-system
  (device (uuid "84fc6b78-d7ff-45df-8659-bef44b5bf0ea"))
  (type "btrfs")
  (title 'uuid)
  (mount-point "/gnu/store")
  (needed-for-boot? #t)
  (options "subvol=/gnu/store"))

I guess one approach for dealing with this would be to allow directly
configuring the stripping of the mount point somehow. Or maybe having
some btrfs-file-system record, which could store the subvol option in a
more machine readable way.

One thing that still makes me uncertian, is how grub actually is trying
to find files on the btrfs filesystem. I tried changing the default
subvolume to the one containing the store, but that didn't seem to
help. Looks like it might not be aware of subvolumes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Maxim Cournoyer]

Hi Christopher,

Christopher Baines <mail@cbaines.net> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hello,
>>
>> Christopher Baines <mail@cbaines.net> skribis:
>>
>>> Unfortunately, it's not a proper solution, as it obviously breaks when
>>> you actually want to strip the mount point off so that grub can find the
>>> right files.
>>
>> Is there a way ‘strip-mount-point’ or some higher-level code could
>> determine whether we actually need to strip the mount point?
>
> So, this is the file-system value that I'm using currently for the
> store. The information about subvolume is in the options value.
>
> (file-system
>   (device (uuid "84fc6b78-d7ff-45df-8659-bef44b5bf0ea"))
>   (type "btrfs")
>   (title 'uuid)
>   (mount-point "/gnu/store")
>   (needed-for-boot? #t)
>   (options "subvol=/gnu/store"))

Ah, that subvolume name explains why your fix would work.  I was
confused at first why such fix should work.

Grub mounts the Btrfs partition at its 'top level'.  In Btrfs, the
subvolume names appear as directories under the top level, so in your
case not striping the mount-point amounts to prepending the subvolume
name to the "real" file path location (as it has the same value as the
mount-point).

That means that your fix only works when mount-point == subvol.

Maxim

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Svante v. Erichsen]

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

The subvolume and the mount point are independent issues.

For example, I have a subvolume named @ as root volume:

(file-system
  (device "/dev/mapper/hd")
  (mount-point "/")
  (type "btrfs")
  (options "compress=lzo,ssd,subvol=@"))

It also appears that the subvolume name must be prepended to the path as if it
just was an ordinary directory (grub.cfg):

…
  linux /@/gnu/store/…
…

I could not yet confirm this from documentation, but it works like that for me.
(I semi-manually edit the generated grub.cfg currently.)

So, additionally to stripping the mount point, the subvolume needs to be
prepended (grub.scm):

  (let ((kernel (prepend-subvol device-subvol
                                (strip-mount-point device-mount-point
                                                   kernel)))
        (initrd (prepend-subvol device-subvol
                                (strip-mount-point device-mount-point
                                                   initrd))))
    …)

This would mean that the menu-entry structure needs the subvol information.
However, I wouldn't want to parse this from the options field in the file-system
entry, so I'd propose allowing a list there, maybe like this:

(file-system
  (device "/dev/mapper/hd")
  (mount-point "/")
  (type "btrfs")
  (options '(("compress" . "lzo")
             "ssd"
             ("subvol" . "@"))))

On the other hand, it might be surprising that declaring the options like this
would work for subvols, while using a string doesn't, especially when some older
documentation/blogs/gists is still hanging around on the internet, so maybe it
would be necessary to parse the options anyway (to this list structure).

I'm lacking experience in guix and guile, so making this work and submit a patch
will take me some time.

Do you think this is sensible?


-- 
Svante von Erichsen

GPG fingerprint: A78A D4FB 762F A922 A495  57E8 2649 9081 6E61 20DE

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[david.larsson]

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

I also lack the experience with guile but it sounds very sensible to
me. I can confirm that just fixing the grub-menu entries with the
subvol path works. I have been invoking guix system reconfigure with a
bash script that in the end calls a grubfix function below with the
path to each newly generated grub.cfg:

In my case the subvol is located at /snap2_rw
function grubfix(){
         sudo sed -i "s/linux\ \/gnu/linux\ \/snap2_rw\/gnu/g" "$1"
         sudo sed -i "s/initrd\ \/gnu/initrd\ \/snap2_rw\/gnu/g" "$1"
         sudo sed -i "s/--set\ \/gnu/--set\ \/snap2_rw\/gnu/g" "$1"
}

Your suggestion to do this directly in config.scm sounds great.

Best regads,
David L

On Sun, 27 Jan 2019, Svante v. Erichsen wrote:

> The subvolume and the mount point are independent issues.
>
> For example, I have a subvolume named @ as root volume:
>
> (file-system
>  (device "/dev/mapper/hd")
>  (mount-point "/")
>  (type "btrfs")
>  (options "compress=lzo,ssd,subvol=@"))
>
> It also appears that the subvolume name must be prepended to the path as if it
> just was an ordinary directory (grub.cfg):
>
> …
>  linux /@/gnu/store/…
> …
>
> I could not yet confirm this from documentation, but it works like that for me.
> (I semi-manually edit the generated grub.cfg currently.)
>
> So, additionally to stripping the mount point, the subvolume needs to be
> prepended (grub.scm):
>
>  (let ((kernel (prepend-subvol device-subvol
>                                (strip-mount-point device-mount-point
>                                                   kernel)))
>        (initrd (prepend-subvol device-subvol
>                                (strip-mount-point device-mount-point
>                                                   initrd))))
>    …)
>
> This would mean that the menu-entry structure needs the subvol information.
> However, I wouldn't want to parse this from the options field in the file-system
> entry, so I'd propose allowing a list there, maybe like this:
>
> (file-system
>  (device "/dev/mapper/hd")
>  (mount-point "/")
>  (type "btrfs")
>  (options '(("compress" . "lzo")
>             "ssd"
>             ("subvol" . "@"))))
>
> On the other hand, it might be surprising that declaring the options like this
> would work for subvols, while using a string doesn't, especially when some older
> documentation/blogs/gists is still hanging around on the internet, so maybe it
> would be necessary to parse the options anyway (to this list structure).
>
> I'm lacking experience in guix and guile, so making this work and submit a patch
> will take me some time.
>
> Do you think this is sensible?
>
>
> -- 
> Svante von Erichsen
>
> GPG fingerprint: A78A D4FB 762F A922 A495  57E8 2649 9081 6E61 20DE
>

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Maxim Cournoyer]

Hello,

"Svante v. Erichsen" <Svante.v.Erichsen@web.de> writes:

> The subvolume and the mount point are independent issues.
>
> For example, I have a subvolume named @ as root volume:
>
> (file-system
>   (device "/dev/mapper/hd")
>   (mount-point "/")
>   (type "btrfs")
>   (options "compress=lzo,ssd,subvol=@"))
>
> It also appears that the subvolume name must be prepended to the path as if it
> just was an ordinary directory (grub.cfg):
>
> …
>   linux /@/gnu/store/…
> …
>
> I could not yet confirm this from documentation, but it works like that for me.
> (I semi-manually edit the generated grub.cfg currently.)
>
> So, additionally to stripping the mount point, the subvolume needs to be
> prepended (grub.scm):
>
>   (let ((kernel (prepend-subvol device-subvol
>                                 (strip-mount-point device-mount-point
>                                                    kernel)))
>         (initrd (prepend-subvol device-subvol
>                                 (strip-mount-point device-mount-point
>                                                    initrd))))
>     …)
>
> This would mean that the menu-entry structure needs the subvol information.
> However, I wouldn't want to parse this from the options field in the file-system
> entry, so I'd propose allowing a list there, maybe like this:
>
> (file-system
>   (device "/dev/mapper/hd")
>   (mount-point "/")
>   (type "btrfs")
>   (options '(("compress" . "lzo")
>              "ssd"
>              ("subvol" . "@"))))
>
> On the other hand, it might be surprising that declaring the options like this
> would work for subvols, while using a string doesn't, especially when some older
> documentation/blogs/gists is still hanging around on the internet, so maybe it
> would be necessary to parse the options anyway (to this list structure).
>
> I'm lacking experience in guix and guile, so making this work and submit a patch
> will take me some time.
>
> Do you think this is sensible?

I had not seen this issue before implementing the solution, but it seems
we've reached the same conclusion in that the entries of initrd and
kernel of the Grub configuration need to be prepended with the subvolume
name in order for Grub to successfully boot from a subvolume.

My implementation differs a bit in that the subvolume name to be used in
the Grub config is parsed from the kernel argument "rootflags=" value;
the reasoning being that you'd have to specify it there anyway for the
kernel to mount the root partition correctly (for non-root partitions,
fstab (file-system fields) can take care of it alone).

If anyone wants to give it a try, it's available to review here:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37305.

Maxim

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Maxim Cournoyer]

Hello,

Christopher Baines <mail@cbaines.net> writes:

> I'm loosing track of this issue a bit, as I've been dealing with it for
> a while. I have a machine that I've setup where /gnu/store is a btrfs
> subvolume. I do this so that I can make flexible use of the space on
> that btrfs filesystem.
>
> Unfortunately, the grub configuration generated for this doesn't seem to
> account for this, and so it requires some tweaking to get it to boot.

[...]

This issue is now resolved as of commit
12df6684b983507b2a73e14f45d28a71cddfb3b1 on master.

Closing!

Maxim

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Christopher Baines]

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


Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hello,
>
> Christopher Baines <mail@cbaines.net> writes:
>
>> I'm loosing track of this issue a bit, as I've been dealing with it for
>> a while. I have a machine that I've setup where /gnu/store is a btrfs
>> subvolume. I do this so that I can make flexible use of the space on
>> that btrfs filesystem.
>>
>> Unfortunately, the grub configuration generated for this doesn't seem to
>> account for this, and so it requires some tweaking to get it to boot.
>
> [...]
>
> This issue is now resolved as of commit
> 12df6684b983507b2a73e14f45d28a71cddfb3b1 on master.

Thanks Maxim, I'm guessing the commit that fixes this is
b460ba7992a0b4af2ddb5927dcf062784539ef7b.

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

bug#33517: Problem booting when using btrfs subvolume for /gnu/store

[Maxim Cournoyer]

Hi Christopher,

Christopher Baines <mail@cbaines.net> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Hello,
>>
>> Christopher Baines <mail@cbaines.net> writes:
>>
>>> I'm loosing track of this issue a bit, as I've been dealing with it for
>>> a while. I have a machine that I've setup where /gnu/store is a btrfs
>>> subvolume. I do this so that I can make flexible use of the space on
>>> that btrfs filesystem.
>>>
>>> Unfortunately, the grub configuration generated for this doesn't seem to
>>> account for this, and so it requires some tweaking to get it to boot.
>>
>> [...]
>>
>> This issue is now resolved as of commit
>> 12df6684b983507b2a73e14f45d28a71cddfb3b1 on master.
>
> Thanks Maxim, I'm guessing the commit that fixes this is
> b460ba7992a0b4af2ddb5927dcf062784539ef7b.
>
> Chris

Yes (and the two supporting commits right before this one).  The one I
pointed to includes a news entry.

Maxim