On Tue, Oct 10, 2023 at 12:31 AM Philipp Rudo <prudo(a)redhat.com> wrote:
Hi Tao,
On Thu, 28 Sep 2023 21:20:29 +0800
Tao Liu <ltao(a)redhat.com> wrote:
> The kdumpctl test-check involves the following procedures:
>
> 1) Get the test id from KDUMP_TEST_ENV file.
> 2) Mount the remote fs and check if the test id exists in the specific
> dump path
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> kdumpctl | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/kdumpctl b/kdumpctl
> index b54b5ae..5e9d1e7 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -1671,6 +1671,73 @@ reset_crashkernel_for_installed_kernel()
> _update_crashkernel "$_installed_kernel"
> }
>
> +# $1: dump target
> +# $2: dump path
> +# $3: kdump test id string
> +kdump_test_check_id()
> +{
> + local _path=$2 _id_str=$3
> + local kexec_dmesg_file=$(find "$_path" \( -name "$(basename
"$KDUMP_LOG_FILE")" -or -name "vmcore-dmesg.txt" \) -exec grep -l
"$_id_str" {} +)
> +
> + if [ -n "$kexec_dmesg_file" ]; then
> + return 0
> + else
> + return 1
> + fi
> +}
Please use the bash specific [[...]] notation for tests in kdumpctl.
Furthermore you don't need the if-else block. You can simply use
return [[ -n "$kexec_dmesg_file" ]]
In fact you could even drop the 'return' as bash automatically returns
the return value of the last statement of a function. But that's more a
matter of taste.
Agreed, thanks a lot!
> +
> +# $1: dump target
> +# $2: dump path
> +# $3: kdump test id string
> +kdump_test_ssh_check_id()
> +{
> + local kexec_dmesg_file
> + local _opt _target=$1 _path=$2 _id_str=$3
> +
> + get_ssh_key_location
> + _opt=(-i "$SSH_KEY_LOCATION" -o BatchMode=yes -o
StrictHostKeyChecking=yes)
/get_ssh_key_location/d
s/$SSH_KEY_LOCATION/${OPT[sshkey]}/
> + kexec_dmesg_file=$(ssh -qn "${_opt[@]}" "$_target" find
"$_path" -name "$(basename "$KDUMP_LOG_FILE")" -exec grep -l
"$_id_str" {} +)
> + kexec_dmesg_file+=$(ssh -qn "${_opt[@]}" "$_target" find
"$_path" -name "vmcore-dmesg.txt" -exec grep -l "$_id_str"
{} +)
> +
> + if [ -n "$kexec_dmesg_file" ]; then
> + return 0
> + else
> + return 1
> + fi
> +}
same like above.
> +kdump_test_check_target()
> +{
> + local _test_id
> +
> + _test_id=$(sed -ne "s/.*\($KDUMP_TEST_PREFIX.*\)\"/\1/p"
"$KDUMP_TEST_ENV" 2>/dev/null)
> +
> + if [[ ! -f "$KDUMP_TEST_ENV" || -z "$_test_id" ]]; then
> + dinfo "Need kdump test, please run \"kdumpctl test\"
first!"
> + return
> + fi
> +
> + [[ ${#OPT[@]} -eq 0 ]] && { parse_config || return ;}
Is is possible to call parse_config twice? If so I think this check
should go into parse_config so that it can be called multiple times
without causing problems.
No, call parse_config twice will hit the error: Duplicated kdump
config value of option $opt, e.g. path will be set as "/var/crash" the
1st time, the 2nd parse_config will re-set path as "/var/crash" and
hit the error. Though it is not a difficult issue we can solve.
Thanks,
Tao Liu
Thanks
Philipp
> + if is_fs_dump_target || is_nfs_dump_target; then
> + check_user_configured_target "${OPT[_target]}"
"${OPT[_fstype]}" kdump_test_check_id "$_test_id"
> + elif is_ssh_dump_target; then
> + kdump_test_ssh_check_id "${OPT[_target]}"
"${OPT[path]}" "$_test_id"
> + elif is_raw_dump_target; then
> + dinfo "Kdump test is not supported for raw dump target."
> + return
> + else
> + handle_default_dump_target kdump_test_check_id "$_test_id"
> + fi
> +
> + if [[ $? -eq 0 ]]; then
> + dinfo "kdump test successful!"
> + else
> + dinfo "kdump test failed!"
> + fi
> +
> + rm -f "$KDUMP_TEST_ENV"
> +}
> +
> countdown()
> {
> local _sec=$1 _msg=$2
> @@ -1879,6 +1946,9 @@ main()
> test)
> kdump_do_test
> ;;
> + test-check)
> + kdump_test_check_target
> + ;;
> *)
> dinfo $"Usage: $0
{estimate|start|stop|status|restart|reload|rebuild|reset-crashkernel|propagate|showmem|test|test-check}"
> exit 1