Subject: Re: bug#44196: [PATCH 2/3] system: Add store-directory-prefix to boot-parameters.

From: Miguel Ángel Arruga Vivas <[email-masked]>
Date: Sun, 01 Nov 2020 00:02:14 +0100

Hi Ludo!

First of all, thanks for the review.

Ludovic Courtès <[email-masked]> writes:
> Could you explain why we need ‘store-directory-prefix’ in addition to
> ‘store-mount-point’?  At first sight, looking at the fields in there,
> these two can seem to be synonymous.

The difference is subtle unless you use btrfs subvolumes... or bind
mounts, which currently probably would fail as the check is only
performed for btrfs partitions---note to self, think about this and
probably prepare a patch.  This check was already implemented, but the
parameter was only provided to grub-configuration-file during
init/reconfigure.

Should I add an explanation of/link the bug on the commit message?  Or
maybe is something like this better?
----
;; Mount point of the store device (as in /etc/fstab's second field)
(store-mount-point boot-parameters-store-mount-point)
;; Actual path of the store inside of the device at boot time.
(store-directory-prefix boot-parameters-store-directory-prefix)
----

In any case, if that doesn't clarify enough, it can be better understood
with an example:
-----
Disk configuration:
/dev/xda1: btrfs file system
  - /rootfs: subvolume mounted on /
  - /gnufs: subvolume mounted on /gnu.

Therefore the serialized boot-parameters should be:
(boot-parameters
  ...
  (store
    (device "/dev/xda1")
    (mount-point "/gnu")
    (directory-prefix "/gnufs"))
  ...)
-----
This way grub.cfg generation is able to `normalize-file' store paths and
transform the user visible file /gnu/store/xxxxx-kernel to the grub
visible path /gnufs/store/xxxx-kernel in the final file.  This is
already provided by the operating-system definition, but the other calls
to the boot configuration generator only rely on the information
provided by boot-parameters.

A simple test case that I haven't implemented yet as a marionette---but
I should anyway---only needs this steps:

1. Create initial os with that kind of disk configuration.
2. Boot on it.
3. Create a second generation.
4. Call guix system delete-generations.
5. Reboot---it doesn't as grub.cfg is wrong.
6. Usual tests.

Tomorrow I won't be able to do much, but this Monday I could try to
implement something like this too if you think it's worth.

> Also patch #3 doesn’t depend on it, does it?

All the patches are independent, as they fix separate issues, but all
are related with bugs detected with my tests with btrfs subvolumes.  I
should have specified that they were a patch set, even though that one
was getting close to become a series... hopefully convergent. :)

Thanks again,
Miguel