dracut: Verify multipath config_dir option
PATCH 1/1 dracut: Verify multipath config_dir option
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.
--- modules.d/90multipath/module-setup.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
modules.d/90multipath/module-setup.sh +4 -1
@@ -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/"$//'`/* + inst_multiple -o \ dmsetup \ kpartx \
@@ -90,7 +93,7 @@ install() { /etc/xdrdevices.conf \ /etc/multipath.conf \ /etc/multipath/* \ - /etc/multipath/conf.d/* + $config_dir [[ $hostonly ]] && [[ $hostonly_mode = "strict" ]] && { for_each_host_dev_and_slaves_all add_hostonly_mpath_conf
Signature · 1 line
2.20.1
Patchset imported to github.
Pull request:
<https://github.com/dracutdevs/dracut/compare/master...dracut-mailing-devs:20200121120834.GA23866%40machine1>
Quoted 25 lines · Milan P. Gandhi
The
90multipath/module-setup.shfile currently does not check the dm-multipathconfig_diroption. 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.dCurrently install function of
module-setup.shhas hardcoded the above path, but users could change it withconfig_diroption. So, adding a command to get the directory specified withconfig_diroption 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(-)modules.d/90multipath/module-setup.sh +1
-0@@ -78,6 +78,9 @@ install() { } } + # Include multipath configuration files from path specified with
Quoted 3 lines
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)
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.
And: fall back to /etc/multipath/conf.d if the command fails for some reason?
Martin
Quoted 13 lines
modules.d/90multipath/module-setup.sh +2
-1+ inst_multiple -o \ dmsetup \ kpartx \@@ -90,7 +93,7 @@ install() { /etc/xdrdevices.conf \ /etc/multipath.conf \ /etc/multipath/* \ - /etc/multipath/conf.d/* + $config_dir [[ $hostonly ]] && [[ $hostonly_mode = "strict" ]] && { for_each_host_dev_and_slaves_all add_hostonly_mpath_conf
A message that is not in the archive (sent off-list or not yet received): 2c0c0f100442edd43b10085fa47177b54cd6d0c9.camel-IBi9RG/[email protected]
Quoted 34 lines · Martin Wilck
Quoted 25 lines · Milan P. Gandhi
The
90multipath/module-setup.shfile currently does not check the dm-multipathconfig_diroption. 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.dCurrently install function of
module-setup.shhas hardcoded the above path, but users could change it withconfig_diroption. So, adding a command to get the directory specified withconfig_diroption 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(-)modules.d/90multipath/module-setup.sh +1
-0@@ -78,6 +78,9 @@ install() { } } + # Include multipath configuration files from path specified withQuoted 3 lines
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:
Quoted 6 lines
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.
Quoted 3 lines
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 [...]
Quoted 2 lines
And: fall back to
/etc/multipath/conf.dif 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
Hello Milan,
thanks a lot for your response. I'm fine with your suggestions.
Quoted 11 lines · Milan P. Gandhi
Quoted 5 lines · Martin Wilck
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/multipathto make it more
consistent with the systemctl output for multipathd.service
Well, Fedora made the /usr move, which openSUSE hasn't completed yet. In general IMO using /usr/sbin is fine, distributions can patch this as it fits. But for clarity, the move should be made in a separate patch, so I for your patch, sticking with /sbin is better.
Quoted 7 lines
$ whereis multipath multipath: /usr/sbin/multipath /usr/lib64/multipath [...]Quoted 3 lines
And: fall back to
/etc/multipath/conf.dif the command fails for some
reason?
Quoted 6 lines
The 'multipath -t' command runs successfully even when multipathd itself
is stopped. Andconfig_diroption is one of the built-in parameters, so
if users do not modify it then it is automatically set to/etc/multipath/conf.d
Right, that makes sense. If "multipath -t" fails, something is really fishy and we should probably error out.
Quoted 3 lines
I can add a check to verify if the
config_dirparameter read from multipath -t is an actual directory, and if not, fall back to above default path -/etc/multipath/conf.d
I'm not sure about that. If config_dir exists and is not a directory, we should error out. If it doesn't exist, we should just skip it.
Btw, while you are at it: There's also the "multipath_dir" config option that specifies the location of the "plugin" libraries for multipath. This stuff is currently installed via
inst_libdir_file "libmultipath*" "multipath/"
But in theory at least, users can change this directory, and if dracut supports a modified "config_dir", it might as well support a modified "multipath_dir." You may argue that that's unlikely to be modified, but the same can be said about config_dir. Just a suggestion.
Regards
Martin
A message that is not in the archive (sent off-list or not yet received): cc660dff208809a5511ab837f4d781d8faeae442.camel-IBi9RG/[email protected]
Quoted 39 lines · Martin Wilck
Hello Milan,
thanks a lot for your response. I'm fine with your suggestions.
Quoted 11 lines · Milan P. Gandhi
Quoted 5 lines · Martin Wilck
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/multipathto make it more
consistent with the systemctl output for multipathd.serviceWell, Fedora made the /usr move, which openSUSE hasn't completed yet. In general IMO using
/usr/sbinis fine, distributions can patch this as it fits. But for clarity, the move should be made in a separate patch, so I for your patch, sticking with /sbin is better.Quoted 7 lines
$ whereis multipath multipath: /usr/sbin/multipath /usr/lib64/multipath [...]Quoted 3 lines
And: fall back to
/etc/multipath/conf.dif the command fails for some
reason?Quoted 6 lines
The 'multipath -t' command runs successfully even when multipathd itself
is stopped. Andconfig_diroption is one of the built-in parameters, so
if users do not modify it then it is automatically set to/etc/multipath/conf.dRight, that makes sense. If "multipath -t" fails, something is really fishy and we should probably error out.
Quoted 6 lines
Quoted 3 lines
I can add a check to verify if the
config_dirparameter read from multipath -t is an actual directory, and if not, fall back to above default path -/etc/multipath/conf.dI'm not sure about that. If
config_direxists and is not a directory, we should error out. If it doesn't exist, we should just skip it.
Thanks Martin!
I have just now sent an updated patch with the changes.
Quoted 11 lines
Btw, while you are at it: There's also the "
multipath_dir" config option that specifies the location of the "plugin" libraries for multipath. This stuff is currently installed via
inst_libdir_file"libmultipath*" "multipath/"But in theory at least, users can change this directory, and if dracut supports a modified "
config_dir", it might as well support a modified "multipath_dir." You may argue that that's unlikely to be modified, but the same can be said aboutconfig_dir. Just a suggestion.
Users can change "multipath_dir" config option as well, but currently there is no check for this option. I will soon send a patch to verify "multipath_dir" option as well.
Regards,
Milan.