Re: [dm-devel] [PATCH v2 7/9] libmultipath: deprecate file and directory config options

[Martin Wilck <martin.wilck@suse.com>]

On Wed, 2021-11-10 at 19:06 -0600, Benjamin Marzinski wrote:
> Having multipath able to select pathnames for the files and
> directories
> it needs causes unnecessary maintainer headaches. Mark these as
> deprecated, but still support them for now.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

I would have preferred to start ignoring these options right away
(spitting out warnings). After all, this is upstream, we don't have to 
take care of long-term-support users (distros can keep the old behavior
if they want), and experience shows that depreciation warnings are
ignored until the deprecated feature is actually removed. But if you
prefer doing it this way, fine.

Let's agree to remove these options soon, i.e. with the official
release after the next one (0.8.9, presumably).

> ---
>  libmultipath/dict.c        | 40 +++++++++++++++++++++++++++++-------
> --
>  multipath/multipath.conf.5 |  5 +++++
>  2 files changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index 149d3348..1b4e1106 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -268,6 +268,15 @@ def_ ## option ## _handler (struct config *conf,
> vector strvec,            \
>         return function (strvec, &conf->option, file,
> line_nr);         \
>  }
>  
> +#define declare_def_warn_handler(option,
> function)                     \
> +static
> int                                                             \
> +def_ ## option ## _handler (struct config *conf, vector
> strvec,                \
> +                           const char *file, int
> line_nr)              \
> +{                                                                   
>    \
> +       condlog(2, "%s line %d, \"" #option "\" is deprecated and
> will be disabled in a future release", file,
> line_nr);                                \
> +       return function (strvec, &conf->option, file,
> line_nr);         \
> +}
> +
>  #define declare_def_range_handler(option, minval,
> maxval)                      \
>  static
> int                                                             \
>  def_ ## option ## _handler (struct config *conf, vector
> strvec,         \
> @@ -284,6 +293,17 @@ snprint_def_ ## option (struct config *conf,
> struct strbuf *buff,  \
>         return function(buff, conf-
> >option);                            \
>  }
>  
> +#define declare_def_snprint_non_defstr(option, function,
> value)                \
> +static
> int                                                             \
> +snprint_def_ ## option (struct config *conf, struct strbuf
> *buff,      \
> +                       const void
> *data)                               \
> +{                                                                   
>    \
> +       static const char *s =
> value;                                   \
> +       if (!conf->option || strcmp(conf->option, s) ==
> 0)              \
> +               return
> 0;                                               \
> +       return function(buff, conf-
> >option);                            \
> +}

I'd actually print the value here, even if it's empty or equal to the
default. That would be helpful in the future too, when these options
are removed. This way customers can verify the settings multipathd is
using by default.

Regards,
Martin




--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel