[PATCH] kdumpctl: Export DUMP_TARGET

[Bhupesh Sharma]

When building kdump initramfs for a SSH dump target, mkdumprd would
check whether it has the write permission on the SSH Server's
$DUMP_TARGET.

However $DUMP_TARGET is missing in the actual error message when the
user doesn't not have the write permission. For example:

 # kdumpctl restart
   kexec: unloaded kdump kernel
   Stopping kdump: [OK]
   Could not create temporary directory on :/home/bhsharma/test. Make
   sure user has write permission on destination
   mkdumprd: failed to make kdump initrd
   Starting kdump: [FAILED]

This patch exports $DUMP_TARGET from kdumpctl to make sure that
mkdumprd can use the same inside mkdir_save_path_ssh() function to
fix the issue. Note that without this patch, $DUMP_TARGET is
uninitialized in mkdumprd.

Signed-off-by: Bhupesh Sharma <bhsharma(a)redhat.com&gt;
---
 kdumpctl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kdumpctl b/kdumpctl
index 88128a3e73d6..0d5760891069 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -720,6 +720,7 @@ check_ssh_config()
 			;;
 		ssh)
 			DUMP_TARGET=$config_val
+			export DUMP_TARGET
 			;;
 		*)
 			;;
-- 
2.7.4

[Kairui Song]

Hi Bhupesh,

Thanks for the patch.

On Tue, Feb 25, 2020 at 3:36 AM Bhupesh Sharma <bhsharma(a)redhat.com&gt; wrote:
>  When building kdump initramfs for a SSH dump target, mkdumprd would
>  check whether it has the write permission on the SSH Server's
>  $DUMP_TARGET.
>
>  However $DUMP_TARGET is missing in the actual error message when the
>  user doesn't not have the write permission. For example:
>
>   # kdumpctl restart
>     kexec: unloaded kdump kernel
>     Stopping kdump: [OK]
>     Could not create temporary directory on :/home/bhsharma/test. Make
>     sure user has write permission on destination
>     mkdumprd: failed to make kdump initrd
>     Starting kdump: [FAILED]
>
>  This patch exports $DUMP_TARGET from kdumpctl to make sure that
>  mkdumprd can use the same inside mkdir_save_path_ssh() function to
>  fix the issue. Note that without this patch, $DUMP_TARGET is
>  uninitialized in mkdumprd.
>
>  Signed-off-by: Bhupesh Sharma <bhsharma(a)redhat.com&gt;
>  ---
>   kdumpctl | 1 +
>   1 file changed, 1 insertion(+)
>
>  diff --git a/kdumpctl b/kdumpctl
>  index 88128a3e73d6..0d5760891069 100755
>  --- a/kdumpctl
>  +++ b/kdumpctl
>  @@ -720,6 +720,7 @@ check_ssh_config()
>                          ;;
>                  ssh)
>                          DUMP_TARGET=$config_val
>  +                       export DUMP_TARGET
>                          ;;
>                  *)
>                          ;;
>  --
>  2.7.4
>  
I think it looks very hacky this way. mkdumprd only use DUMP_TARGET
enviroment variable in mkdir_save_path_ssh, and mkdir_save_path_ssh
should already have the dump target as the argument ($1) already, so
no need to export this. mkdir_save_path_ssh can use $1 instead, right?

-- 
Best Regards,
Kairui Song

[Bhupesh Sharma]

Hi Kairui,

On Tue, Mar 3, 2020 at 12:04 PM Kairui Song <kasong(a)redhat.com&gt; wrote:
>  Hi Bhupesh,
>
>  Thanks for the patch.
>
>  On Tue, Feb 25, 2020 at 3:36 AM Bhupesh Sharma <bhsharma(a)redhat.com&gt; wrote:
>  >
>  > When building kdump initramfs for a SSH dump target, mkdumprd would
>  > check whether it has the write permission on the SSH Server's
>  > $DUMP_TARGET.
>  >
>  > However $DUMP_TARGET is missing in the actual error message when the
>  > user doesn't not have the write permission. For example:
>  >
>  >  # kdumpctl restart
>  >    kexec: unloaded kdump kernel
>  >    Stopping kdump: [OK]
>  >    Could not create temporary directory on :/home/bhsharma/test. Make
>  >    sure user has write permission on destination
>  >    mkdumprd: failed to make kdump initrd
>  >    Starting kdump: [FAILED]
>  >
>  > This patch exports $DUMP_TARGET from kdumpctl to make sure that
>  > mkdumprd can use the same inside mkdir_save_path_ssh() function to
>  > fix the issue. Note that without this patch, $DUMP_TARGET is
>  > uninitialized in mkdumprd.
>  >
>  > Signed-off-by: Bhupesh Sharma <bhsharma(a)redhat.com&gt;
>  > ---
>  >  kdumpctl | 1 +
>  >  1 file changed, 1 insertion(+)
>  >
>  > diff --git a/kdumpctl b/kdumpctl
>  > index 88128a3e73d6..0d5760891069 100755
>  > --- a/kdumpctl
>  > +++ b/kdumpctl
>  > @@ -720,6 +720,7 @@ check_ssh_config()
>  >                         ;;
>  >                 ssh)
>  >                         DUMP_TARGET=$config_val
>  > +                       export DUMP_TARGET
>  >                         ;;
>  >                 *)
>  >                         ;;
>  > --
>  > 2.7.4
>  >
>
>  I think it looks very hacky this way. mkdumprd only use DUMP_TARGET
>  enviroment variable in mkdir_save_path_ssh, and mkdir_save_path_ssh
>  should already have the dump target as the argument ($1) already, so
>  no need to export this. mkdir_save_path_ssh can use $1 instead, right? 
Thanks for the suggestion. I think this is a good idea as too.
I will fix it up and send a v2 shortly.

Regards,
Bhupesh