-
Notifications
You must be signed in to change notification settings - Fork 21
kdump: Introduce new kdump_emergency option and /etc/kdump/emergency.d directory interface #143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
abfe792
88c7b76
f0add69
633cb23
777dc95
eb7f3b3
a435be1
1cdee08
c1b768b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -660,7 +660,7 @@ kdump_install_net() { | |||||||
| fi | ||||||||
| } | ||||||||
|
|
||||||||
| # install etc/kdump/pre.d and /etc/kdump/post.d | ||||||||
| # install etc/kdump/pre.d and /etc/kdump/post.d /etc/kdump/emergency.d | ||||||||
| kdump_install_pre_post_conf() { | ||||||||
| if [[ -d /etc/kdump/pre.d ]]; then | ||||||||
| for file in /etc/kdump/pre.d/*; do | ||||||||
|
|
@@ -681,6 +681,16 @@ kdump_install_pre_post_conf() { | |||||||
| fi | ||||||||
| done | ||||||||
| fi | ||||||||
|
|
||||||||
| if [[ -d /etc/kdump/emergency.d ]]; then | ||||||||
| for file in /etc/kdump/emergency.d/*; do | ||||||||
| if [[ -x $file ]]; then | ||||||||
| dracut_install "$file" | ||||||||
| elif [[ $file != "/etc/kdump/emergency.d/*" ]]; then | ||||||||
| echo "$file is not executable" | ||||||||
|
Comment on lines
+689
to
+690
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The echo statement does not redirect to standard error, so the message will not be visible in logs. This could make it difficult to diagnose issues with the emergency scripts.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn’t an emergency-time message; it can be logged during initramfs generation when kdump.service starts. |
||||||||
| fi | ||||||||
| done | ||||||||
| fi | ||||||||
| } | ||||||||
|
|
||||||||
| default_dump_target_install_conf() { | ||||||||
|
|
@@ -743,7 +753,7 @@ kdump_install_conf() { | |||||||
| kdump_collect_netif_usage "$(get_dracut_args_target "$_val")" | ||||||||
| fi | ||||||||
| ;; | ||||||||
| kdump_pre | kdump_post | extra_bins) | ||||||||
| kdump_pre | kdump_post | kdump_emergency | extra_bins) | ||||||||
| # shellcheck disable=SC2086 | ||||||||
| dracut_install $_val | ||||||||
| ;; | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -368,7 +368,7 @@ parse_config() | |||||
| dwarn "Please update $KDUMP_CONFIG_FILE to use option 'failure_action' instead." | ||||||
| _set_config failure_action "$config_val" || return 1 | ||||||
| ;; | ||||||
| path | core_collector | kdump_post | kdump_pre | extra_bins | extra_modules | failure_action | final_action | force_rebuild | force_no_rebuild | fence_kdump_args | fence_kdump_nodes | auto_reset_crashkernel) ;; | ||||||
| path | core_collector | kdump_post | kdump_pre | kdump_emergency | extra_bins | extra_modules | failure_action | final_action | force_rebuild | force_no_rebuild | fence_kdump_args | fence_kdump_nodes | auto_reset_crashkernel) ;; | ||||||
|
|
||||||
| net | options | link_delay | disk_timeout | debug_mem_level | blacklist) | ||||||
| derror "Deprecated kdump config option: $config_opt. Refer to kdump.conf manpage for alternatives." | ||||||
|
|
@@ -461,9 +461,6 @@ check_files_modified() | |||||
| #also rebuild when Pacemaker cluster conf is changed and fence kdump is enabled. | ||||||
| modified_files=$(get_pcs_cluster_modified_files "$image_time") | ||||||
|
|
||||||
| EXTRA_BINS=${OPT[kdump_post]} | ||||||
| CHECK_FILES=${OPT[kdump_pre]} | ||||||
| HOOKS="/etc/kdump/post.d/ /etc/kdump/pre.d/" | ||||||
| if [[ -d /etc/kdump/post.d ]]; then | ||||||
| for file in /etc/kdump/post.d/*; do | ||||||
| if [[ -x $file ]]; then | ||||||
|
|
@@ -478,13 +475,18 @@ check_files_modified() | |||||
| fi | ||||||
| done | ||||||
| fi | ||||||
| HOOKS="$HOOKS $POST_FILES $PRE_FILES" | ||||||
| if [[ -d /etc/kdump/emergency.d ]]; then | ||||||
| for file in /etc/kdump/emergency.d/*; do | ||||||
| if [[ -x $file ]]; then | ||||||
| EMERGENCY_FILES="$EMERGENCY_FILES $file" | ||||||
| fi | ||||||
| done | ||||||
| fi | ||||||
| HOOKS="$POST_FILES $PRE_FILES $EMERGENCY_FILES" | ||||||
| CORE_COLLECTOR=$(echo "${OPT[core_collector]}" | awk '{print $1}') | ||||||
| CORE_COLLECTOR=$(type -P "$CORE_COLLECTOR") | ||||||
| # POST_FILES and PRE_FILES are already checked against executable, need not to check again. | ||||||
| EXTRA_BINS="$EXTRA_BINS $CHECK_FILES" | ||||||
| CHECK_FILES=${OPT[extra_bins]} | ||||||
| EXTRA_BINS="$EXTRA_BINS $CHECK_FILES" | ||||||
| EXTRA_BINS="${OPT[kdump_post]} ${OPT[kdump_pre]} ${OPT[kdump_emergency]} ${OPT[extra_bins]}" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variables
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original code did not take into account cases where the values contain spaces or special characters. |
||||||
| files="$KDUMP_CONFIG_FILE $KDUMP_KERNEL $EXTRA_BINS $CORE_COLLECTOR" | ||||||
| [[ -e /etc/fstab ]] && files="$files /etc/fstab" | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
do_kdump_emergencyfunction does not have any error handling for the directory iteration. If/etc/kdump/emergency.ddoes not exist or is not a directory, the script will continue without any warning. This could lead to unexpected behavior if the administrator expects the emergency scripts to be executed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is implemented exactly the same way as pre.d and post.d, if we fix this, we also need to apply the same fix to those as well.
I think it would be better to bundle the changes for pre.d and post.d into a separate pull request.