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