Skip to content

[PATCH v1]kdumpctl: add kdump tested status updating/reporting support - #8

Closed
liutgnu wants to merge 1 commit into
rhkdump:mainfrom
liutgnu:kdump-test
Closed

[PATCH v1]kdumpctl: add kdump tested status updating/reporting support#8
liutgnu wants to merge 1 commit into
rhkdump:mainfrom
liutgnu:kdump-test

Conversation

@liutgnu

@liutgnu liutgnu commented May 20, 2024

Copy link
Copy Markdown
Collaborator

Motivation

People usually won't test if kdump can really generate a vmcore before regarding kdump as workable, which as a result, a possibility of no vmcores generated after a real system crash. It is unexpected for kdump.

Thought it is highly recommented people to test kdump after any system modification, such as:

a. after kernel patching or whole yum update, as it might break something
on which kdump is dependent, maybe due to introduction of any new bug etc.
b. after any change at hardware level, maybe storage, networking,
firmware upgrading etc.
c. after implementing any new application, like which involves 3rd party modules
etc.

Though these exceed the range of kdump, however a simple test notification is good to have for now.

Design

Kdump currently will check any relating files/fs/drivers modified before determine if initrd should rebuild when (re)start. A rebuild is an indicator of modification, so kdump need to be tested. This will clear the test status specified in $KDUMP_STATUS.

Kdump test check will happen at "kdumpctl (re)start/status", and will report the tested/untested status to users. A tested status indicates previously there was a vmcore successfully generated based on the current env, so it is more likely a vmcore will be generated later when real crash happens.

$KDUMP_STATUS is used for recording the newest vmcore and the test status. The format will be like:

root@1.2.3.4:/var/crash 127.0.0.1-2024-05-01-15:54:29/vmcore 1714550071 untested

Which means, the vmcore saved at this path, with this timestamp is the newest one, and the kdump is not tested. If later another vmcore in the same path been found, with larger(newer) timestamp. The newer vmcore will be updated into $KDUMP_STATUS, and the status will be marked as tested. (Note: There is a premise the newer vmcore is generated by the current machine. If not then the kdump test status is incorrect, see the following concurrent test case:

 machine1:                   machine2:

 start test checking         start test checking
         |                          |
     V                          V
 get the newest vmcore       get the newest vmcore
         |                          |
         V                          V
 notice user untested        notice user untested
         |                          |
         V                          V
 expect user test kdump  <-- generate vmcore
 by generate a vmcore               |
         |                          V
         V                         ...
 start test checking
         |
         V
 find a newer vmcore
         |
         V
 update $KDUMP_STATUS
 and notice user tested  <-- wrong test status

In order to differentiate vmcores and corresponding machine, ip address is not reliable, like ssh dump through a NAT network. Extra code will be used to implementing this feature. Besides personally I think concurrent kdump test on multi-machines is rare. So only serial kdump test is supported for now.)

The detailed updating/checking rules can be found in check_kdump_tested().

Motivation
==========

People usually won't test if kdump can really generate a vmcore before
regarding kdump as workable, which as a result, a possibility of no vmcores
generated after a real system crash. It is unexpected for kdump.

Thought it is highly recommented people to test kdump after any system
modification, such as:

a. after kernel patching or whole yum update, as it might break something
   on which kdump is dependent, maybe due to introduction of any new bug etc.
b. after any change at hardware level, maybe storage, networking,
   firmware upgrading etc.
c. after implementing any new application, like which involves 3rd party modules
   etc.

Though these exceed the range of kdump, however a simple test notification is
good to have for now.

Design
======

Kdump currently will check any relating files/fs/drivers modified before
determine if initrd should rebuild when (re)start. A rebuild is an
indicator of modification, so kdump need to be tested. This will clear
the test status specified in $KDUMP_STATUS.

Kdump test check will happen at "kdumpctl (re)start/status", and will
report the tested/untested status to users. A tested status indicates
previously there was a vmcore successfully generated based on the current
env, so it is more likely a vmcore will be generated later when real crash
happens.

$KDUMP_STATUS is used for recording the newest vmcore and the test status.
The format will be like:

  root@1.2.3.4:/var/crash 127.0.0.1-2024-05-01-15:54:29/vmcore 1714550071 untested

