Hi,
These patches are improvement and cleanup for sanity checking kdump.conf. Please take a look.
v3 - v2: minor nit update per Vivek
WANG Chao (3): Check "default" option config in kdump.conf kdumpctl: clean up fence_kdump_nodes config check kdumpctl: check_config_fence_kdump also check short hostname
kdumpctl | 55 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 18 deletions(-)
"default" option only takes reboot/halt/poweroff/shell/dump_to_rootfs as its config value. Otherwise kdump service fails early.
Signed-off-by: WANG Chao chaowang@redhat.com --- kdumpctl | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/kdumpctl b/kdumpctl index ee3214a..49ff98e 100755 --- a/kdumpctl +++ b/kdumpctl @@ -226,6 +226,22 @@ backup_initrd() fi }
+check_config_default_action() +{ + local default_action=$1 + + case "$default_action" in + reboot|halt|poweroff|shell|dump_to_rootfs) + ;; + *) + echo "Invalid kdump config value for option default" + return 1 + ;; + esac + + return 0 +} + check_config() { local nr @@ -242,7 +258,7 @@ check_config() case "$config_opt" in #* | "") ;; - raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|default|force_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes) + raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|force_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes) [ -z "$config_val" ] && { echo "Invalid kdump config value for option $config_opt." return 1; @@ -252,6 +268,9 @@ check_config() echo "Deprecated kdump config option: $config_opt. Refer to kdump.conf manpage for alternatives." return 1 ;; + default) + check_config_default_action "$config_val" || return 1 + ;; *) echo "Invalid kdump config option $config_opt" return 1;
We can do the checking for fence_kdump_nodes while we scan kdump.conf. Rename the checking function to check_config_fence_kdump_nodes() and move the function to be around check_config()
Signed-off-by: WANG Chao chaowang@redhat.com Acked-by: Vivek Goyal vgoyal@redhat.com --- kdumpctl | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 49ff98e..c144c19 100755 --- a/kdumpctl +++ b/kdumpctl @@ -226,6 +226,21 @@ backup_initrd() fi }
+check_config_fence_kdump_nodes() +{ + local hostname=`hostname` + local nodes="$1" + + for node in $nodes; do + if [ "$node" = "$hostname" ]; then + echo "Option fence_kdump_nodes cannot contain $hostname" + return 1 + fi + done + + return 0 +} + check_config_default_action() { local default_action=$1 @@ -258,7 +273,7 @@ check_config() case "$config_opt" in #* | "") ;; - raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|force_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes) + raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|force_rebuild|dracut_args|fence_kdump_args) [ -z "$config_val" ] && { echo "Invalid kdump config value for option $config_opt." return 1; @@ -271,6 +286,9 @@ check_config() default) check_config_default_action "$config_val" || return 1 ;; + fence_kdump_nodes) + check_config_fence_kdump_nodes "$config_val" || return 1 + ;; *) echo "Invalid kdump config option $config_opt" return 1; @@ -278,8 +296,6 @@ check_config() esac done < $KDUMP_CONFIG_FILE
- check_fence_kdump_config || return 1 - return 0 }
@@ -735,21 +751,6 @@ check_kdump_feasibility() fi }
-check_fence_kdump_config() -{ - local hostname=`hostname` - local nodes=$(get_option_value "fence_kdump_nodes") - - for node in $nodes; do - if [ "$node" = "$hostname" ]; then - echo "Option fence_kdump_nodes cannot contain $hostname" - return 1 - fi - done - - return 0 -} - check_dump_feasibility() { if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
We only checked `hostname` is in listed in fence_kdump_nodes of kdump.conf. But `hostname -s` also can not be listed in fence_kdump_nodes. Now adding this check for short hostname.
Signed-off-by: WANG Chao chaowang@redhat.com Acked-by: Vivek Goyal vgoyal@redhat.com --- kdumpctl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index c144c19..9f9af2d 100755 --- a/kdumpctl +++ b/kdumpctl @@ -228,12 +228,11 @@ backup_initrd()
check_config_fence_kdump_nodes() { - local hostname=`hostname` local nodes="$1"
for node in $nodes; do - if [ "$node" = "$hostname" ]; then - echo "Option fence_kdump_nodes cannot contain $hostname" + if [ "$node" = $(hostname) -o "$node" = $(hostname -s) ]; then + echo "Option fence_kdump_nodes cannot contain $(hostname) or $(hostname -s)" return 1 fi done