[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 
---
 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
    		;;
    	*)
    		;;

[Kairui Song]

Hi Bhupesh,

Thanks for the patch.

On Tue, Feb 25, 2020 at 3:36 AM Bhupesh Sharma  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
>
> 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?

[Bhupesh Sharma]

Hi Kairui,

On Tue, Mar 3, 2020 at 12:04 PM Kairui Song  wrote:
> Hi Bhupesh,
>
> Thanks for the patch.
>
> On Tue, Feb 25, 2020 at 3:36 AM Bhupesh Sharma  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
> >
> > 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