Hi Tao,
On Tue, 10 Oct 2023 11:25:00 +0800
Tao Liu <ltao(a)redhat.com> wrote:
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.
Sorry, the question was meant if there are occurrences where
parse_config is called twice. Not if it is possible to call it twice.
Because that will definitely cause the problem you mentioned.
Anyway, it's probably better to fix parse_config even when there is no
way to call it twice at the moment. Could you do that?
Thanks
Philipp
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
>
_______________________________________________
kexec mailing list -- kexec(a)lists.fedoraproject.org
To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue