GNU bug report logs - #70826
luks-device-mapping-with-options breaks bootloader

Previous Next

To reply to this bug, email your comments to 70826 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
To: bug-guix <at> gnu.org
Subject: luks-device-mapping-with-options breaks bootloader
Date: Tue, 7 May 2024 14:54:12 -0400

[Message part 1 (text/plain, inline)]

using the `luks-device-mapping-with-options` mapped device type defined in
(gnu system mapped-devices) causes grub or other bootloaders to not
properly attempt to mount the encrypted drive. This is caused by the
commit 39a9404 which identifies luks mapped devices by checking if the type
is equal to `luks-device-mapping`, so by using a different routine that is
a proxy to that one it doesn't forward it to grub in the
store-crypto-devices list.

For anyone who finds this before it is fixed, you can boot your device by
hitting 'c' in grub and typing these commands:
grub> insmod luks
grub> insmod luks2
grub> cryptomount (XXX)
grub> set root=(crypto)
grub> configfile (YYY)/grub/grub.cfg

Where (XXX) is the encrypted partition and (YYY) is the boot partition with
the grub config, these can be found by doing `ls` command.

[Message part 2 (text/html, inline)]

Message #10 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
Cc: 70826 <at> debbugs.gnu.org
Subject: Re: bug#70826: luks-device-mapping-with-options breaks bootloader
Date: Sat, 25 May 2024 11:47:03 +0200

[Message part 1 (text/plain, inline)]

Hi,

Tadhg McDonald-Jensen <tadhgmister <at> gmail.com> skribis:

> using the `luks-device-mapping-with-options` mapped device type defined in
> (gnu system mapped-devices) causes grub or other bootloaders to not
> properly attempt to mount the encrypted drive. This is caused by the
> commit 39a9404 which identifies luks mapped devices by checking if the type
> is equal to `luks-device-mapping`, so by using a different routine that is
> a proxy to that one it doesn't forward it to grub in the
> store-crypto-devices list.

Ouch, indeed.  The immediate fix is:

[Message part 2 (text/x-patch, inline)]

