[bug#77499,v3] mapped-devices/luks: Support extra options.

Message ID b1b89a7997c492def17e26d874d90a6d78a25c06.1758024769.git.45mg.writes@gmail.com
State New
Headers
Series [bug#77499,v3] mapped-devices/luks: Support extra options. |

Commit Message

45mg Sept. 16, 2025, 12:17 p.m. UTC
Allow passing extra options to the 'cryptsetup open' command.

* gnu/system/mapped-devices.scm (luks-device-mapping-with-options):
[#:extra-options]: New argument.
(open-luks-device): Use it.
(check-luks-device): Validate it.
* 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: Ibbc3cf4f2ee4d49099a3155a015f54d319515663
---

Add default value '() as suggested by Maxim. Also, add the suggested
validation in check-luks-device.

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


base-commit: ea4a3af73940e3fd578326510eccb2d5747352b4
  

Comments

Ludovic Courtès Sept. 16, 2025, 1:55 p.m. UTC | #1
Hello,

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

> Allow passing extra options to the 'cryptsetup open' command.
>
> * gnu/system/mapped-devices.scm (luks-device-mapping-with-options):
> [#:extra-options]: New argument.
> (open-luks-device): Use it.
> (check-luks-device): Validate it.
> * 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: Ibbc3cf4f2ee4d49099a3155a015f54d319515663

[...]

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

Instead of repeating the keyword name, maybe you can write:

  List of additional command-line options for …

> +@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")))))

The indentation and syntax are incorrect, if I’m not mistaken.

Otherwise LGTM!

Could you send an updated version?

Thanks,
Ludo’.

PS: Use of guix-patches will be discontinued at the end of the year.  I
    would encourage you to try out the pull request workflow on Codeberg
    and to report any questions or issues you may have.
  
45mg Sept. 16, 2025, 3:29 p.m. UTC | #2
Hi,

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

> Hello,
>
> 45mg <45mg.writes@gmail.com> writes:
>
>> Allow passing extra options to the 'cryptsetup open' command.
>>

[...]

>> +@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
>
> Instead of repeating the keyword name, maybe you can write:
>
>   List of additional command-line options for …
>

OK.

>> +@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")))))
>
> The indentation and syntax are incorrect, if I’m not mistaken.

Is it? `indent-region` in Emacs (C-M-\) leaves it unchanged. How should I
indent it instead?

As for the syntax,
guix shell -D guix --pure -- make check-system TESTS=encrypted-root-extra-options-os
passes.

> Otherwise LGTM!
>
> Could you send an updated version?
>
> Thanks,
> Ludo’.
>
> PS: Use of guix-patches will be discontinued at the end of the year.  I
>     would encourage you to try out the pull request workflow on Codeberg
>     and to report any questions or issues you may have.
  

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index 0924aebf4a..74a6367e43 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18774,6 +18774,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..a2d49c55a5 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -43,6 +43,7 @@  (define-module (gnu system mapped-devices)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 optargs)
   #:use-module (ice-9 format)
   #:export (%mapped-device
             mapped-device
@@ -200,10 +201,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 +247,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
@@ -271,6 +279,16 @@  (define* (check-luks-device md #:key
   "Ensure the source of MD is valid."
   (let ((source   (mapped-device-source md))
         (location (mapped-device-location md)))
+    (let-keywords (mapped-device-arguments md) #t
+                  (key-file allow-discards extra-options)
+      (unless (pair? extra-options)
+        (raise (make-compound-condition
+                (formatted-message (G_ "invalid value ~s for #:extra-options \
+argument of `open-luks-device'")
+                                   extra-options)
+                (condition
+                 (&error-location
+                  (location (source-properties->location location))))))))
     (or (not (zero? (getuid)))
         (if (uuid? source)
             (match (find-partition-by-luks-uuid (uuid-bytevector source))
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