Subject: Re: [bug#64259] [PATCH 2/2] Provide md-array-device-mapping to start MD arrays via UUID or name.

From: Ludovic Courtès <[email-masked]>
Date: Fri, 20 Oct 2023 23:55:30 +0200

Hi,

Felix Lechner <[email-masked]> skribis:

> This commit cures the most precipitous danger for users of MD arrays in GNU
> Guix, namely that their equipment may not boot after a drive failure.

Why would that happen?  Could be because the device names specified in
the ‘source’ field of the mapped device become invalid?

> Adjusts the documentation by erasing any mention of the obsolete
> raid-device-mapping. No one should use that any longer. Ideally, users would
> be a deprecation warning, but I was unable to adapt 'define-deprecated' to
> this use case. Please feel free to make further changes.

If it has to be deprecated then yes, we try and use ‘define-deprecated’.

> Please feel free to insert better changelog messages. I had some difficulty
> meeting the likely expectations of any reviewer. Please also feel free to make
> any other adjustments as needed without checking with me. Thanks!

The reviewer may feel free, sure…  :-)

>  @item source
> -This is either a string specifying the name of the block device to be mapped,
> -such as @code{"/dev/sda3"}, or a list of such strings when several devices
> -need to be assembled for creating a new one.  In case of LVM this is a
> -string specifying name of the volume group to be mapped.
> +This is either a string specifying the name of the block device to be
> +mapped, such as @code{"/dev/sda3"}.  For MD array devices it is either
> +the UUID of the array or a string that is interpreted as the array name
> +(see Mdadm documentation).  In case of LVM it is a string specifying
> +name of the volume group to be mapped.

Instead of “see Mdadm documentation”, could you add a link or a command
to type to access said documentation?  Better yet, an example of what an
mdadm device name or UUID is and how to obtain it would be welcome.

> +(define (open-md-array-device source targets)
> +  "Return a gexp that assembles SOURCE to the MD device
> +TARGET (e.g., \"/dev/md0\"), using 'mdadm'."
> +  (let ((array-selector
> +         (match source
> +           ((? uuid?)
> +            (string-append "--uuid=" (uuid->string source)))
> +           ((? string?)
> +            (string-append "--name=" source))))
> +        (md-device
> +         (match targets
> +           ((target)
> +            target))))
> +    (if (and array-selector md-device)
           ^
This condition is always true.

> +        ;; Use 'mdadm-static' rather than 'mdadm' to avoid pulling its whole
> +        ;; closure (80 MiB) in the initrd when an MD device is needed for boot.
> +        #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
> +                          "--assemble" #$md-device
> +                          "--run"
> +                          #$array-selector))
> +        #f)))
> +
> +(define (close-md-array-device source targets)
> +  "Return a gexp that stops the MD device TARGET."
> +  (match targets
> +    ((target)
> +     #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
> +                       "--stop" #$target)))))
> +
> +(define md-array-device-mapping
> +  ;; The type of MD mapped device.
> +  (mapped-device-kind
> +   (open open-md-array-device)
> +   (close close-md-array-device)))

Instead of renaming and duplicating part of the logic, how about
supporting those new ‘source’ specification right in ‘open-raid-device’?
It would emit a deprecation warning when ‘source’ is a list of strings.

Does the busy wait loop currently in ‘open-raid-device’ need to be
preserved?

Thanks,
Ludo’.