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

[Benjamin Marzinski]

On Thu, Nov 11, 2021 at 11:44:44AM +0000, Martin Wilck wrote:
> 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 at 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).

Sure. Thanks.

> 
> > ---
> >  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.

Sure.

-Ben

> Regards,
> Martin
> 
>