Now kdump cannt parse the path correctly, if the path contains duplicated "/". Following is an example to explain it detail. path //mnt/var/crash Then the warning will raise, as kdump cannt parse this path correctly. Force rebuild /boot/initramfs-3.19.1kdump.img Rebuilding /boot/initramfs-3.19.1kdump.img df: ‘/mnt///mnt/var/crash’: No such file or directory /sbin/mkdumprd: line 239: [: -lt: unary operator expected kexec: loaded kdump kernel Starting kdump: [OK] Kdump fails to cut out the substring, if the string contains dumplicated "/". Add the new function which it will cut out the substring accurately. Signed-off-by: Minfei Huang <mhuang(a)redhat.com&gt; --- dracut-module-setup.sh | 7 +++---- kdump-lib.sh | 8 ++++++++ mkdumprd | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 4641025..477ede1 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -311,9 +311,8 @@ kdump_install_net() { default_dump_target_install_conf() { local _target _fstype - local _s _t local _mntpoint - local _path _save_path + local _save_path is_user_configured_dump_target && return @@ -334,11 +333,11 @@ default_dump_target_install_conf() echo "$_fstype $_target" >> ${initdir}/tmp/$$-kdump.conf - _path=${_save_path##"$_mntpoint"} + _save_path=$(cut_out_substring $_save_path $_mntpoint) #erase the old path line, then insert the parsed path sed -i "/^path/d" ${initdir}/tmp/$$-kdump.conf - echo "path $_path" >> ${initdir}/tmp/$$-kdump.conf + echo "path $_save_path" >> ${initdir}/tmp/$$-kdump.conf fi } diff --git a/kdump-lib.sh b/kdump-lib.sh index f24f08d..ab01d74 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -188,3 +188,11 @@ is_ipv6_target() _server=${_server:-$_server_tmp} echo $_server | grep -q ":" } +

