Re: [PATCH] dracut: Verify multipath config_dir option

On Wed, Jan 29, 2020 at 10:49:21AM +0100, Martin Wilck wrote:
> On Tue, 2020-01-21 at 17:38 +0530, Milan P. Gandhi wrote:
> > The 90multipath/module-setup.sh file currently does not check the
> > dm-multipath config_dir option. This option in multipath.conf file is
> > used to specify a custom directory/path that contains the multipath
> > configuration files. It's default value is /etc/multipath/conf.d
> > 
> > Currently install function of module-setup.sh has hardcoded the above
> > path, but users could change it with config_dir option. So, adding a
> > command to get the directory specified with config_dir option and add
> > these configuration files to the initial ram disk image.
> > 
> > Signed-off-by: Milan P. Gandhi <[email protected]>
> > ---
> >  modules.d/90multipath/module-setup.sh | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/modules.d/90multipath/module-setup.sh
> > b/modules.d/90multipath/module-setup.sh
> > index 1f6a55ec..f7c521c1 100755
> > --- a/modules.d/90multipath/module-setup.sh
> > +++ b/modules.d/90multipath/module-setup.sh
> > @@ -78,6 +78,9 @@ install() {
> >          }
> >      }
> >  
> > +    # Include multipath configuration files from path specified with
> > config_dir
> > +    config_dir=`/usr/sbin/multipath -t|grep -i config_dir|awk
> > '{print $2}'|sed -e 's/^"//' -e 's/"$//'`/*
> 
> No need to use 'grep -i', multipath configuration directives are case-
> sensitive. At least one member of that grep|awk|sed pipe can be
> dropped. Actually, all of them, for example like this:
> 
> while read _k _v; do 
>    if [[ "$_k" = config_dir ]]; then 
>        _v=${_v#\"}
>        config_dir=${_v%\"}
>    fi
> done < <(multipath -t)

Thank you for your suggestion, Martin. I will update the patch, do some
more testing with it and send a V2 with changes.

> The path (/usr/sbin/multipath) is inconsistent with what we use in
> multipathd.service (/sbin/multipath). I'm not against changing this to
> /usr, but we should be consistent.

I used (/usr/sbin/multipath) because location of multipath binary is
shown in /usr/sbin. I will change it to /sbin/multipath to make it more
consistent with the systemctl output for multipathd.service

$ whereis multipath
multipath: /usr/sbin/multipath /usr/lib64/multipath [...]
 
> And: fall back to /etc/multipath/conf.d if the command fails for some
> reason?

The 'multipath -t' command runs successfully even when multipathd itself
is stopped. And config_dir option is one of the built-in parameters, so
if users do not modify it then it is automatically set to 
/etc/multipath/conf.d

I can add a check to verify if the config_dir parameter read from
multipath -t is an actual directory, and if not, fall back to above
default path - /etc/multipath/conf.d