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

Previous Next

Full log

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: tsmish <tsymsh <at> gmail.com>
Cc: 41143 <at> debbugs.gnu.org
Subject: Re: [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list
 of strings
Date: Wed, 09 Sep 2020 22:38:34 +0200

Hi Mikhail,

Sorry for the very late reply!  Vacations came by, and by now this entry
is at the bottom of the patch tracker.  :-)

People repeatedly ask for LVM support, so I guess you’ll make them all
happy!  Great you got it into shape.

tsmish <tsymsh <at> gmail.com> skribis:

> (let ...) stuff should be in function, but I don't know in which
> module it should go.
> Code is somewhat untested, proceed with caution.
>
> ---
>  gnu/services/base.scm         |  5 ++++-
>  gnu/system.scm                | 13 ++++++++-----
>  gnu/system/mapped-devices.scm |  2 +-
>  3 files changed, 13 insertions(+), 7 deletions(-)

Side note: We’ll a commit log that follows our conventions¹ but that’s
something I can help with.

¹ https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html

> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
> index 0c154d1c4e..3d09e8220c 100644
> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -408,7 +408,10 @@ FILE-SYSTEM."
>  (define (mapped-device->shepherd-service-name md)
>    "Return the symbol that denotes the shepherd service of MD, a
> <mapped-device>."
>    (symbol-append 'device-mapping-
> -                 (string->symbol (mapped-device-target md))))
> +                 (string->symbol (string-join
> +                                  (let ((t (mapped-device-target md)))
> +                                    (if (list? t) t (list t)))
> +                                  "-"))))

To avoid duplicating the (if (list? t) …) everywhere, I propose instead
the following approach:

  1. Rename ‘target’ to ‘targets’ (plural) and likewise for the
     accessor, and agree that it always contains a list;

  2. Rename ‘mapped-device’ to ‘%mapped-device’ and add a
     ‘mapped-device’ backward-compatibility macro that allows for a
     ‘target’ (singular) field and automatically turns its value into a
     list.  See the ‘origin’ macro in (guix packages) for an example of
     how to do that (that macro allows users to specify ‘sha256’ instead
     of ‘hash’).

  3. Add a deprecated ‘mapped-device-target’ (singular) that returns the
     first element returned by ‘mapped-device-targets’.

We’ll need to adjust doc/guix.texi accordingly.

How does that sound?

Thanks,
Ludo’.

Previous Next

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