Package: guix-patches;
Reported by: 45mg <45mg.writes <at> gmail.com>
Date: Thu, 3 Apr 2025 17:48:01 UTC
Severity: normal
Tags: patch
Message #8 received at 77499 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: 45mg <45mg.writes <at> gmail.com> Cc: soeren <at> soeren-tempel.net, Sisiutl <sisiutl <at> egregore.fun>, Ludovic Courtès <ludo <at> gnu.org>, 77499 <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: Sat, 26 Apr 2025 22:16:43 +0900
Hi,
45mg <45mg.writes <at> 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.
> * 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.
Sounds good.
> Change-Id: Ia9fd129d1c66cbf27abdd3064d59188083465247
> ---
> CCing everyone who worked on the allow-discards option - this change is very
> similar.
>
> %encrypted-root-extra-options-os is copied from %encrypted-root-os; only
> the mapped-devices field is changed. I wish I could avoid this code
> duplication by having `(inherit %encrypted-root-os)` in the os
> definition, but when I do that, the test fails with this error in the
> build log:
>
> /mnt/etc/config.scm:1:100: error: %encrypted-root-os: unbound variable
>
> Any chance you Guile wizards know how to make this work?
I think I've probably banged my head on this at some point but don't
have an immediate idea.
>
> doc/guix.texi | 20 ++++++++++-
> gnu/system/mapped-devices.scm | 25 ++++++++-----
> gnu/tests/install.scm | 68 +++++++++++++++++++++++++++++++++++
> 3 files changed, 104 insertions(+), 9 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index bcb1f9d9cf..9cd1304522 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -18461,7 +18461,7 @@ Mapped Devices
> @code{dm-crypt} Linux kernel module.
> @end defvar
>
> -@deffn {Procedure} luks-device-mapping-with-options [#:key-file #:allow-discards?]
> +@deffn {Procedure} luks-device-mapping-with-options [#:key-file #:allow-discards? #:extra-options]
Was there a way to break a line in Texinfo?
> Return a @code{luks-device-mapping} object, which defines LUKS block
> device encryption using the @command{cryptsetup} command from the
> package with the same name. It relies on the @code{dm-crypt} Linux
> @@ -18492,6 +18492,24 @@ Mapped Devices
> information, refer to the description of the @code{--allow-discards}
> option in the @code{cryptsetup-open(8)} man page.
>
> +@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
> +@code{--perf-no_read_workqueue} and @code{--perf-no_write_workqueue}
> +options, along with @code{--allow-discards}:
For the command-line options, you can use @option{...}
(see: (info "(texinfo) @option")).
> +
> +@lisp
> +(mapped-device
> + (source "/dev/sdb1)
> + (target "data)
Your strings are double quoted only on the left side.
> + (type (luks-device-mapping-with-options
> + #:allow-discards? #t
> + #:extra-options '("--perf-no_read_workqueue"
> + "--perf-no_write_workqueue"))))
> +@end lisp
> +
> @end deffn
>
> @defvar raid-device-mapping
> diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
> index 667a495570..520ade9ef8 100644
> --- a/gnu/system/mapped-devices.scm
> +++ b/gnu/system/mapped-devices.scm
> @@ -194,10 +194,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
> @@ -238,10 +240,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
> @@ -290,7 +297,8 @@ (define luks-device-mapping
> ((gnu build file-systems)
> #:select (find-partition-by-luks-uuid system*/tty))))))
>
> -(define* (luks-device-mapping-with-options #:key key-file allow-discards?)
> +(define* (luks-device-mapping-with-options
> + #:key key-file allow-discards? extra-options)
> "Return a luks-device-mapping object with open modified to pass the arguments
> into the open-luks-device procedure."
> (mapped-device-kind
> @@ -298,7 +306,8 @@ (define* (luks-device-mapping-with-options #:key key-file allow-discards?)
> (open (λ (source targets)
> (open-luks-device source targets
> #:key-file key-file
> - #:allow-discards? allow-discards?)))))
> + #:allow-discards? allow-discards?
> + #:extra-options extra-options)))))
>
> (define (open-raid-device sources targets)
> "Return a gexp that assembles SOURCES (a list of devices) to the RAID device
> diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
> index a837637b18..fd9f17eb4d 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-with-options
> + #: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)))))
Looks good to me. I haven't tried running it yet; if you send a v2 with
the small problem I've seen above I'll happily try it and if it passes
merge it.
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.