+cut_out_substring() +{ + # filter out the duplicated "/" + local _main_str=$(echo $1|sed "s#/\{1,\}#/#g") + local _sub_str=$(echo $2|sed "s#/\{1,\}#/#g") + echo ${_main_str#*"$_sub_str"} +} diff --git a/mkdumprd b/mkdumprd index 4d251ba..a8f9cbb 100644 --- a/mkdumprd +++ b/mkdumprd @@ -364,7 +364,7 @@ handle_default_dump_target() _mntpoint=$(get_mntpoint_from_path $SAVE_PATH) _target=$(get_target_from_path $SAVE_PATH) if [ "$_mntpoint" != "/" ]; then - SAVE_PATH=${SAVE_PATH##"$_mntpoint"} + SAVE_PATH=$(cut_out_substring $SAVE_PATH $_mntpoint) _fstype=$(get_fs_type_from_target $_target) if $(is_fs_type_nfs $_fstype); then -- 1.9.3

Hi Minfei, On 04/07/15 at 05:20pm, Minfei Huang wrote: > Now kdump cannt parse the path correctly, if the path contains > duplicated "/". Following is an example to explain it detail. > > path //mnt/var/crash > > Then the warning will raise, as kdump cannt parse this path correctly. > > Force rebuild /boot/initramfs-3.19.1kdump.img > Rebuilding /boot/initramfs-3.19.1kdump.img > df: ‘/mnt///mnt/var/crash’: No such file or directory > /sbin/mkdumprd: line 239: [: -lt: unary operator expected > kexec: loaded kdump kernel > Starting kdump: [OK] > > Kdump fails to cut out the substring, if the string contains dumplicated > "/". > > Add the new function which it will cut out the substring accurately. > > Signed-off-by: Minfei Huang <mhuang(a)redhat.com&gt; > --- > dracut-module-setup.sh | 7 +++---- > kdump-lib.sh | 8 ++++++++ > mkdumprd | 2 +- > 3 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh > index 4641025..477ede1 100755 > --- a/dracut-module-setup.sh > +++ b/dracut-module-setup.sh > @@ -311,9 +311,8 @@ kdump_install_net() { > default_dump_target_install_conf() > { > local _target _fstype > - local _s _t > local _mntpoint > - local _path _save_path > + local _save_path > > is_user_configured_dump_target && return > > @@ -334,11 +333,11 @@ default_dump_target_install_conf() > > echo "$_fstype $_target" >> ${initdir}/tmp/$$-kdump.conf > > - _path=${_save_path##"$_mntpoint"} > + _save_path=$(cut_out_substring $_save_path $_mntpoint) > > #erase the old path line, then insert the parsed path > sed -i "/^path/d" ${initdir}/tmp/$$-kdump.conf > - echo "path $_path" >> ${initdir}/tmp/$$-kdump.conf > + echo "path $_save_path" >> ${initdir}/tmp/$$-kdump.conf > fi > > } > diff --git a/kdump-lib.sh b/kdump-lib.sh > index f24f08d..ab01d74 100755 > --- a/kdump-lib.sh > +++ b/kdump-lib.sh > @@ -188,3 +188,11 @@ is_ipv6_target() > _server=${_server:-$_server_tmp} > echo $_server | grep -q ":" > } > + Serious, this is not good. I remember we have talked about this. You implemented a specific function but give it a generic name. 2 choices to change it: 1) strip_duplicated_slash get a specific name as it really does.

2) cut_out_substring But pass a parameter "/" when call it. Seems it doesn't fit in this case. My opinion is the 1st one, and repost your patch. It should not affect your bind mount patchset. Thanks Baoquan > +cut_out_substring() > +{ > + # filter out the duplicated "/" > + local _main_str=$(echo $1|sed "s#/\{1,\}#/#g") > + local _sub_str=$(echo $2|sed "s#/\{1,\}#/#g") > + echo ${_main_str#*"$_sub_str"} > +} > diff --git a/mkdumprd b/mkdumprd > index 4d251ba..a8f9cbb 100644 > --- a/mkdumprd > +++ b/mkdumprd > @@ -364,7 +364,7 @@ handle_default_dump_target() > _mntpoint=$(get_mntpoint_from_path $SAVE_PATH) > _target=$(get_target_from_path $SAVE_PATH) > if [ "$_mntpoint" != "/" ]; then > - SAVE_PATH=${SAVE_PATH##"$_mntpoint"} > + SAVE_PATH=$(cut_out_substring $SAVE_PATH $_mntpoint) > _fstype=$(get_fs_type_from_target $_target) > > if $(is_fs_type_nfs $_fstype); then > -- > 1.9.3 >

