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

Previous Next

Full log

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

From: Mikhail Tsykalov <tsymsh <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 41143 <at> debbugs.gnu.org
Subject: Re: [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list
 of strings
Date: Thu, 24 Sep 2020 19:09:57 +0300

Hi Ludovic,

On 09.09.2020 23:38, Ludovic Courtès wrote:
>> 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’.

While this looks like a good idea, doesn't this break code that 
implements mapped-device and assumes that target is a string. Suddenly 
passing a string to a mapped-device constructor results in a list passed 
to open/close. Also, what functions should do if they expect a string 
but get a list of them? Ignore everything but the first item? Implement 
mandatory check function? Doesn't this change push complexity out of 
mapped-device to implementations of it.

Basically, this change will break all current implementations of 
mapped-devices, so I wanted to hear your thoughts on it before continuing.

Mikhail

Previous Next

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