GNU bug report logs - #77499
[PATCH] mapped-devices/luks: Support extra options.

Previous Next

Full log

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

From: Maxim Cournoyer <maxim <at> guixotic.coop>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Gabriel Wicki <gabriel <at> erlikon.ch>, soeren <at> soeren-tempel.net,
 Tadhg McDonald-Jensen <tadhgmister <at> gmail.com>,
 Ludovic Courtès <ludo <at> gnu.org>, 77499 <at> debbugs.gnu.org,
 gmail.com <at> debbugs.gnu.org, Sisiutl <sisiutl <at> egregore.fun>,
 70826 <at> debbugs.gnu.org, Hilton Chain <hako <at> ultrarare.space>,
 Tomas Volf <~@wolfsden.cz>
Subject: Re: [bug#77499] [PATCH] mapped-devices/luks: Support extra options.
Date: Thu, 14 Aug 2025 10:26:15 +0900

Hi,

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

> 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)

I guess it'd be nicer if the default was '() for extra-options, then you
don't need to check its value later (unless if you want to validate the
inputs).

>    "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
> +                                           '())

Then the if can be removed, else turned into some input validation like:

--8<---------------cut here---------------start------------->8---
(unless (pair? '#$extra-options)
 (error "invalid value for #:extra-options argument of `open-luks-device'"))
--8<---------------cut here---------------end--------------->8---

I haven't reviewed where this gets used (I assume in the early boot);
perhaps it could be possible to use (guix diagnostics) as well if it's
already imported there to produce a nicer error message.

Could you send a revised version doing the above?

-- 
Thanks,
Maxim

Previous Next

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