Which means, the vmcore saved at this path, with this timestamp is the
newest one, and the kdump is not tested. If later another vmcore in the
same path been found, with larger(newer) timestamp. The newer vmcore
will be updated into $KDUMP_STATUS, and the status will be marked as
tested. (Note: There is a premise the newer vmcore is generated by the
current machine. If not then the kdump test status is incorrect, see the
following concurrent test case:

     machine1:                   machine2:

     start test checking         start test checking
             |                          |
	     V                          V
     get the newest vmcore       get the newest vmcore
             |                          |
             V                          V
     notice user untested        notice user untested
             |                          |
             V                          V
     expect user test kdump  <-- generate vmcore
     by generate a vmcore               |
             |                          V
             V                         ...
     start test checking
             |
             V
     find a newer vmcore
             |
             V
     update $KDUMP_STATUS
     and notice user tested  <-- wrong test status

In order to differentiate vmcores and corresponding machine, ip address
is not reliable, like ssh dump through a NAT network. Extra code will be
used to implementing this feature. Besides personally I think concurrent
kdump test on multi-machines is rare. So only serial kdump test is
supported for now.)

The detailed updating/checking rules can be found in check_kdump_tested().

Signed-off-by: Tao Liu <ltao@redhat.com>
Comment thread kdumpctl
TARGET_INITRD=""
#kdump shall be the default dump mode
DEFAULT_DUMP_MODE="kdump"
KDUMP_STATUS="/var/crash/kdump.status"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /var/crash could not exist, how to ensure it being created? Is this status saved on dump target or the crashing machine? Probably save on crashing machine looks better

Comment thread kdumpctl
dinfo "Unable to check for raw dump. Manual verification of vmcore collection required."
return 1
elif is_ssh_dump_target; then
_latest_record=$(ssh -i "${OPT[sshkey]}" -o BatchMode=yes "${OPT[_target]}" \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are multiple machine dumping to same ssh/nfs server, then just finding the latest timestamp of the vmcore will be not enough here..

Comment thread kdumpctl
find . -name "vmcore" -or -name "vmcore.flat" \
| xargs stat -c %n\ %Y 2> /dev/null \
| sort -k2 | sed 's|^\./||' \
| tail -1")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here and other same code, maybe create a function for these search and call them for different cases, ssh, nfs. And it would be good to add a brief code comment about the purpose of the function. Otherwise if an incomplete vmcore detected, it can be regarded as tested, but it would be good to report it in the warning/notification.

Comment thread kdumpctl
if [[ $_kdstatus_core_host == $_current_core_host ]]; then
if [[ $_current_core_timestamp -gt $_kdstatus_core_timestamp ]]; then
echo "$_record_of_current tested" > $KDUMP_STATUS
dinfo "Test success: Last tested on $_current_core_date"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use "Tesed: last tested on ..." "success" should be fore vmcore saved case, and fail should be for no vmcore or incomplete?

@daveyoung daveyoung left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Tao, thanks for posting this, not sure how to comment about the patch description in github. Let me pasted something here:

People usually won't test if kdump can really generate a vmcore before

may forget to test to ensure kdump works.

Thought it is highly recommented people to test kdump after any system

recommended
remove Thought

[...]
-> same path been found, with larger(newer) timestamp. The newer vmcore

being

[...]
-> used to implementing this feature. Besides personally I think concurrent

implement

-> kdump test on multi-machines is rare. So only serial kdump test is
supported for now.)

remove "for now" Since this is best effort for the common cases and
we do not want to over engineering.

OTOH I think create a txt doc in source tree about the design or just put
more comments in code with details would be better.

Also there should be some way for people to manually update the tested
status and mute the warnings.

For example by default enable the test check, but there is a switch in
sysconfig people can disable it. Or people can mark it as tested.

@liutgnu

liutgnu commented Jun 18, 2024

Copy link
Copy Markdown
Collaborator Author

I will close this MR because v2 been posted in [1]. Thanks!

[1] #13

@liutgnu liutgnu closed this Jun 18, 2024
@daveyoung

daveyoung commented Jun 21, 2024 via email

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants