GNU bug report logs - #41143
[PATCH 1/2] Add 'lvm-device-mapping'

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mikhail Tsykalov <tsymsh <at> gmail.com>
Cc: 41143 <at> debbugs.gnu.org
Subject: [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings.
Date: Sun, 04 Oct 2020 12:28:32 +0200

Hi Mikhail,

Mikhail Tsykalov <tsymsh <at> gmail.com> skribis:

> * gnu/system/mapped-devices.scm (<mapped-device>): Rename constructor to
> %mapped-device.
> [target]: Remove field.
> [targets]: New field. Adjust users.
> (mapped-device-compatibility-helper, mapped-device): New macros.
> (mapped-device-target): New deprecated procedure.

Thanks for following up.  I think we’re almost done, some comments
below:

> --- a/gnu/system/linux-initrd.scm
> +++ b/gnu/system/linux-initrd.scm
> @@ -196,7 +196,7 @@ upon error."
>      ;; List of gexps to open the mapped devices.
>      (map (lambda (md)
>             (let* ((source (mapped-device-source md))
> -                  (target (mapped-device-target md))
> +                  (target (mapped-device-targets md))

I think we should write ‘targets’ (plural) everywhere.  That can help
avoid confusion IMO.

> -                        #$target)))))
> +                        #$(car target))))))
>  
>  (define (close-luks-device source target)
>    "Return a gexp that closes TARGET, a LUKS device."
>    #~(zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
> -                    "close" #$target)))
> +                    "close" #$(car target))))

As per our coding convention (info "(guix) Data Types and Pattern
Matching"), I’d recommend using ‘match’

  (define (close-luks-device source targets)
    (match targets
      ((target)
       #~(zero? (system* … #$target)))))

That has the added benefit that it errors out if TARGETS is not exactly
a one-element list.

>  (define (close-raid-device sources target)
>    "Return a gexp that stops the RAID device TARGET."
>    #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
> -                    "--stop" #$target)))
> +                    "--stop" #$(car target))))

Same here.

Could you also update “Mapped Devices” in doc/guix.texi to mention the
new ‘targets’ field?

Thanks,
Ludo’.

Previous Next

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