diff --git a/gnu/system.scm b/gnu/system.scm
index c76f4d7c502..bb851b1b75f 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -667,10 +667,13 @@ (define (operating-system-boot-mapped-devices os)
 (define operating-system-bootloader-crypto-devices
   (mlambdaq (os)                        ;to avoid duplicated output
     "Return the sources of the LUKS mapped devices specified by UUID."
+    (define (luks-device? m)
+      (memq (mapped-device-type m)
+            (list luks-device-mapping-with-options
+                  luks-device-mapping)))
+
     ;; XXX: Device ordering is important, we trust the returned one.
-    (let* ((luks-devices (filter (lambda (m)
-                                   (eq? luks-device-mapping
-                                        (mapped-device-type m)))
+    (let* ((luks-devices (filter luks-device?
                                  (operating-system-boot-mapped-devices os)))
            (uuid-crypto-devices non-uuid-crypto-devices
                                 (partition (compose uuid? mapped-device-source)

[Message part 3 (text/plain, inline)]

Not ideal, but it fixes the problem.

I’ll go ahead with this patch if there are no objections.

Thanks!

Ludo’.

Message #13 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 70826 <at> debbugs.gnu.org
Subject: Re: bug#70826: luks-device-mapping-with-options breaks bootloader
Date: Sat, 25 May 2024 10:30:49 -0400

That unfortunately doesn't fix the problem, 
`luks-device-mapping-with-options` is a routine that returns the 
`mapped-device-kind` so it won't check by equality.

A possible solution is to check whether the `mapped-device-kind-close` 
routines are the same as these are shared.


diff --git a/gnu/system.scm b/gnu/system.scm
index cb6e719ca6..b564bf3788 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -661,10 +661,12 @@ (define (operating-system-boot-mapped-devices os)
 (define operating-system-bootloader-crypto-devices
   (mlambdaq (os)                        ;to avoid duplicated output
     "Return the sources of the LUKS mapped devices specified by UUID."
+    (define (luks-device? m)
+      (eq? (mapped-device-kind-close (mapped-device-type m))
+           (mapped-device-kind-close luks-device-mapping)))
+
     ;; XXX: Device ordering is important, we trust the returned one.
-    (let* ((luks-devices (filter (lambda (m)
-                                   (eq? luks-device-mapping
-                                        (mapped-device-type m)))
+    (let* ((luks-devices (filter luks-device?
                                  (operating-system-boot-mapped-devices 
os)))
            (uuid-crypto-devices non-uuid-crypto-devices
                                 (partition (compose uuid? 
mapped-device-source)



(I apologize if my email client is adding line wraps to the diffs, I 
will look into it after sending this)

I tried to implement this initially but it didn't work on my previous 
attempt so I abandoned trying to submit a patch, but this version does 
do the trick even if it seems inelegant.

On 2024-05-25 5:47 a.m., Ludovic Courtès wrote:
> Hi,
> 
> Tadhg McDonald-Jensen <tadhgmister <at> gmail.com> skribis:
> 
>> using the `luks-device-mapping-with-options` mapped device type defined in
>> (gnu system mapped-devices) causes grub or other bootloaders to not
>> properly attempt to mount the encrypted drive. This is caused by the
>> commit 39a9404 which identifies luks mapped devices by checking if the type
>> is equal to `luks-device-mapping`, so by using a different routine that is
>> a proxy to that one it doesn't forward it to grub in the
>> store-crypto-devices list.
> 
> Ouch, indeed.  The immediate fix is:
> 
> 
> diff --git a/gnu/system.scm b/gnu/system.scm
> index c76f4d7c502..bb851b1b75f 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -667,10 +667,13 @@ (define (operating-system-boot-mapped-devices os)
>   (define operating-system-bootloader-crypto-devices
>     (mlambdaq (os)                        ;to avoid duplicated output
>       "Return the sources of the LUKS mapped devices specified by UUID."
> +    (define (luks-device? m)
> +      (memq (mapped-device-type m)
> +            (list luks-device-mapping-with-options
> +                  luks-device-mapping)))
> +
>       ;; XXX: Device ordering is important, we trust the returned one.
> -    (let* ((luks-devices (filter (lambda (m)
> -                                   (eq? luks-device-mapping
> -                                        (mapped-device-type m)))
> +    (let* ((luks-devices (filter luks-device?
>                                    (operating-system-boot-mapped-devices os)))
>              (uuid-crypto-devices non-uuid-crypto-devices
>                                   (partition (compose uuid? mapped-device-source)
> 
> 
> 
> Not ideal, but it fixes the problem.
> 
> I’ll go ahead with this patch if there are no objections.
> 
> Thanks!
> 
> Ludo’.

Message #16 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Tomas Volf <~@wolfsden.cz>
To: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 70826 <at> debbugs.gnu.org
Subject: Re: bug#70826: luks-device-mapping-with-options breaks bootloader
Date: Tue, 23 Jul 2024 20:19:49 +0200

[Message part 1 (text/plain, inline)]

On 2024-05-25 10:30:49 -0400, Tadhg McDonald-Jensen wrote:
> That unfortunately doesn't fix the problem,
> `luks-device-mapping-with-options` is a routine that returns the
> `mapped-device-kind` so it won't check by equality.
>
> A possible solution is to check whether the `mapped-device-kind-close`
> routines are the same as these are shared.

What I find interesting is that I too am using luks-device-mapping-with-options
and my system boots just fine.  So I wonder what the difference is.  Could you
share your system configuration please?  Or at least the relevant parts (I
assume at least bootloader, file-systems and mapped-devices fields)?

I would like to properly understand the problem here and why it works for me.

Thanks,
Tomas Volf

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[signature.asc (application/pgp-signature, inline)]

Message #19 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
To: Tomas Volf <~@wolfsden.cz>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 70826 <at> debbugs.gnu.org
Subject: Re: bug#70826: luks-device-mapping-with-options breaks bootloader
Date: Sun, 11 Aug 2024 18:33:14 -0400

[Message part 1 (text/plain, inline)]

I have attached a config I just did `sudo guix system reconfigure`
and confirmed it was missing the `insmod luks` in /boot/grub/grub.cfg

Sorry for the delay,
Tadhg McD-J

On 2024-07-23 2:19 p.m., Tomas Volf wrote:
> On 2024-05-25 10:30:49 -0400, Tadhg McDonald-Jensen wrote:
>> That unfortunately doesn't fix the problem,
>> `luks-device-mapping-with-options` is a routine that returns the
>> `mapped-device-kind` so it won't check by equality.
>>
>> A possible solution is to check whether the `mapped-device-kind-close`
>> routines are the same as these are shared.
> 
> What I find interesting is that I too am using luks-device-mapping-with-options
> and my system boots just fine.  So I wonder what the difference is.  Could you
> share your system configuration please?  Or at least the relevant parts (I
> assume at least bootloader, file-systems and mapped-devices fields)?
> 
> I would like to properly understand the problem here and why it works for me.
> 
> Thanks,
> Tomas Volf
> 
> --
> There are only two hard things in Computer Science:
> cache invalidation, naming things and off-by-one errors.

[os.tmp.scm (text/x-scheme, attachment)]

Message #22 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
To: Tomas Volf <~@wolfsden.cz>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 70826 <at> debbugs.gnu.org
Subject: Re: bug#70826: luks-device-mapping-with-options breaks bootloader
Date: Sun, 11 Aug 2024 19:19:06 -0400

[Message part 1 (text/plain, inline)]

In case it is relevant my disk is using GPT partition table with this
layout:

$ lsblk --output="NAME,MAJ:MIN,TYPE,MOUNTPOINTS,UUID"
NAME MAJ:MIN TYPE MOUNTPOINTS UUID
nvme0n1 259:0 disk
├─nvme0n1p1 259:1 part /boot 5190-E840
└─nvme0n1p2 259:2 part c0010d06-0bd1-4ae2-93e6-f2f89a3a670b
└─cryptroot 253:0 crypt /gnu/store
/

Only the main partition is encrypted with LUKS and grub is located on
its own partition not in the in between space in an MBR drive.

It is grub that is being responsible for decrypting the partition
not my UEFI decrypting the whole drive.

Tadhg

On Sun, Aug 11, 2024 at 6:33 PM Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
wrote:

> I have attached a config I just did `sudo guix system reconfigure`
> and confirmed it was missing the `insmod luks` in /boot/grub/grub.cfg
>
> Sorry for the delay,
> Tadhg McD-J
>
> On 2024-07-23 2:19 p.m., Tomas Volf wrote:
> > On 2024-05-25 10:30:49 -0400, Tadhg McDonald-Jensen wrote:
> >> That unfortunately doesn't fix the problem,
> >> `luks-device-mapping-with-options` is a routine that returns the
> >> `mapped-device-kind` so it won't check by equality.
> >>
> >> A possible solution is to check whether the `mapped-device-kind-close`
> >> routines are the same as these are shared.
> >
> > What I find interesting is that I too am using
> luks-device-mapping-with-options
> > and my system boots just fine.  So I wonder what the difference is.
> Could you
> > share your system configuration please?  Or at least the relevant parts
> (I
> > assume at least bootloader, file-systems and mapped-devices fields)?
> >
> > I would like to properly understand the problem here and why it works
> for me.
> >
> > Thanks,
> > Tomas Volf
> >
> > --
> > There are only two hard things in Computer Science:
> > cache invalidation, naming things and off-by-one errors.

[Message part 2 (text/html, inline)]

Message #25 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 70826 <at> debbugs.gnu.org
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>,
 Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>, Tomas Volf <~@wolfsden.cz>
Subject: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Sat,  5 Apr 2025 01:14:55 +0530

We use <mapped-device-type> records to represent the different types of
mapped devices (LUKS, RAID, LVM).  When variables are defined for these
records, we can distinguish them with eq?; when they are created by
procedures, like luks-device-mapping-with-options, this does not work.
Therefore, add a 'name' field to <mapped-device-type> to distinguish
them.

* gnu/system/mapped-devices.scm (<mapped-device-type>): Add name field.
(luks-device-mapping, raid-device-mapping, lvm-device-mapping):
Initialize it with appropriate values for each of these types.
* gnu/system.scm (operating-system-bootloader-crypto-devices): Use it to
identify LUKS mapped devices.

Change-Id: I4c85824f74316f07239374d9df6c007dd47a9d0c
---

I've tested this on my system; in conjunction with [1], I can finally mount my
LUKS volume with the no_read_workqueue and no_write_workqueue flags.

[1] [bug#77499] [PATCH] mapped-devices/luks: Support extra options.
https://issues.guix.gnu.org/77499
https://yhetil.org/guix/fb637872bd14abe305d810b9d32e0db290b26dd6.1743702237.git.45mg.writes <at> gmail.com/


 gnu/system.scm                | 6 ++++--
 gnu/system/mapped-devices.scm | 6 ++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 0d98e5a036..87247f06ee 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -16,6 +16,7 @@
 ;;; Copyright © 2021 raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
 ;;; Copyright © 2023 Bruno Victal <mirai <at> makinata.eu>
 ;;; Copyright © 2024 Nicolas Graves <ngraves <at> ngraves.fr>
+;;; Copyright © 2025 45mg <45mg.writes <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -676,8 +677,9 @@ (define operating-system-bootloader-crypto-devices
     "Return the sources of the LUKS mapped devices specified by UUID."
     ;; XXX: Device ordering is important, we trust the returned one.
     (let* ((luks-devices (filter (lambda (m)
-                                   (eq? luks-device-mapping
-                                        (mapped-device-type m)))
+                                   (eq? (mapped-device-kind-name
+                                         (mapped-device-type m))
+                                        'luks))
                                  (operating-system-boot-mapped-devices os)))
            (uuid-crypto-devices non-uuid-crypto-devices
                                 (partition (compose uuid? mapped-device-source)
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 667a495570..50626b8df9 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Andreas Enge <andreas <at> enge.fr>
 ;;; Copyright © 2017, 2018 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2024 Tomas Volf <~@wolfsden.cz>
+;;; Copyright © 2025 45mg <45mg.writes <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -55,6 +56,7 @@ (define-module (gnu system mapped-devices)
 
             mapped-device-kind
             mapped-device-kind?
+            mapped-device-kind-name
             mapped-device-kind-open
             mapped-device-kind-close
             mapped-device-kind-modules
@@ -110,6 +112,7 @@ (define-deprecated (mapped-device-target md)
 (define-record-type* <mapped-device-type> mapped-device-kind
   make-mapped-device-kind
   mapped-device-kind?
+  (name      mapped-device-kind-name)
   (open      mapped-device-kind-open)             ;source target -> gexp
   (close     mapped-device-kind-close             ;source target -> gexp
              (default (const #~(const #f))))
@@ -283,6 +286,7 @@ (define* (check-luks-device md #:key
 (define luks-device-mapping
   ;; The type of LUKS mapped devices.
   (mapped-device-kind
+   (name 'luks)
    (open open-luks-device)
    (close close-luks-device)
    (check check-luks-device)
@@ -338,6 +342,7 @@ (define (close-raid-device sources targets)
 (define raid-device-mapping
   ;; The type of RAID mapped devices.
   (mapped-device-kind
+   (name 'raid)
    (open open-raid-device)
    (close close-raid-device)))
 
@@ -358,6 +363,7 @@ (define (close-lvm-device source targets)
 
 (define lvm-device-mapping
   (mapped-device-kind
+   (name 'lvm)
    (open open-lvm-device)
    (close close-lvm-device)
    (modules '((srfi srfi-1)))))

base-commit: 0b754ceeded322e8079130e6793b0c68356967cf
-- 
2.49.0

Message #28 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, Tomas Volf <~@wolfsden.cz>,
 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Thu, 10 Apr 2025 21:18:30 +0200

[Message part 1 (text/plain, inline)]

Hi,

45mg <45mg.writes <at> gmail.com> skribis:

> We use <mapped-device-type> records to represent the different types of
> mapped devices (LUKS, RAID, LVM).  When variables are defined for these
> records, we can distinguish them with eq?; when they are created by
> procedures, like luks-device-mapping-with-options, this does not work.
> Therefore, add a 'name' field to <mapped-device-type> to distinguish
> them.
>
> * gnu/system/mapped-devices.scm (<mapped-device-type>): Add name field.
> (luks-device-mapping, raid-device-mapping, lvm-device-mapping):
> Initialize it with appropriate values for each of these types.
> * gnu/system.scm (operating-system-bootloader-crypto-devices): Use it to
> identify LUKS mapped devices.
>
> Change-Id: I4c85824f74316f07239374d9df6c007dd47a9d0c
> ---
>
> I've tested this on my system; in conjunction with [1], I can finally mount my
> LUKS volume with the no_read_workqueue and no_write_workqueue flags.
>
> [1] [bug#77499] [PATCH] mapped-devices/luks: Support extra options.
> https://issues.guix.gnu.org/77499
> https://yhetil.org/guix/fb637872bd14abe305d810b9d32e0db290b26dd6.1743702237.git.45mg.writes <at> gmail.com/

You can add a “Fixes” line for the bug it fixes.

>      (let* ((luks-devices (filter (lambda (m)
> -                                   (eq? luks-device-mapping
> -                                        (mapped-device-type m)))
> +                                   (eq? (mapped-device-kind-name
> +                                         (mapped-device-type m))
> +                                        'luks))

[...]

>  (define-record-type* <mapped-device-type> mapped-device-kind
>    make-mapped-device-kind
>    mapped-device-kind?
> +  (name      mapped-device-kind-name)

As a rule of thumb, I think comparing by identity (as was done before)
is more robust and cleaner: that avoids the whole problem of this
secondary name space where name clashes may occur involuntarily.

But this problem the patch fixes was introduced by
‘luks-device-mapping-with-options’ I believe, which returns a new device
type.

If we take a step back, I wonder if a better solution would not be to
add an ‘arguments’ field to <mapped-device>, following the same pattern
as <package>.

Here’s a preliminary patch to illustrate that:

[Message part 2 (text/x-patch, inline)]

diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 72da8e55d3..17c2e6f6bf 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -229,7 +229,8 @@ (define* (raw-initrd file-systems
                   (targets (mapped-device-targets md))
                   (type    (mapped-device-type md))
                   (open    (mapped-device-kind-open type)))
-             (open source targets)))
+             (apply open source targets
+                    (mapped-device-arguments md))))
          mapped-devices))
 
   (define file-system-scan-commands
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 667a495570..29d0dc95cf 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -83,6 +83,8 @@ (define-record-type* <mapped-device> %mapped-device
   (source    mapped-device-source)                ;string | list of strings
   (targets   mapped-device-targets)               ;list of strings
   (type      mapped-device-type)                  ;<mapped-device-kind>
+  (arguments mapped-device-arguments              ;list passed to open/close/check
+             (default '()))
   (location  mapped-device-location
              (default (current-source-location)) (innate)))
 
@@ -128,13 +130,16 @@ (define device-mapping-service-type
    'device-mapping
    (match-lambda
      (($ <mapped-device> source targets
-                         ($ <mapped-device-type> open close modules))
+                         ($ <mapped-device-type> open close modules)
+                         arguments)
       (shepherd-service
        (provision (list (symbol-append 'device-mapping- (string->symbol (string-join targets "-")))))
        (requirement '(udev))
        (documentation "Map a device node using Linux's device mapper.")
-       (start #~(lambda () #$(open source targets)))
-       (stop #~(lambda _ (not #$(close source targets))))
+       (start #~(lambda ()
+                  #$(apply open source targets arguments)))
+       (stop #~(lambda _
+                 (not #$(apply close source targets arguments))))
        (modules (append %default-modules modules))
        (respawn? #f))))
    (description "Map a device node using Linux's device mapper.")))
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 23eb215561..8a56f1cc63 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -680,9 +680,10 @@ (define (check-mapped-devices os)
                             (mapped-device-type md))))
                 ;; We expect CHECK to raise an exception with a detailed
                 ;; '&message' if something goes wrong.
-                (check md
+                (apply check md
                        #:needed-for-boot? (needed-for-boot? md)
-                       #:initrd-modules initrd-modules)))
+                       #:initrd-modules initrd-modules
+                       (mapped-device-arguments md))))
             (operating-system-mapped-devices os)))
 
 (define (check-initrd-modules os)

[Message part 3 (text/plain, inline)]

WDYT?

Ludo’.

Message #31 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Tomas Volf <~@wolfsden.cz>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, 45mg <45mg.writes <at> gmail.com>,
 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Thu, 10 Apr 2025 23:55:13 +0200

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

> [..]
>>  (define-record-type* <mapped-device-type> mapped-device-kind
>>    make-mapped-device-kind
>>    mapped-device-kind?
>> +  (name      mapped-device-kind-name)
>
> As a rule of thumb, I think comparing by identity (as was done before)
> is more robust and cleaner: that avoids the whole problem of this
> secondary name space where name clashes may occur involuntarily.
>
> But this problem the patch fixes was introduced by
> ‘luks-device-mapping-with-options’ I believe, which returns a new device
> type.
>
> If we take a step back, I wonder if a better solution would not be to
> add an ‘arguments’ field to <mapped-device>, following the same pattern
> as <package>.
>
> Here’s a preliminary patch to illustrate that:
>
> diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
> index 72da8e55d3..17c2e6f6bf 100644
> --- a/gnu/system/linux-initrd.scm
> +++ b/gnu/system/linux-initrd.scm
> @@ -229,7 +229,8 @@ (define* (raw-initrd file-systems
>                    (targets (mapped-device-targets md))
>                    (type    (mapped-device-type md))
>                    (open    (mapped-device-kind-open type)))
> -             (open source targets)))
> +             (apply open source targets
> +                    (mapped-device-arguments md))))
>           mapped-devices))
>  
>    (define file-system-scan-commands
> diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
> index 667a495570..29d0dc95cf 100644
> --- a/gnu/system/mapped-devices.scm
> +++ b/gnu/system/mapped-devices.scm
> @@ -83,6 +83,8 @@ (define-record-type* <mapped-device> %mapped-device
>    (source    mapped-device-source)                ;string | list of strings
>    (targets   mapped-device-targets)               ;list of strings
>    (type      mapped-device-type)                  ;<mapped-device-kind>
> +  (arguments mapped-device-arguments              ;list passed to open/close/check
> +             (default '()))

I do not think it is a good idea to expect the same identical list for
all of the operations, so we would need arguments-open, arguments-close
and arguments-check fields.  Other that that, this would work, sure.

>    (location  mapped-device-location
>               (default (current-source-location)) (innate)))
>  
> @@ -128,13 +130,16 @@ (define device-mapping-service-type
>     'device-mapping
>     (match-lambda
>       (($ <mapped-device> source targets
> -                         ($ <mapped-device-type> open close modules))
> +                         ($ <mapped-device-type> open close modules)
> +                         arguments)
>        (shepherd-service
>         (provision (list (symbol-append 'device-mapping- (string->symbol (string-join targets "-")))))
>         (requirement '(udev))
>         (documentation "Map a device node using Linux's device mapper.")
> -       (start #~(lambda () #$(open source targets)))
> -       (stop #~(lambda _ (not #$(close source targets))))
> +       (start #~(lambda ()
> +                  #$(apply open source targets arguments)))
> +       (stop #~(lambda _
> +                 (not #$(apply close source targets arguments))))
>         (modules (append %default-modules modules))
>         (respawn? #f))))
>     (description "Map a device node using Linux's device mapper.")))
> diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
> index 23eb215561..8a56f1cc63 100644
> --- a/guix/scripts/system.scm
> +++ b/guix/scripts/system.scm
> @@ -680,9 +680,10 @@ (define (check-mapped-devices os)
>                              (mapped-device-type md))))
>                  ;; We expect CHECK to raise an exception with a detailed
>                  ;; '&message' if something goes wrong.
> -                (check md
> +                (apply check md
>                         #:needed-for-boot? (needed-for-boot? md)
> -                       #:initrd-modules initrd-modules)))
> +                       #:initrd-modules initrd-modules
> +                       (mapped-device-arguments md))))
>              (operating-system-mapped-devices os)))
>  
>  (define (check-initrd-modules os)
>
>
> WDYT?

Only thing I wonder about whether we should be apply-ing the list, or
just passing it as a final arguments.  That would be a change in the
calling convention, but all procedures could just be adjusted to take
the extra argument.

One question would be how to handle if user passes the extra arguments
to a procedure that is not prepared to handle them.  In your approach
(apply) it would be a error due to wrong number of arguments, I am not
sure whether that is ideal.  Maybe a warning would be better?  Not sure.
Opinions?

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

Message #34 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Tomas Volf <~@wolfsden.cz>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, 45mg <45mg.writes <at> gmail.com>,
 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Fri, 11 Apr 2025 10:27:50 +0200

Hello,

Tomas Volf <~@wolfsden.cz> skribis:

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

[...]

>> +++ b/gnu/system/mapped-devices.scm
>> @@ -83,6 +83,8 @@ (define-record-type* <mapped-device> %mapped-device
>>    (source    mapped-device-source)                ;string | list of strings
>>    (targets   mapped-device-targets)               ;list of strings
>>    (type      mapped-device-type)                  ;<mapped-device-kind>
>> +  (arguments mapped-device-arguments              ;list passed to open/close/check
>> +             (default '()))
>
> I do not think it is a good idea to expect the same identical list for
> all of the operations, so we would need arguments-open, arguments-close
> and arguments-check fields.  Other that that, this would work, sure.

Yeah I wondered about that but I thought that in the worst case the
operations could just ignore those arguments; from the user’s viewpoint,
that would keep the interface simple.

> Only thing I wonder about whether we should be apply-ing the list, or
> just passing it as a final arguments.  That would be a change in the
> calling convention, but all procedures could just be adjusted to take
> the extra argument.

It’s a backward-compatible change in the calling convention (nothing
changes for existing open/close/check methods that accept no keyword
arguments).

> One question would be how to handle if user passes the extra arguments
> to a procedure that is not prepared to handle them.  In your approach
> (apply) it would be a error due to wrong number of arguments, I am not
> sure whether that is ideal.  Maybe a warning would be better?  Not sure.
> Opinions?

Yes, you’d get a wrong-type-arg error, similar to what you have if you
pass #:random-number 42 in the ‘arguments’ field of a package.

It’s not ideal in terms of error reporting, but maybe good enough.

Another option would be for instance to always pass a key/value pair;
open/close/check methods would have to look up in that list for relevant
keys and could provide better error reporting, but the downside is extra
boilerplate in those methods.

Ludo’.

Message #37 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, Tomas Volf <~@wolfsden.cz>
Cc: 45mg <45mg.writes <at> gmail.com>, 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Mon, 5 May 2025 10:47:15 -0400

I figured out what is going on,

When the boot partition (the one with grub.cfg) is encrypted the command 
to decrypt it gets baked into the grub executable by `grub-install`. 
When the boot partition is not encrypted (such as when the unencrypted 
partitition is mounted to /boot) then grub determines it doesn't need a 
decryption command to load the grub.cfg and relies on the decrypt 
command being in the config to load the rest of the system.

Thus the default install process that has /boot part of the encrypted 
drive and mounts the efi partition to /boot/efi will be able to use 
`luks-device-mapping-with-options` without issue where as my setup with 
the grub.cfg being in the unencrypted drive runs into the issue I 
initially described.

I can write up a system test to reproduce my setup that causes this to 
be an issue if that'd be helpful.

Tadhg

On 2025-04-11 4:27 a.m., Ludovic Courtès wrote:
> Hello,
>
> Tomas Volf <~@wolfsden.cz> skribis:
>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
> [...]
>
>>> +++ b/gnu/system/mapped-devices.scm
>>> @@ -83,6 +83,8 @@ (define-record-type* <mapped-device> %mapped-device
>>>     (source    mapped-device-source)                ;string | list of strings
>>>     (targets   mapped-device-targets)               ;list of strings
>>>     (type      mapped-device-type)                  ;<mapped-device-kind>
>>> +  (arguments mapped-device-arguments              ;list passed to open/close/check
>>> +             (default '()))
>> I do not think it is a good idea to expect the same identical list for
>> all of the operations, so we would need arguments-open, arguments-close
>> and arguments-check fields.  Other that that, this would work, sure.
> Yeah I wondered about that but I thought that in the worst case the
> operations could just ignore those arguments; from the user’s viewpoint,
> that would keep the interface simple.
>
>> Only thing I wonder about whether we should be apply-ing the list, or
>> just passing it as a final arguments.  That would be a change in the
>> calling convention, but all procedures could just be adjusted to take
>> the extra argument.
> It’s a backward-compatible change in the calling convention (nothing
> changes for existing open/close/check methods that accept no keyword
> arguments).
>
>> One question would be how to handle if user passes the extra arguments
>> to a procedure that is not prepared to handle them.  In your approach
>> (apply) it would be a error due to wrong number of arguments, I am not
>> sure whether that is ideal.  Maybe a warning would be better?  Not sure.
>> Opinions?
> Yes, you’d get a wrong-type-arg error, similar to what you have if you
> pass #:random-number 42 in the ‘arguments’ field of a package.
>
> It’s not ideal in terms of error reporting, but maybe good enough.
>
> Another option would be for instance to always pass a key/value pair;
> open/close/check methods would have to look up in that list for relevant
> keys and could provide better error reporting, but the downside is extra
> boilerplate in those methods.
>
> Ludo’.

Message #40 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, Tomas Volf <~@wolfsden.cz>,
 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Tue, 06 May 2025 11:26:12 +0200

Hi 45mg,

Did you have a chance to look into the proposed change below?

Thanks,
Ludo’.

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

> Hi,
>
> 45mg <45mg.writes <at> gmail.com> skribis:
>
>> We use <mapped-device-type> records to represent the different types of
>> mapped devices (LUKS, RAID, LVM).  When variables are defined for these
>> records, we can distinguish them with eq?; when they are created by
>> procedures, like luks-device-mapping-with-options, this does not work.
>> Therefore, add a 'name' field to <mapped-device-type> to distinguish
>> them.
>>
>> * gnu/system/mapped-devices.scm (<mapped-device-type>): Add name field.
>> (luks-device-mapping, raid-device-mapping, lvm-device-mapping):
>> Initialize it with appropriate values for each of these types.
>> * gnu/system.scm (operating-system-bootloader-crypto-devices): Use it to
>> identify LUKS mapped devices.
>>
>> Change-Id: I4c85824f74316f07239374d9df6c007dd47a9d0c
>> ---
>>
>> I've tested this on my system; in conjunction with [1], I can finally mount my
>> LUKS volume with the no_read_workqueue and no_write_workqueue flags.
>>
>> [1] [bug#77499] [PATCH] mapped-devices/luks: Support extra options.
>> https://issues.guix.gnu.org/77499
>> https://yhetil.org/guix/fb637872bd14abe305d810b9d32e0db290b26dd6.1743702237.git.45mg.writes <at> gmail.com/
>
> You can add a “Fixes” line for the bug it fixes.
>
>>      (let* ((luks-devices (filter (lambda (m)
>> -                                   (eq? luks-device-mapping
>> -                                        (mapped-device-type m)))
>> +                                   (eq? (mapped-device-kind-name
>> +                                         (mapped-device-type m))
>> +                                        'luks))
>
> [...]
>
>>  (define-record-type* <mapped-device-type> mapped-device-kind
>>    make-mapped-device-kind
>>    mapped-device-kind?
>> +  (name      mapped-device-kind-name)
>
> As a rule of thumb, I think comparing by identity (as was done before)
> is more robust and cleaner: that avoids the whole problem of this
> secondary name space where name clashes may occur involuntarily.
>
> But this problem the patch fixes was introduced by
> ‘luks-device-mapping-with-options’ I believe, which returns a new device
> type.
>
> If we take a step back, I wonder if a better solution would not be to
> add an ‘arguments’ field to <mapped-device>, following the same pattern
> as <package>.
>
> Here’s a preliminary patch to illustrate that:
>
> diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
> index 72da8e55d3..17c2e6f6bf 100644
> --- a/gnu/system/linux-initrd.scm
> +++ b/gnu/system/linux-initrd.scm
> @@ -229,7 +229,8 @@ (define* (raw-initrd file-systems
>                    (targets (mapped-device-targets md))
>                    (type    (mapped-device-type md))
>                    (open    (mapped-device-kind-open type)))
> -             (open source targets)))
> +             (apply open source targets
> +                    (mapped-device-arguments md))))
>           mapped-devices))
>  
>    (define file-system-scan-commands
> diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
> index 667a495570..29d0dc95cf 100644
> --- a/gnu/system/mapped-devices.scm
> +++ b/gnu/system/mapped-devices.scm
> @@ -83,6 +83,8 @@ (define-record-type* <mapped-device> %mapped-device
>    (source    mapped-device-source)                ;string | list of strings
>    (targets   mapped-device-targets)               ;list of strings
>    (type      mapped-device-type)                  ;<mapped-device-kind>
> +  (arguments mapped-device-arguments              ;list passed to open/close/check
> +             (default '()))
>    (location  mapped-device-location
>               (default (current-source-location)) (innate)))
>  
> @@ -128,13 +130,16 @@ (define device-mapping-service-type
>     'device-mapping
>     (match-lambda
>       (($ <mapped-device> source targets
> -                         ($ <mapped-device-type> open close modules))
> +                         ($ <mapped-device-type> open close modules)
> +                         arguments)
>        (shepherd-service
>         (provision (list (symbol-append 'device-mapping- (string->symbol (string-join targets "-")))))
>         (requirement '(udev))
>         (documentation "Map a device node using Linux's device mapper.")
> -       (start #~(lambda () #$(open source targets)))
> -       (stop #~(lambda _ (not #$(close source targets))))
> +       (start #~(lambda ()
> +                  #$(apply open source targets arguments)))
> +       (stop #~(lambda _
> +                 (not #$(apply close source targets arguments))))
>         (modules (append %default-modules modules))
>         (respawn? #f))))
>     (description "Map a device node using Linux's device mapper.")))
> diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
> index 23eb215561..8a56f1cc63 100644
> --- a/guix/scripts/system.scm
> +++ b/guix/scripts/system.scm
> @@ -680,9 +680,10 @@ (define (check-mapped-devices os)
>                              (mapped-device-type md))))
>                  ;; We expect CHECK to raise an exception with a detailed
>                  ;; '&message' if something goes wrong.
> -                (check md
> +                (apply check md
>                         #:needed-for-boot? (needed-for-boot? md)
> -                       #:initrd-modules initrd-modules)))
> +                       #:initrd-modules initrd-modules
> +                       (mapped-device-arguments md))))
>              (operating-system-mapped-devices os)))
>  
>  (define (check-initrd-modules os)
>
>
> WDYT?
>
> Ludo’.

Message #43 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, 45mg
 <45mg.writes <at> gmail.com>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, Tomas Volf <~@wolfsden.cz>,
 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Fri, 09 May 2025 07:56:04 +0000

Hi Ludovic,

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

> Hi 45mg,
>
> Did you have a chance to look into the proposed change below?
>
> Thanks,
> Ludo’.

Sorry for the delay, I've been quite busy lately.

If I understand correctly, your proposal would make
luks-device-mapping-with-options obsolete, so users would have to change
from this:

--8<---------------cut here---------------start------------->8---
(mapped-device
 (source "/dev/sdb1")
 (target "data")
 (type (luks-device-mapping-with-options
        #:allow-discards? #t)))
--8<---------------cut here---------------end--------------->8---

to this:

--8<---------------cut here---------------start------------->8---
(mapped-device
 (source "/dev/sdb1")
 (target "data")
 (type luks-device-mapping)
 (arguments (list #:allow-discards? #t)))
--8<---------------cut here---------------end--------------->8---

Is that correct? Just trying to make sure I understand your idea.

Regarding different arguments for open, close and check methods - I
think it's fine to have a single keyword argument list like above, and
then, like you suggested, have the methods only respect the keywords
that are relevant to them (use `#:allow-other-keys #:rest keys` in the
define* form). Like you said, this would lead to extra boilerplate
(though we could reduce this by writing a procedure to inspect `keys`
for a given keyword), but I think it would be more robust.

At any rate, I should be able to work on this within two weeks at most;
feel free to ping me if I don't send a v2 by then.

Message #46 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, Tomas Volf <~@wolfsden.cz>,
 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Fri, 09 May 2025 10:38:51 +0200

Hi,

45mg <45mg.writes <at> gmail.com> writes:

> Sorry for the delay, I've been quite busy lately.

No worries.

> If I understand correctly, your proposal would make
> luks-device-mapping-with-options obsolete, so users would have to change
> from this:
>
> --8<---------------cut here---------------start------------->8---
> (mapped-device
>  (source "/dev/sdb1")
>  (target "data")
>  (type (luks-device-mapping-with-options
>         #:allow-discards? #t)))
> --8<---------------cut here---------------end--------------->8---
>
> to this:
>
> --8<---------------cut here---------------start------------->8---
> (mapped-device
>  (source "/dev/sdb1")
>  (target "data")
>  (type luks-device-mapping)
>  (arguments (list #:allow-discards? #t)))
> --8<---------------cut here---------------end--------------->8---
>
> Is that correct? Just trying to make sure I understand your idea.

Yes.  But we can keep ‘luks-device-mapping-with-options’ for a while and
mark it as deprecated.

> At any rate, I should be able to work on this within two weeks at most;
> feel free to ping me if I don't send a v2 by then.

Excellent.  Feel free to start from the patch I sent.

Thank you!

Ludo’.

Message #49 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Quentin Vincent <quentin_vincent <at> disroot.org>
To: 70826 <at> debbugs.gnu.org
Subject: luks-device-mapping-with-options breaks bootloader
Date: Sun, 15 Jun 2025 06:50:02 +0200

[Message part 1 (text/plain, inline)]

Hi,

I independently ran into and fixed this issue myself, but seeing Ludo's
patch, I'll probably run with that.

@Ludo: I tested your patch on my setup (inside a VM); it needed a few small
fixups (an export was missing and the (apply ...) wasn't working for
some reason), but other than that it worked for me.  I was able to add a
#:key-file parameter to my LUKS device with your patch.  You just need
to apply my fixup on top of your patch.

Cheers,
Quentin

[0001-system-Make-arguments-for-mapped-device-type-s-work.patch (text/x-patch, inline)]

From 4d507479467c6721f911e5776b4bf6b5709b9846 Mon Sep 17 00:00:00 2001
From: Quentin Vincent <quentin_vincent <at> disroot.org>
Date: Sat, 14 Jun 2025 13:20:22 +0200
Subject: [PATCH] system: Make arguments for <mapped-device-type>s work

Change-Id: Ia5e14d0eb516cb20bee00a270cb3446bfdfabf90
---
 gnu/system/linux-initrd.scm   | 12 ++++++------
 gnu/system/mapped-devices.scm |  1 +
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 17c2e6f6bf..0484c899fa 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -225,12 +225,12 @@ (define* (raw-initrd file-systems
   (define device-mapping-commands
     ;; List of gexps to open the mapped devices.
     (map (lambda (md)
-           (let* ((source  (mapped-device-source md))
-                  (targets (mapped-device-targets md))
-                  (type    (mapped-device-type md))
-                  (open    (mapped-device-kind-open type)))
-             (apply open source targets
-                    (mapped-device-arguments md))))
+           (let* ((source    (mapped-device-source md))
+                  (targets   (mapped-device-targets md))
+                  (type      (mapped-device-type md))
+                  (open      (mapped-device-kind-open type))
+                  (arguments (mapped-device-arguments md)))
+             (apply open source targets arguments)))
          mapped-devices))
 
   (define file-system-scan-commands
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 29d0dc95cf..f8ec178ba8 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -51,6 +51,7 @@ (define-module (gnu system mapped-devices)
             mapped-device-target
             mapped-device-targets
             mapped-device-type
+            mapped-device-arguments
             mapped-device-location
 
             mapped-device-kind
-- 
2.49.0

Message #52 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, Tomas Volf <~@wolfsden.cz>,
 70826 <at> debbugs.gnu.org
Subject: Re: [PATCH] system: Allow distinguishing <mapped-device-type>s.
Date: Sun, 06 Jul 2025 17:04:29 +0200

Hello 45mg and all,

I polished the patch I had sent in this thread:

  https://codeberg.org/guix/guix/pulls/1048

As usual, feedback welcome!

Ludo’.

Message #57 received at 70826-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>, Tomas Volf <~@wolfsden.cz>,
 70826-done <at> debbugs.gnu.org
Subject: Re: bug#70826: [PATCH] system: Allow distinguishing
 <mapped-device-type>s.
Date: Fri, 18 Jul 2025 01:22:43 +0200

Hi,

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

> Hello 45mg and all,
>
> I polished the patch I had sent in this thread:
>
>   https://codeberg.org/guix/guix/pulls/1048

Pushed as 14c8728f0d812ea2c396b3c0564fa8da1202f430.

Ludo’.

Message #60 received at 70826 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 70826 <at> debbugs.gnu.org,
	45mg <45mg.writes <at> gmail.com>
Cc: , soeren <at> soeren-tempel.net, Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>,
 Ludovic Courtès <ludo <at> gnu.org>, 77499 <at> debbugs.gnu.org,
 Sisiutl <sisiutl <at> egregore.fun>, Hilton Chain <hako <at> ultrarare.space>, gmail.com,
 Tomas Volf <~@wolfsden.cz>
Subject: [PATCH] mapped-devices/luks: Support extra options.
Date: Sat,  9 Aug 2025 17:40:33 +0530

Allow passing extra options to the 'cryptsetup open' command.

* gnu/system/mapped-devices.scm (open-luks-device)
[#:extra-options]: New argument.
* doc/guix.texi (Mapped Devices): Document it.
* gnu/tests/install.scm (%test-encrypted-root-extra-options-os): New
test for it, as well as the previously untested #:allow-discards?
option.
(%encrypted-root-extra-options-os): New os declaration for the test.

Change-Id: Ia9fd129d1c66cbf27abdd3064d59188083465247
---

Took into account Maxim's review. Also, luks-device-mapping-with-options is
now deprecated [1], so instead use the 'arguments' field of
luks-device-mapping.

[1] https://codeberg.org/guix/guix/pulls/1048

 doc/guix.texi                 | 21 +++++++++++
 gnu/system/mapped-devices.scm | 19 ++++++----
 gnu/tests/install.scm         | 68 +++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index bffaeb5bbc..4bb4f50200 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18731,6 +18731,27 @@ Mapped Devices
 file system level operations visible on the physical device.  For more
 information, refer to the description of the @code{--allow-discards}
 option in the @code{cryptsetup-open(8)} man page.
+
+@item #:extra-options
+@code{extra-options} may be used to specify a list of additional
+command-line options for the @code{cryptsetup open} command.  See the
+@code{cryptsetup-open(8)} man page for a list of supported options.
+
+For example, here is how you could specify the
+@option{--perf-no_read_workqueue} and @option{--perf-no_write_workqueue}
+options, along with @option{--allow-discards}:
+
+@lisp
+(mapped-device
+(source "/dev/sdb1")
+(target "data")
+(type (type luks-device-mapping)
+      (arguments '(#:allow-discards? #t
+                   #:extra-options
+                   ("--perf-no_read_workqueue"
+                    "--perf-no_write_workqueue")))))
+@end lisp
+
 @end table
 @end defvar
 
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index b0a6beef28..034956c616 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -200,10 +200,12 @@ (define (check-device-initrd-modules device linux-modules location)
 ;;; Common device mappings.
 ;;;
 
-(define* (open-luks-device source targets #:key key-file allow-discards?)
+(define* (open-luks-device source targets
+                           #:key key-file allow-discards? extra-options)
   "Return a gexp that maps SOURCE to TARGET as a LUKS device, using
 'cryptsetup'.  When ALLOW-DISCARDS? is true, the use of discard (TRIM)
-requests is allowed for the underlying device."
+requests is allowed for the underlying device.  EXTRA-OPTIONS is a list of
+additional options to be passed to the 'cryptsetup open' command."
   (with-imported-modules (source-module-closure
                           '((gnu build file-systems)
                             (guix build utils))) ;; For mkdir-p
@@ -244,10 +246,15 @@ (define* (open-luks-device source targets #:key key-file allow-discards?)
              (let ((cryptsetup #$(file-append cryptsetup-static
                                               "/sbin/cryptsetup"))
                    (cryptsetup-flags (cons*
-                                      "open" "--type" "luks" partition #$target
-                                      (if #$allow-discards?
-                                          '("--allow-discards")
-                                          '()))))
+                                      "open" "--type" "luks"
+                                      (append
+                                       (if #$allow-discards?
+                                           '("--allow-discards")
+                                           '())
+                                       (if (pair? '#$extra-options)
+                                           '#$extra-options
+                                           '())
+                                       (list partition #$target)))))
                ;; We want to fallback to the password unlock if the keyfile
                ;; fails.
                (or (and keyfile
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index ec31cf2bdf..c6715484cf 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -68,6 +68,7 @@ (define-module (gnu tests install)
             %test-separate-home-os
             %test-raid-root-os
             %test-encrypted-root-os
+            %test-encrypted-root-extra-options-os
             %test-encrypted-home-os
             %test-encrypted-home-os-key-file
             %test-encrypted-root-not-boot-os
@@ -843,6 +844,73 @@ (define %test-encrypted-root-os
       (run-basic-test %encrypted-root-os command "encrypted-root-os"
                       #:initialization enter-luks-passphrase)))))
 
+
+;;;
+;;; LUKS-encrypted root with extra options: --allow-discards,
+;;; --perf-no_read_workqueue and --perf-no_write_workqueue
+;;;
+
+;; Except for the 'mapped-devices' field, this is exactly the same as
+;; %encrypted-root-os.
+(define-os-with-source (%encrypted-root-extra-options-os
+                        %encrypted-root-extra-options-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)
+                 (targets '("/dev/vdb"))))
+
+    ;; Note: Do not pass "console=ttyS0" so we can use our passphrase prompt
+    ;; detection logic in 'enter-luks-passphrase'.
+
+    (mapped-devices (list (mapped-device
+                            (source (uuid "12345678-1234-1234-1234-123456789abc"))
+                            (target "the-root-device")
+                            (type luks-device-mapping)
+                            (arguments '(#:allow-discards? #t
+                                         #:extra-options
+                                         ("--perf-no_read_workqueue"
+                                          "--perf-no_write_workqueue"))))))
+    (file-systems (cons (file-system
+                          (device "/dev/mapper/the-root-device")
+                          (mount-point "/")
+                          (type "ext4"))
+                        %base-file-systems))
+    (users (cons (user-account
+                  (name "charlie")
+                  (group "users")
+                  (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 %test-encrypted-root-extra-options-os
+  (system-test
+   (name "encrypted-root-extra-options-os")
+   (description
+    "Test basic functionality of an OS installed like one would do by hand,
+with an LUKS-encrypted root partition opened with extra options
+(--allow-discards, --perf-no_read_workqueue and --perf-no_write_workqueue).
+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 ((images (run-install %encrypted-root-extra-options-os
+                                              %encrypted-root-extra-options-os-source
+                                              #:script
+                                              %encrypted-root-installation-script))
+                         (command (qemu-command* images)))
+      (run-basic-test %encrypted-root-os command "encrypted-root-extra-options-os"
+                      #:initialization enter-luks-passphrase)))))
+
 
 ;;;
 ;;; Separate /home on LVM

base-commit: 0697809d64d525b5b9146a57f824641f6f9f81ca
-- 
2.50.1

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.