[PATCH] kdump-initramfs-lib: Fix core_collector issue

[WANG Chao]

In ssh or raw dump case, if user do not specify "core_collector" in
kdump.conf, kdump will fail. Because global DEFAULT_CORE_COLLECTOR
variable isn't applied to CORE_COLLECTOR. Now fix it.

Signed-off-by: WANG Chao <chaowang(a)redhat.com&gt;
---
 kdump-lib-initramfs.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
index f882b09..71986be 100755
--- a/kdump-lib-initramfs.sh
+++ b/kdump-lib-initramfs.sh
@@ -77,7 +77,7 @@ get_kdump_confs()
 
     if is_ssh_dump_target || is_raw_dump_target; then
         if [ -z "$user_specified_cc" ]; then
-            CORE_COLLECTOR="$CORE_COLLECTOR -F"
+            CORE_COLLECTOR="$DEFAULT_CORE_COLLECTOR -F"
         fi
     fi
 
-- 
1.9.3

[Dave Young]

On 11/28/14 at 02:00pm, WANG Chao wrote:
>  In ssh or raw dump case, if user do not specify
> "core_collector" in
>  kdump.conf, kdump will fail. Because global DEFAULT_CORE_COLLECTOR
>  variable isn't applied to CORE_COLLECTOR. Now fix it.
>  
>  Signed-off-by: WANG Chao <chaowang(a)redhat.com&gt;
>  ---
>   kdump-lib-initramfs.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>  
>  diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
>  index f882b09..71986be 100755
>  --- a/kdump-lib-initramfs.sh
>  +++ b/kdump-lib-initramfs.sh
>  @@ -77,7 +77,7 @@ get_kdump_confs()
>   
>       if is_ssh_dump_target || is_raw_dump_target; then
>           if [ -z "$user_specified_cc" ]; then
>  -            CORE_COLLECTOR="$CORE_COLLECTOR -F"
>  +            CORE_COLLECTOR="$DEFAULT_CORE_COLLECTOR -F"
>           fi
>       fi 
In dracut-kdump.sh, we have below code:

read_kdump_conf
fence_kdump_notify

if [ -z "$CORE_COLLECTOR" ];then
    CORE_COLLECTOR=$DEFAULT_CORE_COLLECTOR
    if is_ssh_dump_target || is_raw_dump_target; then
        CORE_COLLECTOR="$CORE_COLLECTOR -F"
    fi
fi

get_kdump_confs looks more to get kdump config values from kdump.conf
if there's nothing it make sense to return a nul string. So remove the
duplicate code in get_kdump_confs to set the default value looks better..

Thanks
Dave

[Dave Young]

On 11/28/14 at 03:07pm, Dave Young wrote:
>  On 11/28/14 at 02:00pm, WANG Chao wrote:
>  > In ssh or raw dump case, if user do not specify "core_collector" in
>  > kdump.conf, kdump will fail. Because global DEFAULT_CORE_COLLECTOR
>  > variable isn't applied to CORE_COLLECTOR. Now fix it.
>  > 
>  > Signed-off-by: WANG Chao <chaowang(a)redhat.com&gt;
>  > ---
>  >  kdump-lib-initramfs.sh | 2 +-
>  >  1 file changed, 1 insertion(+), 1 deletion(-)
>  > 
>  > diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
>  > index f882b09..71986be 100755
>  > --- a/kdump-lib-initramfs.sh
>  > +++ b/kdump-lib-initramfs.sh
>  > @@ -77,7 +77,7 @@ get_kdump_confs()
>  >  
>  >      if is_ssh_dump_target || is_raw_dump_target; then
>  >          if [ -z "$user_specified_cc" ]; then
>  > -            CORE_COLLECTOR="$CORE_COLLECTOR -F"
>  > +            CORE_COLLECTOR="$DEFAULT_CORE_COLLECTOR -F"
>  >          fi
>  >      fi
>  
>  In dracut-kdump.sh, we have below code:
>  
>  read_kdump_conf
>  fence_kdump_notify
>  
>  if [ -z "$CORE_COLLECTOR" ];then
>      CORE_COLLECTOR=$DEFAULT_CORE_COLLECTOR
>      if is_ssh_dump_target || is_raw_dump_target; then
>          CORE_COLLECTOR="$CORE_COLLECTOR -F"
>      fi
>  fi
>  
>  get_kdump_confs looks more to get kdump config values from kdump.conf
>  if there's nothing it make sense to return a nul string. So remove the
>  duplicate code in get_kdump_confs to set the default value looks better..
>   
Another issue is is it really good to put the variables in a lib.sh?
I think put functions in lib scripts looks good they can accept input, and
create some output. But for default values maybe we should keep it in
dracut-kdump.sh or create another constants.sh?

Thanks
Dave

[WANG Chao]

On 11/28/14 at 03:12pm, Dave Young wrote:
>  On 11/28/14 at 03:07pm, Dave Young wrote:
>  > On 11/28/14 at 02:00pm, WANG Chao wrote:
>  > > In ssh or raw dump case, if user do not specify "core_collector" in
>  > > kdump.conf, kdump will fail. Because global DEFAULT_CORE_COLLECTOR
>  > > variable isn't applied to CORE_COLLECTOR. Now fix it.
>  > > 
>  > > Signed-off-by: WANG Chao <chaowang(a)redhat.com&gt;
>  > > ---
>  > >  kdump-lib-initramfs.sh | 2 +-
>  > >  1 file changed, 1 insertion(+), 1 deletion(-)
>  > > 
>  > > diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
>  > > index f882b09..71986be 100755
>  > > --- a/kdump-lib-initramfs.sh
>  > > +++ b/kdump-lib-initramfs.sh
>  > > @@ -77,7 +77,7 @@ get_kdump_confs()
>  > >  
>  > >      if is_ssh_dump_target || is_raw_dump_target; then
>  > >          if [ -z "$user_specified_cc" ]; then
>  > > -            CORE_COLLECTOR="$CORE_COLLECTOR -F"
>  > > +            CORE_COLLECTOR="$DEFAULT_CORE_COLLECTOR -F"
>  > >          fi
>  > >      fi
>  > 
>  > In dracut-kdump.sh, we have below code:
>  > 
>  > read_kdump_conf
>  > fence_kdump_notify
>  > 
>  > if [ -z "$CORE_COLLECTOR" ];then
>  >     CORE_COLLECTOR=$DEFAULT_CORE_COLLECTOR
>  >     if is_ssh_dump_target || is_raw_dump_target; then
>  >         CORE_COLLECTOR="$CORE_COLLECTOR -F"
>  >     fi
>  > fi
>  > 
>  > get_kdump_confs looks more to get kdump config values from kdump.conf
>  > if there's nothing it make sense to return a nul string. So remove the
>  > duplicate code in get_kdump_confs to set the default value looks better.. 
Right. These lines are duplicates. I'll remove them.

>  Another issue is is it really good to put the variables in a lib.sh?
>  I think put functions in lib scripts looks good they can accept input, and
>  create some output. But for default values maybe we should keep it in
>  dracut-kdump.sh or create another constants.sh? 
Putting the global variables in dracut-kdump.sh doesn't sound promising.
The common functions would still access these variables and modify them.
And another shell script to hold these variables sould too much to me.

I think I'd prefer the way it is now.

Thanks
WANG Chao