4:08 a.m.
On Fri, Sep 3, 2021 at 8:06 PM Philipp Rudo <prudo(a)redhat.com> wrote:
Hi Kairui,
On Thu, 19 Aug 2021 19:39:01 +0800
Kairui Song <kasong(a)redhat.com> wrote:
> Add a helper `kdump_read_conf` to replace read_strip_comments.
> `kdump_read_conf` does a few more things:
>
> - remove tailing spaces.
s/tailing/trailing/
AFAIK tailing isn't wrong but trailing is much more common. At least it won the
poll i did in the kernel (git log --grep <word> --oneline | wc -l) with 2045 to
76 ;-)
Cool, thanks! I never noticed this difference before :)
> - format the content, remove duplicated spaces between name and value.
> - read from KDUMP_CONFIG_FILE (/etc/kdump.conf) directly, avoid pasting
> "/etc/kdump.conf" path everywhere in the code.
> - check if config file exists, just in case.
>
> Also unify the environmental variable, now KDUMP_CONFIG_FILE stands for
> the default config location.
>
> This helps avoid some shell pitfalls about spaces when reading config.
>
> Signed-off-by: Kairui Song <kasong(a)redhat.com>
> ---
> dracut-kdump.sh | 10 +++++-----
> dracut-module-setup.sh | 5 +++--
> kdump-lib-initramfs.sh | 3 +--
> kdump-lib.sh | 11 ++++++-----
> kdumpctl | 7 +++----
> mkdumprd | 2 +-
> 6 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/dracut-kdump.sh b/dracut-kdump.sh
> index 3e65c447..3c165b3a 100755
> --- a/dracut-kdump.sh
> +++ b/dracut-kdump.sh
> @@ -246,10 +246,10 @@ get_host_ip()
> return 0
> }
>
> -read_kdump_conf()
> +read_kdump_confs()
> {
> - if [ ! -f "$KDUMP_CONF" ]; then
> - derror "$KDUMP_CONF not found"
> + if [ ! -f "$KDUMP_CONFIG_FILE" ]; then
> + derror "$KDUMP_CONFIG_FILE not found"
> return
> fi
>
> @@ -278,7 +278,7 @@ read_kdump_conf()
> add_dump_code "dump_ssh $SSH_KEY_LOCATION $config_val"
> ;;
> esac
> - done <<< "$(read_strip_comments $KDUMP_CONF)"
> + done <<< "$(kdump_read_conf)"
> }
>
> fence_kdump_notify()
> @@ -288,7 +288,7 @@ fence_kdump_notify()
> fi
> }
>
> -read_kdump_conf
> +read_kdump_confs
> fence_kdump_notify
>
> get_host_ip
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index a1f33e8c..ff9f7fee 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -682,7 +682,8 @@ default_dump_target_install_conf()
> #install kdump.conf and what user specifies in kdump.conf
> kdump_install_conf() {
> local _opt _val _pdev
> - (read_strip_comments /etc/kdump.conf) > ${initdir}/tmp/$$-kdump.conf
> +
> + kdump_read_conf > "${initdir}/tmp/$$-kdump.conf"
>
> while read _opt _val;
> do
> @@ -711,7 +712,7 @@ kdump_install_conf() {
> dracut_install "${_val%%[[:blank:]]*}"
> ;;
> esac
> - done <<< "$(read_strip_comments /etc/kdump.conf)"
> + done <<< "$(kdump_read_conf)"
>
> kdump_install_pre_post_conf
>
> diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
> index 319f9a07..50443e55 100755
> --- a/kdump-lib-initramfs.sh
> +++ b/kdump-lib-initramfs.sh
> @@ -16,7 +16,6 @@ SSH_KEY_LOCATION="/root/.ssh/kdump_id_rsa"
> KDUMP_SCRIPT_DIR="/kdumpscripts"
> DD_BLKSIZE=512
> FINAL_ACTION="systemctl reboot -f"
> -KDUMP_CONF="/etc/kdump.conf"
> KDUMP_PRE=""
> KDUMP_POST=""
> NEWROOT="/sysroot"
> @@ -93,7 +92,7 @@ get_kdump_confs()
> esac
> ;;
> esac
> - done <<< "$(read_strip_comments $KDUMP_CONF)"
> + done <<< "$(kdump_read_conf)"
>
> if [ -z "$CORE_COLLECTOR" ]; then
> CORE_COLLECTOR="$DEFAULT_CORE_COLLECTOR"
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 4bd9751b..6ed4f5a9 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -7,6 +7,7 @@ DEFAULT_PATH="/var/crash/"
> FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump"
> FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send"
> FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
> +KDUMP_CONFIG_FILE="/etc/kdump.conf"
>
> is_fadump_capable()
> {
> @@ -80,12 +81,12 @@ strip_comments()
> echo $@ | sed -e 's/\(.*\)#.*/\1/'
> }
>
> -# Read from kdump config file stripping all comments
> -read_strip_comments()
> +# Read kdump config in well formatted style
> +kdump_read_conf()
> {
> - # strip heading spaces, and print any content starting with
> - # neither space or #, and strip everything after #
> - sed -n -e "s/^\s*\([^# \t][^#]\+\).*/\1/gp" $1
> + # Following steps are applied in order: strip tailing comment, strip tailing
space,
s/tailing/trailing/
Thanks
Philipp
> + # strip heading space, match non-empty line, remove duplicated spaces between
conf name and value
> + [ -f "$KDUMP_CONFIG_FILE" ] && sed -n -e
"s/#.*//;s/\s*$//;s/^\s*//;s/\(\S\+\)\s*\(.*\)/\1 \2/p" $KDUMP_CONFIG_FILE
> }
>
> # Check if fence kdump is configured in Pacemaker cluster
> diff --git a/kdumpctl b/kdumpctl
> index c2f0b3f2..5d9420da 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -5,7 +5,6 @@ KDUMP_KERNELVER=""
> KDUMP_KERNEL=""
> KDUMP_COMMANDLINE=""
> KEXEC_ARGS=""
> -KDUMP_CONFIG_FILE="/etc/kdump.conf"
> KDUMP_LOG_PATH="/var/log"
> MKDUMPRD="/sbin/mkdumprd -f"
> MKFADUMPRD="/sbin/mkfadumprd"
> @@ -272,7 +271,7 @@ check_config()
> return 1
> fi
> _opt_rec[$config_opt]="$config_val"
> - done <<< "$(read_strip_comments $KDUMP_CONFIG_FILE)"
> + done <<< "$(kdump_read_conf)"
>
> check_failure_action_config || return 1
> check_final_action_config || return 1
> @@ -731,7 +730,7 @@ check_ssh_config()
> *)
> ;;
> esac
> - done <<< "$(read_strip_comments $KDUMP_CONFIG_FILE)"
> + done <<< "$(kdump_read_conf)"
>
> #make sure they've configured kdump.conf for ssh dumps
> local SSH_TARGET=`echo -n $DUMP_TARGET | sed -n '/.*@/p'`
> @@ -1362,7 +1361,7 @@ reset_crashkernel() {
> grubby --args "$crashkernel_default" --update-kernel
"$entry" $zipl_arg
> [[ $zipl_arg ]] && zipl > /dev/null
> fi
> - }
> +}
>
> if [ ! -f "$KDUMP_CONFIG_FILE" ]; then
> derror "Error: No kdump config file found!"
> diff --git a/mkdumprd b/mkdumprd
> index 89a160d1..d5545114 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -443,7 +443,7 @@ do
> *)
> ;;
> esac
> -done <<< "$(read_strip_comments $conf_file)"
> +done <<< "$(kdump_read_conf)"
>
> handle_default_dump_target
>
--
Best Regards,
Kairui Song