[PATCH] get_persistent_dev(): fix name contention with dracut's similar function

[Pratyush Anand]

Resolves: BZ1348898

dracut-functions.sh defines a get_persistent_dev(). Earlier, we had another
local get_persistent_dev() in mkdumprd, however that was moved to
kdump-lib.sh, so that it can be reused in kdumpctl.

Since, dracut-module-setup.sh (which is dracut's
99kdumpbase/module-setup.sh) sources kdump-lib.sh. Therefore, once dracut
will execute 99kdumpbase module, it's own get_persistent_dev() function is
overwritten by kdump's version. If any other dracut module calls
get_persistent_dev() thereafter then, kdump's version is executed, which was
not expected.

Therefore rename kdump's get_persistent_dev() as get_kdump_persistent_dev()
to avoid any name contention.

Signed-off-by: Pratyush Anand 
---
 kdump-lib.sh | 2 +-
 kdumpctl     | 2 +-
 mkdumprd     | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh
index 66f2f0d77401..0769e2630d6c 100755
--- a/kdump-lib.sh
+++ b/kdump-lib.sh
@@ -86,7 +86,7 @@ to_dev_name() {
     echo $dev
 }

-get_persistent_dev() {
+get_kdump_persistent_dev() {
     local i _tmp _dev _lookup_dirs

_dev=$(udevadm info --query=name --name="$1" 2>/dev/null)
diff --git a/kdumpctl b/kdumpctl
index fcc9ad081437..21d83525680b 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -390,7 +390,7 @@ check_dump_fs_modified()
    if [[ $(expr substr $_new_fstype 1 3) = "nfs" ]];then
    	_new_dev=$_target
    else
-		_new_dev=$(get_persistent_dev $_target $_new_fstype)
+		_new_dev=$(get_kdump_persistent_dev $_target $_new_fstype)
    	if ! findmnt $_target >/dev/null; then
    		echo "Dump target $_target is probably not mounted."
    		return 2
diff --git a/mkdumprd b/mkdumprd
index 78afb1a9425d..37b36af37174 100644
--- a/mkdumprd
+++ b/mkdumprd
@@ -109,7 +109,7 @@ to_mount() {
     _mntopts="$_target $_fstype $_options"
     #for non-nfs _dev converting to use udev persistent name
     if [ -b "$_source" ]; then
-        _pdev="$(get_persistent_dev $_source $_fstype)"
+        _pdev="$(get_kdump_persistent_dev $_source $_fstype)"
         if [ $? -ne 0 ]; then
             return 1
         fi
@@ -522,7 +522,7 @@ do
         dd if=$config_val count=1 of=/dev/null > /dev/null 2>&1 || {
             perror_exit "Bad raw disk $config_val"
         }
-        _praw=$(get_persistent_dev $config_val "raw")
+        _praw=$(get_kdump_persistent_dev $config_val "raw")
         if [ $? -ne 0 ]; then
             exit 1
         fi

[Dave Young]

Hi, Pratyush

On 06/22/16 at 03:04pm, Pratyush Anand wrote:
> Resolves: BZ1348898
>
> dracut-functions.sh defines a get_persistent_dev(). Earlier, we had another
> local get_persistent_dev() in mkdumprd, however that was moved to
> kdump-lib.sh, so that it can be reused in kdumpctl.
>
> Since, dracut-module-setup.sh (which is dracut's
> 99kdumpbase/module-setup.sh) sources kdump-lib.sh. Therefore, once dracut
> will execute 99kdumpbase module, it's own get_persistent_dev() function is
> overwritten by kdump's version. If any other dracut module calls
> get_persistent_dev() thereafter then, kdump's version is executed, which was
> not expected.
>
> Therefore rename kdump's get_persistent_dev() as get_kdump_persistent_dev()
> to avoid any name contention.

kdump_get_persistent_dev sounds better because this is a verion of
get_presistent_dev for kdump.

BTW, we have plan to drop kdump get_presistent_dev and use dracut one,
but it need more work. So for temporally fixing our regression we can
get this merged first. In the future we can do the cleanup.
> Signed-off-by: Pratyush Anand
>
> kdump-lib.sh | 2 +-
>  kdumpctl     | 2 +-
>  mkdumprd     | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 66f2f0d77401..0769e2630d6c 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -86,7 +86,7 @@ to_dev_name() {
>      echo $dev
>  }
>
> -get_persistent_dev() {
> +get_kdump_persistent_dev() {
>      local i _tmp _dev _lookup_dirs
>  _dev=$(udevadm info --query=name --name="$1" 2>/dev/null)
>
> diff --git a/kdumpctl b/kdumpctl
> index fcc9ad081437..21d83525680b 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -390,7 +390,7 @@ check_dump_fs_modified()
>    if [[ $(expr substr $_new_fstype 1 3) = "nfs" ]];then
>    	_new_dev=$_target
>    else
> _new_dev=$(get_persistent_dev $_target $_new_fstype)
> _new_dev=$(get_kdump_persistent_dev $_target $_new_fstype)
> if ! findmnt $_target >/dev/null; then
> 	echo "Dump target $_target is probably not mounted."
> 	return 2
>
> diff --git a/mkdumprd b/mkdumprd
> index 78afb1a9425d..37b36af37174 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -109,7 +109,7 @@ to_mount() {
>      _mntopts="$_target $_fstype $_options"
>      #for non-nfs _dev converting to use udev persistent name
>      if [ -b "$_source" ]; then
>    _pdev="$(get_persistent_dev $_source $_fstype)"
>    _pdev="$(get_kdump_persistent_dev $_source $_fstype)"
>    if [ $? -ne 0 ]; then
>        return 1
>    fi
>
> @@ -522,7 +522,7 @@ do
>          dd if=$config_val count=1 of=/dev/null > /dev/null 2>&1 || {
>              perror_exit "Bad raw disk $config_val"
>          }
>    _praw=$(get_persistent_dev $config_val "raw")
>    _praw=$(get_kdump_persistent_dev $config_val "raw")
>    if [ $? -ne 0 ]; then
>        exit 1
>    fi

Thanks
Dave

[Pratyush Anand]

On 22/06/2016:06:06:11 PM, Dave Young wrote:
> Hi, Pratyush
>
> On 06/22/16 at 03:04pm, Pratyush Anand wrote:
> > Resolves: BZ1348898
> >
> > dracut-functions.sh defines a get_persistent_dev(). Earlier, we had another
> > local get_persistent_dev() in mkdumprd, however that was moved to
> > kdump-lib.sh, so that it can be reused in kdumpctl.
> >
> > Since, dracut-module-setup.sh (which is dracut's
> > 99kdumpbase/module-setup.sh) sources kdump-lib.sh. Therefore, once dracut
> > will execute 99kdumpbase module, it's own get_persistent_dev() function is
> > overwritten by kdump's version. If any other dracut module calls
> > get_persistent_dev() thereafter then, kdump's version is executed, which was
> > not expected.
> >
> > Therefore rename kdump's get_persistent_dev() as get_kdump_persistent_dev()
> > to avoid any name contention.
>
> kdump_get_persistent_dev sounds better because this is a verion of
> get_presistent_dev for kdump.

OK, will rename it like that.
> BTW, we have plan to drop kdump get_presistent_dev and use dracut one,
> but it need more work. So for temporally fixing our regression we can
> get this merged first. In the future we can do the cleanup.

Yes, While fixing, I had thought to do that change, but then it did not seem
straight forward.  It will need couple of other cleanup, and some work at dracut
end as well. So, yes we can take it up latter.

~Pratyush
> > Signed-off-by: Pratyush Anand
> >
> > kdump-lib.sh | 2 +-
> >  kdumpctl     | 2 +-
> >  mkdumprd     | 4 ++--
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/kdump-lib.sh b/kdump-lib.sh
> > index 66f2f0d77401..0769e2630d6c 100755
> > --- a/kdump-lib.sh
> > +++ b/kdump-lib.sh
> > @@ -86,7 +86,7 @@ to_dev_name() {
> >      echo $dev
> >  }
> >
> > -get_persistent_dev() {
> > +get_kdump_persistent_dev() {
> >      local i _tmp _dev _lookup_dirs
> >  _dev=$(udevadm info --query=name --name="$1" 2>/dev/null)
> >
> > diff --git a/kdumpctl b/kdumpctl
> > index fcc9ad081437..21d83525680b 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -390,7 +390,7 @@ check_dump_fs_modified()
> >    if [[ $(expr substr $_new_fstype 1 3) = "nfs" ]];then
> >    	_new_dev=$_target
> >    else
> > _new_dev=$(get_persistent_dev $_target $_new_fstype)
> > _new_dev=$(get_kdump_persistent_dev $_target $_new_fstype)
> > if ! findmnt $_target >/dev/null; then
> > 	echo "Dump target $_target is probably not mounted."
> > 	return 2
> >
> > diff --git a/mkdumprd b/mkdumprd
> > index 78afb1a9425d..37b36af37174 100644
> > --- a/mkdumprd
> > +++ b/mkdumprd
> > @@ -109,7 +109,7 @@ to_mount() {
> >      _mntopts="$_target $_fstype $_options"
> >      #for non-nfs _dev converting to use udev persistent name
> >      if [ -b "$_source" ]; then
> >    _pdev="$(get_persistent_dev $_source $_fstype)"
> >    _pdev="$(get_kdump_persistent_dev $_source $_fstype)"
> >    if [ $? -ne 0 ]; then
> >        return 1
> >    fi
> >
> > @@ -522,7 +522,7 @@ do
> >          dd if=$config_val count=1 of=/dev/null > /dev/null 2>&1 || {
> >              perror_exit "Bad raw disk $config_val"
> >          }
> >    _praw=$(get_persistent_dev $config_val "raw")
> >    _praw=$(get_kdump_persistent_dev $config_val "raw")
> >    if [ $? -ne 0 ]; then
> >        exit 1
> >    fi
>
> Thanks
> Dave