On 04/07/15 at 05:20pm, Minfei Huang wrote: > +cut_out_substring() > +{ > + # filter out the duplicated "/" > + local _main_str=$(echo $1|sed "s#/\{1,\}#/#g") > + local _sub_str=$(echo $2|sed "s#/\{1,\}#/#g") > + echo ${_main_str#*"$_sub_str"} > +} > diff --git a/mkdumprd b/mkdumprd > index 4d251ba..a8f9cbb 100644 > --- a/mkdumprd > +++ b/mkdumprd > @@ -364,7 +364,7 @@ handle_default_dump_target() > _mntpoint=$(get_mntpoint_from_path $SAVE_PATH) > _target=$(get_target_from_path $SAVE_PATH) > if [ "$_mntpoint" != "/" ]; then OK, I got it. You should do it this way. Define function strip_duplicated_slash and filter the suspect string. Then do the cutting out job by old code. Here code would be: SAVE_PATH=strip_duplicated_slash($_mntpoint) SAVE_PATH=${SAVE_PATH##"$_mntpoint"} Very few code change.

> - SAVE_PATH=${SAVE_PATH##"$_mntpoint"} > + SAVE_PATH=$(cut_out_substring $SAVE_PATH $_mntpoint) > _fstype=$(get_fs_type_from_target $_target) > > if $(is_fs_type_nfs $_fstype); then > -- > 1.9.3 >

On 04/08/15 at 03:27pm, Minfei Huang wrote: > On 04/08/15 at 03:10pm, Baoquan He wrote: > > On 04/07/15 at 05:20pm, Minfei Huang wrote: > > > > > > > +cut_out_substring() > > > +{ > > > + # filter out the duplicated "/" > > > + local _main_str=$(echo $1|sed "s#/\{1,\}#/#g") > > > + local _sub_str=$(echo $2|sed "s#/\{1,\}#/#g") > > > + echo ${_main_str#*"$_sub_str"} > > > +} > > > diff --git a/mkdumprd b/mkdumprd > > > index 4d251ba..a8f9cbb 100644 > > > --- a/mkdumprd > > > +++ b/mkdumprd > > > @@ -364,7 +364,7 @@ handle_default_dump_target() > > > _mntpoint=$(get_mntpoint_from_path $SAVE_PATH) > > > _target=$(get_target_from_path $SAVE_PATH) > > > if [ "$_mntpoint" != "/" ]; then > > > > OK, I got it. You should do it this way. Define function > > strip_duplicated_slash and filter the suspect string. Then do the > > cutting out job by old code. > > > > Here code would be: > > SAVE_PATH=strip_duplicated_slash($_mntpoint) > > SAVE_PATH=${SAVE_PATH##"$_mntpoint"} > > > > Very few code change. > > > > Hi, Bao. > > Yes, this is an idea to fix this issue. Since there are several place > which we shall modify, I think wrapping a function is more convenience > and readable. Honestly I don't like it. All about it is duplicated slash. Now what you have done is wrapping SAVE_PATH=${SAVE_PATH##"$_mntpoint"} into a function. I really don't see why that is more readable and necessary.

On 04/07/15 at 05:20pm, Minfei Huang wrote: > Now kdump cannt parse the path correctly, if the path contains > duplicated "/". Following is an example to explain it detail. > > path //mnt/var/crash > > Then the warning will raise, as kdump cannt parse this path correctly. > > Force rebuild /boot/initramfs-3.19.1kdump.img > Rebuilding /boot/initramfs-3.19.1kdump.img > df: ‘/mnt///mnt/var/crash’: No such file or directory > /sbin/mkdumprd: line 239: [: -lt: unary operator expected > kexec: loaded kdump kernel > Starting kdump: [OK] > > Kdump fails to cut out the substring, if the string contains dumplicated > "/". > > Add the new function which it will cut out the substring accurately. > > Signed-off-by: Minfei Huang <mhuang(a)redhat.com&gt; > --- > dracut-module-setup.sh | 7 +++---- > kdump-lib.sh | 8 ++++++++ > mkdumprd | 2 +- > 3 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh > index 4641025..477ede1 100755 > --- a/dracut-module-setup.sh > +++ b/dracut-module-setup.sh > @@ -311,9 +311,8 @@ kdump_install_net() { > default_dump_target_install_conf() > { > local _target _fstype > - local _s _t > local _mntpoint > - local _path _save_path > + local _save_path > > is_user_configured_dump_target && return > > @@ -334,11 +333,11 @@ default_dump_target_install_conf() > > echo "$_fstype $_target" >> ${initdir}/tmp/$$-kdump.conf > > - _path=${_save_path##"$_mntpoint"} > + _save_path=$(cut_out_substring $_save_path $_mntpoint) > > #erase the old path line, then insert the parsed path > sed -i "/^path/d" ${initdir}/tmp/$$-kdump.conf > - echo "path $_path" >> ${initdir}/tmp/$$-kdump.conf > + echo "path $_save_path" >> ${initdir}/tmp/$$-kdump.conf > fi > > } > diff --git a/kdump-lib.sh b/kdump-lib.sh > index f24f08d..ab01d74 100755 > --- a/kdump-lib.sh > +++ b/kdump-lib.sh > @@ -188,3 +188,11 @@ is_ipv6_target() > _server=${_server:-$_server_tmp} > echo $_server | grep -q ":" > } > + > +cut_out_substring() > +{ > + # filter out the duplicated "/" > + local _main_str=$(echo $1|sed "s#/\{1,\}#/#g") > + local _sub_str=$(echo $2|sed "s#/\{1,\}#/#g") I found a simpler way to do this: echo $1 | tr -s /

> + echo ${_main_str#*"$_sub_str"} > +} > diff --git a/mkdumprd b/mkdumprd > index 4d251ba..a8f9cbb 100644 > --- a/mkdumprd > +++ b/mkdumprd > @@ -364,7 +364,7 @@ handle_default_dump_target() > _mntpoint=$(get_mntpoint_from_path $SAVE_PATH) > _target=$(get_target_from_path $SAVE_PATH) > if [ "$_mntpoint" != "/" ]; then > - SAVE_PATH=${SAVE_PATH##"$_mntpoint"} > + SAVE_PATH=$(cut_out_substring $SAVE_PATH $_mntpoint) > _fstype=$(get_fs_type_from_target $_target) > > if $(is_fs_type_nfs $_fstype); then > -- > 1.9.3 > The function name is misleading, it is not a general function. How about change the name to trim_mount_point_from_path() Or I agree with Bao to drop this patch.

Thanks Dave

On 04/08/15 at 06:00pm, Minfei Huang wrote: > On 04/08/15 at 05:49pm, Dave Young wrote: > > On 04/07/15 at 05:20pm, Minfei Huang wrote: > > > Now kdump cannt parse the path correctly, if the path contains > > > duplicated "/". Following is an example to explain it detail. > > > > > > path //mnt/var/crash > > > > > > Then the warning will raise, as kdump cannt parse this path correctly. > > > > > > Force rebuild /boot/initramfs-3.19.1kdump.img > > > Rebuilding /boot/initramfs-3.19.1kdump.img > > > df: ‘/mnt///mnt/var/crash’: No such file or directory > > > /sbin/mkdumprd: line 239: [: -lt: unary operator expected > > > kexec: loaded kdump kernel > > > Starting kdump: [OK] > > > > > > Kdump fails to cut out the substring, if the string contains dumplicated > > > "/". > > > > > > Add the new function which it will cut out the substring accurately. > > > > > > Signed-off-by: Minfei Huang <mhuang(a)redhat.com&gt; > > > --- > > > dracut-module-setup.sh | 7 +++---- > > > kdump-lib.sh | 8 ++++++++ > > > mkdumprd | 2 +- > > > 3 files changed, 12 insertions(+), 5 deletions(-) > > > > > > diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh > > > index 4641025..477ede1 100755 > > > --- a/dracut-module-setup.sh > > > +++ b/dracut-module-setup.sh > > > @@ -311,9 +311,8 @@ kdump_install_net() { > > > default_dump_target_install_conf() > > > { > > > local _target _fstype > > > - local _s _t > > > local _mntpoint > > > - local _path _save_path > > > + local _save_path > > > > > > is_user_configured_dump_target && return > > > > > > @@ -334,11 +333,11 @@ default_dump_target_install_conf() > > > > > > echo "$_fstype $_target" >> ${initdir}/tmp/$$-kdump.conf > > > > > > - _path=${_save_path##"$_mntpoint"} > > > + _save_path=$(cut_out_substring $_save_path $_mntpoint) > > > > > > #erase the old path line, then insert the parsed path > > > sed -i "/^path/d" ${initdir}/tmp/$$-kdump.conf > > > - echo "path $_path" >> ${initdir}/tmp/$$-kdump.conf > > > + echo "path $_save_path" >> ${initdir}/tmp/$$-kdump.conf > > > fi > > > > > > } > > > diff --git a/kdump-lib.sh b/kdump-lib.sh > > > index f24f08d..ab01d74 100755 > > > --- a/kdump-lib.sh > > > +++ b/kdump-lib.sh > > > @@ -188,3 +188,11 @@ is_ipv6_target() > > > _server=${_server:-$_server_tmp} > > > echo $_server | grep -q ":" > > > } > > > + > > > +cut_out_substring() > > > +{ > > > + # filter out the duplicated "/" > > > + local _main_str=$(echo $1|sed "s#/\{1,\}#/#g") > > > + local _sub_str=$(echo $2|sed "s#/\{1,\}#/#g") > > > > I found a simpler way to do this: > > echo $1 | tr -s / > > > > No matter which command we use, we still need to strip the duplicated > "/" for both "$_main_str" and "$_sub_str". > Both sed and tr can fix this issue. IMO tr is more readable than sed. > > > > > + echo ${_main_str#*"$_sub_str"} > > > +} > > > diff --git a/mkdumprd b/mkdumprd > > > index 4d251ba..a8f9cbb 100644 > > > --- a/mkdumprd > > > +++ b/mkdumprd > > > @@ -364,7 +364,7 @@ handle_default_dump_target() > > > _mntpoint=$(get_mntpoint_from_path $SAVE_PATH) > > > _target=$(get_target_from_path $SAVE_PATH) > > > if [ "$_mntpoint" != "/" ]; then > > > - SAVE_PATH=${SAVE_PATH##"$_mntpoint"} > > > + SAVE_PATH=$(cut_out_substring $SAVE_PATH $_mntpoint) > > > _fstype=$(get_fs_type_from_target $_target) > > > > > > if $(is_fs_type_nfs $_fstype); then > > > -- > > > 1.9.3 > > > > > > > The function name is misleading, it is not a general function. > > > > How about change the name to trim_mount_point_from_path() > > > > Or I agree with Bao to drop this patch. > > > > The patch can fix the bz1209391, so we cannt drop it simply. > > What we concern is to cut out the same string in $_main_str. I know it can fix it, now we are discussing about your implementaion, right? We are not objecting about if we should fix it, instead we are talking about how to fix it. Both Bao and me do not like your function.

Thanks Dave

Now kdump cannt parse the path correctly, if the path contains duplicated "/". Following is an example to explain it detail. path //mnt/var/crash Then the warning will raise, as kdump cannt parse this path correctly. Force rebuild /boot/initramfs-3.19.1kdump.img Rebuilding /boot/initramfs-3.19.1kdump.img df: ‘/mnt///mnt/var/crash’: No such file or directory /sbin/mkdumprd: line 239: [: -lt: unary operator expected kexec: loaded kdump kernel Starting kdump: [OK] Kdump fails to cut out the substring, if the string contains dumplicated "/". Add the new function which it will cut out the substring accurately. Signed-off-by: Minfei Huang <mhuang(a)redhat.com&gt; --- dracut-module-setup.sh | 7 +++---- kdump-lib.sh | 8 ++++++++ mkdumprd | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 4641025..477ede1 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -311,9 +311,8 @@ kdump_install_net() { default_dump_target_install_conf() { local _target _fstype - local _s _t local _mntpoint - local _path _save_path + local _save_path is_user_configured_dump_target && return @@ -334,11 +333,11 @@ default_dump_target_install_conf() echo "$_fstype $_target" >> ${initdir}/tmp/$$-kdump.conf - _path=${_save_path##"$_mntpoint"} + _save_path=$(cut_out_substring $_save_path $_mntpoint) #erase the old path line, then insert the parsed path sed -i "/^path/d" ${initdir}/tmp/$$-kdump.conf - echo "path $_path" >> ${initdir}/tmp/$$-kdump.conf + echo "path $_save_path" >> ${initdir}/tmp/$$-kdump.conf fi } diff --git a/kdump-lib.sh b/kdump-lib.sh index f24f08d..ab01d74 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -188,3 +188,11 @@ is_ipv6_target() _server=${_server:-$_server_tmp} echo $_server | grep -q ":" } + +cut_out_substring() +{ + # filter out the duplicated "/" + local _main_str=$(echo $1|sed "s#/\{1,\}#/#g") + local _sub_str=$(echo $2|sed "s#/\{1,\}#/#g") + echo ${_main_str#*"$_sub_str"} +} diff --git a/mkdumprd b/mkdumprd index 4d251ba..a8f9cbb 100644 --- a/mkdumprd +++ b/mkdumprd @@ -364,7 +364,7 @@ handle_default_dump_target() _mntpoint=$(get_mntpoint_from_path $SAVE_PATH) _target=$(get_target_from_path $SAVE_PATH) if [ "$_mntpoint" != "/" ]; then - SAVE_PATH=${SAVE_PATH##"$_mntpoint"} + SAVE_PATH=$(cut_out_substring $SAVE_PATH $_mntpoint) _fstype=$(get_fs_type_from_target $_target) if $(is_fs_type_nfs $_fstype); then -- 1.9.3