Hi everybody,
this series got spit from the CLI rework series as they are independent and can be merged earlier to reduce the overall series size. While most of the patches were part of the rfc v1 of the CLI rework series patches 9-13 are new.
Thanks Philipp
v1 -> v2 - Added new patch 13, which I've noticed during reviewing the patch Coiby posted to split up the package. - Improved the commit message of patch 10 with comments from Pingfan. - Added Pingfan's Reviewed-by to all patches contained in v1.
rfc -> v1: - Rebased to latest rawhide - Patch 1 added two more fixes. - Patch 2,4&8 improved the commit message - Patch 3 dropped definition of _is_valid_kver as it was already present on latest rawhide - Added new patches 9-12
Philipp Rudo (13): Fix various shellcheck findings kdump-lib: make is_zstd_command_available more generic kdump-lib: simplify _get_kdump_kernel_version kdumpctl: drop _get_current_running_kernel_path kdumpctl: drop condrestart subcommand kdumpctl: simplify _update_kernel_cmdline kdumpctl: Prevent option --fadump on non-PPC in reset_crashkernel kdumpctl: Stop updating grub config in reset_crashkernel kdump.conf: Remove option override_resettable spec: Clean up handling of dracut files spec: Silence unversioned Obsolete warning kdump-lib: Harden _crashkernel_add spec: Drop special handling for IA64 machines
gen-kdump-conf.sh | 6 -- kdump-lib.sh | 179 +++++++++++++++++++++---------- kdump.conf.5 | 8 -- kdumpctl | 236 +++++++++++++---------------------------- kexec-tools.spec | 72 ++++--------- mkdumprd | 57 +--------- mkfadumprd | 2 +- spec/kdump-lib_spec.sh | 38 ++++--- 8 files changed, 237 insertions(+), 361 deletions(-)
This includes fixes for
SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns. SC2005 (style): Useless echo? Instead of 'echo $(cmd)', just use 'cmd'. SC2162 (info): read without -r will mangle backslashes. SC2086 (info): Double quote to prevent globbing and word splitting. SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).
In addition add some source hints to prevent false positive findings.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdump-lib.sh | 10 +++++----- kdumpctl | 8 ++++++-- 2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 16238c5..dfb663f 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -197,7 +197,7 @@ get_bind_mount_source() local _fsroot _src_nofsroot
_mnt=$(df "$1" | tail -1 | awk '{print $NF}') - _path=${1#$_mnt} + _path=${1#"$_mnt"}
_src=$(get_mount_info SOURCE target "$_mnt" -f) _opt=$(get_mount_info OPTIONS target "$_mnt" -f) @@ -214,7 +214,7 @@ get_bind_mount_source() echo "$_mnt$_path" && return fi
- _fsroot=${_src#${_src_nofsroot}[} + _fsroot=${_src#"${_src_nofsroot}"[} _fsroot=${_fsroot%]} _mnt=$(get_mount_info TARGET source "$_src_nofsroot" -f)
@@ -223,7 +223,7 @@ get_bind_mount_source() local _subvol _subvol=${_opt#*subvol=} _subvol=${_subvol%,*} - _fsroot=${_fsroot#$_subvol} + _fsroot=${_fsroot#"$_subvol"} fi echo "$_mnt$_fsroot$_path" } @@ -270,7 +270,7 @@ kdump_get_persistent_dev() dev=$(blkid -L "${dev#LABEL=}") ;; esac - echo $(get_persistent_dev "$dev") + get_persistent_dev "$dev" }
is_ostree() @@ -823,7 +823,7 @@ get_recommend_size() while read -r -d , range; do # need to use non-default IFS as double spaces are used as a # single delimiter while commas aren't... - IFS=, read start start_unit end end_unit size <<< \ + IFS=, read -r start start_unit end end_unit size <<< \ "$(echo "$range" | sed -n "s/([0-9]+)([GT]?)-([0-9]*)([GT]?):([0-9]+[MG])/\1,\2,\3,\4,\5/p")"
# aka. 102400T diff --git a/kdumpctl b/kdumpctl index 1f7f4ce..7844488 100755 --- a/kdumpctl +++ b/kdumpctl @@ -29,14 +29,17 @@ if [[ -f /etc/sysconfig/kdump ]]; then fi
[[ $dracutbasedir ]] || dracutbasedir=/usr/lib/dracut -. $dracutbasedir/dracut-functions.sh +# shellcheck source=/dev/null +. "$dracutbasedir"/dracut-functions.sh
if [[ ${__SOURCED__:+x} ]]; then KDUMP_LIB_PATH=. else KDUMP_LIB_PATH=/lib/kdump fi +# shellcheck source=./kdump-lib.sh . $KDUMP_LIB_PATH/kdump-lib.sh +# shellcheck source=./kdump-logger.sh . $KDUMP_LIB_PATH/kdump-logger.sh
#initiate the kdump logger @@ -247,7 +250,7 @@ restore_default_initrd() if [[ $default_checksum != "$backup_checksum" ]]; then dwarn "WARNING: checksum mismatch! Can't restore original initrd.." else - rm -f $INITRD_CHECKSUM_LOCATION + rm -f "$INITRD_CHECKSUM_LOCATION" if mv "$DEFAULT_INITRD_BAK" "$DEFAULT_INITRD"; then derror "Restoring original initrd as fadump mode is disabled." sync -f "$DEFAULT_INITRD" @@ -497,6 +500,7 @@ check_drivers_modified() # If it's dump target is on block device, detect the block driver _target=$(get_block_dump_target) if [[ -n $_target ]]; then + # shellcheck disable=SC2317 _record_block_drivers() { local _drivers
There is value to use the function in other places as well. For example it can be used to check whether optional dependencies, like grubby, are installed. Thus make it more generic so it can be reused in later commits.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdump-lib.sh | 4 ++-- mkdumprd | 2 +- mkfadumprd | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index dfb663f..3b29c4d 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -48,9 +48,9 @@ is_squash_available() done }
-is_zstd_command_available() +has_command() { - [[ -x "$(command -v zstd)" ]] + [[ -x $(command -v "$1") ]] }
dracut_have_option() diff --git a/mkdumprd b/mkdumprd index f93874d..4feac28 100644 --- a/mkdumprd +++ b/mkdumprd @@ -443,7 +443,7 @@ handle_default_dump_target if ! have_compression_in_dracut_args; then if is_squash_available && dracut_have_option "--squash-compressor"; then add_dracut_arg "--squash-compressor" "zstd" - elif is_zstd_command_available; then + elif has_command zstd; then add_dracut_arg "--compress" "zstd" fi fi diff --git a/mkfadumprd b/mkfadumprd index 719e150..dd6840f 100755 --- a/mkfadumprd +++ b/mkfadumprd @@ -64,7 +64,7 @@ _dracut_isolate_args=(
# Use zstd compression method, if available if ! have_compression_in_dracut_args; then - if is_zstd_command_available; then + if has_command zstd; then _dracut_isolate_args+=(--compress zstd) fi fi
Only check whether modules for a given kernel version are installed instead of searching for a kernel image. It's safer to assume that every kernel uses kernel modules compared to that it follows certain naming conventions. Furthermore it is much more lightweight and thus allows to determine the KDUMP_KERNELVER much earlier for every command in kdumpctl.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdump-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 3b29c4d..3f2ab6b 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -599,7 +599,7 @@ _get_kdump_kernel_version()
_version_nondebug=${_version%+debug} _version_nondebug=${_version_nondebug%-debug} - if [[ -f "$(prepare_kdump_kernel "$_version_nondebug")" ]]; then + if _is_valid_kver "$_version_nondebug"; then dinfo "Use of debug kernel detected. Trying to use $_version_nondebug" echo "$_version_nondebug" else
_get_current_running_kernel_path is identical to _find_kernel_path_by_release $(uname -r) so simply use this instead of defining a new function.
While at it simplify reset_crashkernel slightly. This changes the behavior of the function for the case when KDUMP_KERNELVER is defined but no kernel with this version is installed. Before, the missing kernel is silently ignored and the currently running kernel is used instead. Now, kdumpctl will exit with an error.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdumpctl | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 7844488..8cd423c 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1422,18 +1422,6 @@ _find_kernel_path_by_release() echo -n "$_kernel_path" }
-_get_current_running_kernel_path() -{ - local _release _path - - _release=$(uname -r) - if _path=$(_find_kernel_path_by_release "$_release"); then - echo -n "$_path" - else - return 1 - fi -} - _update_kernel_cmdline() { local _kernel_path=$1 _crashkernel=$2 _dump_mode=$3 _fadump_val=$4 @@ -1628,14 +1616,11 @@ reset_crashkernel() # - use KDUMP_KERNELVER if it's defined # - use current running kernel if [[ -z $_grubby_kernel_path ]]; then - if [[ -z $KDUMP_KERNELVER ]] || - ! _kernel_path=$(_find_kernel_path_by_release "$KDUMP_KERNELVER"); then - if ! _kernel_path=$(_get_current_running_kernel_path); then - derror "no running kernel found" - exit 1 - fi + [[ -n $KDUMP_KERNELVER ]] || KDUMP_KERNELVER=$(uname -r) + if ! _kernels=$(_find_kernel_path_by_release "$KDUMP_KERNELVER"); then + derror "no kernel for version $KDUMP_KERNELVER found" + exit 1 fi - _kernels=$_kernel_path else _kernels=$(_get_all_kernels_from_grubby "$_grubby_kernel_path") fi
condrestart is a left over from the time of SysVinit that is no longer needed since the kexec-tools switched to systemd (10c91a1 ("Removing sysvinit files") plus the one before). What's especially intriguing is that from the beginning (0112f36 ("- Add a kdump sysconfig file and init script - Spec file additions for pre/post install/uninstall")) the sub-command never did any actual work (other than not returning an error). Thus simply remove the condrestart sub-command.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdumpctl | 2 -- 1 file changed, 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8cd423c..7ef23a8 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1795,8 +1795,6 @@ main() rebuild) rebuild ;; - condrestart) ;; - propagate) propagate_ssh_key ;;
_update_kernel_cmdline handles two cmdline parameters at once. This does not only make the function itself but also its callers more complicated than necessary. For example in _update_crashkernel the fadump gets "updated" to the value that has been read from grubby. Thus simplify _update_kernel_cmdline to only update one parameter at once.
While at it shorten some variable named in the callers.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdumpctl | 106 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 56 insertions(+), 50 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 7ef23a8..5088c32 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1424,21 +1424,33 @@ _find_kernel_path_by_release()
_update_kernel_cmdline() { - local _kernel_path=$1 _crashkernel=$2 _dump_mode=$3 _fadump_val=$4 + local _kernel _param _old _new + + _kernel="$1" + _param="$2" + _old="$3" + _new="$4" + + # _new = "" removes a _param, so _new = _old = "" is fine + [[ -n "$_new" ]] && [[ "$_old" == "$_new" ]] && return 1
if is_ostree; then - if rpm-ostree kargs | grep -q "crashkernel="; then - rpm-ostree kargs --replace="crashkernel=$_crashkernel" + if [[ -z "$_new" ]]; then + rpm-ostree kargs --delete="$_param=$_old" + elif [[ -n "$_old" ]]; then + rpm-ostree kargs --replace="$_param=$_new" else - rpm-ostree kargs --append="crashkernel=$_crashkernel" + rpm-ostree kargs --append="$_param=$_new" fi - else - grubby --args "crashkernel=$_crashkernel" --update-kernel "$_kernel_path" - if [[ $_dump_mode == kdump ]]; then - grubby --remove-args="fadump" --update-kernel "$_kernel_path" + elif has_command grubby; then + if [[ -n "$_new" ]]; then + grubby --update-kernel "$_kernel" --args "$_param=$_new" else - grubby --args="fadump=$_fadump_val" --update-kernel "$_kernel_path" + grubby --update-kernel "$_kernel" --remove-args "$_param" fi + else + derror "Unable to update kernel command line" + exit 1 fi
# Don't use "[[ CONDITION ]] && COMMAND" which returns 1 under false CONDITION @@ -1530,9 +1542,11 @@ _read_kernel_arg_in_grub_etc_default()
reset_crashkernel() { - local _opt _val _dump_mode _fadump_val _reboot _grubby_kernel_path _kernel _kernels - local _old_crashkernel _new_crashkernel _new_dump_mode _crashkernel_changed - local _new_fadump_val _old_fadump_val _what_is_updated + local _opt _val _fadump_val _reboot _grubby_kernel_path _kernel _kernels + local _dump_mode _new_dump_mode + local _has_changed _needs_reboot + local _old_ck _new_ck + local _old_fadump _new_fadump
for _opt in "$@"; do case "$_opt" in @@ -1570,14 +1584,10 @@ reset_crashkernel() # modifying the kernel command line so there is no need for kexec-tools # to repeat it. if is_ostree; then - _old_crashkernel=$(rpm-ostree kargs | sed -n -E 's/.*(^|\s)crashkernel=(\S*).*/\2/p') - _new_dump_mode=kdump - _new_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") - if [[ $_old_crashkernel != "$_new_crashkernel" ]]; then - _update_kernel_cmdline "" "$_new_crashkernel" "$_new_dump_mode" "" - if [[ $_reboot == yes ]]; then - systemctl reboot - fi + _old_ck=$(rpm-ostree kargs | sed -n -E 's/.*(^|\s)crashkernel=(\S*).*/\2/p') + _new_ck=$(kdump_get_arch_recommend_crashkernel kdump) + if _update_kernel_cmdline "" crashkernel "$_old_ck" "$_new_ck"; then + [[ $_reboot == yes ]] && systemctl reboot fi return fi @@ -1626,35 +1636,33 @@ reset_crashkernel() fi
for _kernel in $_kernels; do + _has_changed="" if [[ -z $_dump_mode ]]; then _new_dump_mode=$(get_dump_mode_by_kernel "$_kernel") - _new_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") - _new_fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump) + _new_ck=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") + _new_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) else _new_dump_mode=$_dump_mode - _new_crashkernel=$_crashkernel - _new_fadump_val=$_fadump_val + _new_ck=$_crashkernel + _new_fadump=$_fadump_val fi
- _old_crashkernel=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) - _old_fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump) - if [[ $_old_crashkernel != "$_new_crashkernel" || $_old_fadump_val != "$_new_fadump_val" ]]; then - _update_kernel_cmdline "$_kernel" "$_new_crashkernel" "$_new_dump_mode" "$_new_fadump_val" - if [[ $_reboot != yes ]]; then - if [[ $_old_crashkernel != "$_new_crashkernel" ]]; then - _what_is_updated="Updated crashkernel=$_new_crashkernel" - else - # This case happens only when switching between fadump=on and fadump=nocma - _what_is_updated="Updated fadump=$_new_fadump_val" - fi - dwarn "$_what_is_updated for kernel=$_kernel. Please reboot the system for the change to take effect." - fi - _crashkernel_changed=yes + _old_ck=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) + _old_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) + if _update_kernel_cmdline "$_kernel" fadump "$_old_fadump" "$_new_fadump"; then + _has_changed="Updated fadump=$_new_fadump" + fi + if _update_kernel_cmdline "$_kernel" crashkernel "$_old_ck" "$_new_ck"; then + _has_changed="Updated crashkernel=$_new_ck" + fi + if [[ -n "$_has_changed" ]]; then + _needs_reboot=1 + dwarn "$_has_changed for kernel=$_kernel. Please reboot the system for the change to take effect." fi done
- if [[ $_reboot == yes && $_crashkernel_changed == yes ]]; then - reboot + if [[ $_reboot == yes && $_needs_reboot == 1 ]]; then + systemctl reboot fi }
@@ -1670,21 +1678,19 @@ _is_bootloader_installed()
_update_crashkernel() { - local _kernel _kver _dump_mode _old_default_crashkernel _new_default_crashkernel _fadump_val _msg + local _kernel _kver _dump_mode _msg + local _old_ck _new_ck
_kernel=$1 _dump_mode=$(get_dump_mode_by_kernel "$_kernel") - _old_default_crashkernel=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) + _old_ck=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) _kver=$(parse_kver_from_path "$_kernel") # The second argument is for the case of aarch64, where installing a 64k variant on a 4k kernel, or vice versa - _new_default_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_dump_mode" "$_kver") - if [[ $_old_default_crashkernel != "$_new_default_crashkernel" ]]; then - _fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump) - if _update_kernel_cmdline "$_kernel" "$_new_default_crashkernel" "$_dump_mode" "$_fadump_val"; then - _msg="For kernel=$_kernel, crashkernel=$_new_default_crashkernel now. Please reboot the system for the change to take effet." - _msg+=" Note if you don't want kexec-tools to manage the crashkernel kernel parameter, please set auto_reset_crashkernel=no in /etc/kdump.conf." - dinfo "$_msg" - fi + _new_ck=$(kdump_get_arch_recommend_crashkernel "$_dump_mode" "$_kver") + if _update_kernel_cmdline "$_kernel" crashkernel "$_old_ck" "$_new_ck"; then + _msg="For kernel=$_kernel, crashkernel=$_new_ck now. Please reboot the system for the change to take effect." + _msg+=" Note if you don't want kexec-tools to manage the crashkernel kernel parameter, please set auto_reset_crashkernel=no in /etc/kdump.conf." + dinfo "$_msg" fi }
Prevent the --fadump option to be used on non-PPC systems. This not only prevents user errors but also guarantees that _dump_mode and _fadump_val are empty on these systems.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdumpctl | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 5088c32..113d8bc 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1551,6 +1551,10 @@ reset_crashkernel() for _opt in "$@"; do case "$_opt" in --fadump=*) + if [[ $(uname -m) != ppc64le ]]; then + derror "Option --fadump only valid on PPC" + exit 1 + fi _val=${_opt#*=} if _dump_mode=$(get_dump_mode_by_fadump_val $_val); then _fadump_val=$_val @@ -1592,12 +1596,7 @@ reset_crashkernel() return fi
- # For non-ppc64le systems, the dump mode is always kdump since only ppc64le - # has FADump. - if [[ -z $_dump_mode && $(uname -m) != ppc64le ]]; then - _dump_mode=kdump - _fadump_val=off - fi + [[ $(uname -m) != ppc64le ]] && _dump_mode=kdump
# If the dump mode is determined, we can also know the default crashkernel value if [[ -n $_dump_mode ]]; then
With multiple kernel variants on the same architecture, e.g. the 4k and 64k kernel on aarch64, we can no longer assume that the crashkernel value for the currently running kernel will work for all installed kernels. This also means that we can no longer update the grub config as we don't know which value to set it to. Thus get the crashkernel value for each kernel and stop updating the grub config.
While at it merge the _new_fadump and _fadump_val variables and remove _read_kernel_arg_in_grub_etc_default which has no user.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdumpctl | 96 +++----------------------------------------------------- 1 file changed, 5 insertions(+), 91 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 113d8bc..6e4685f 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1480,69 +1480,9 @@ _get_all_kernels_from_grubby() echo -n "$_kernels" }
-GRUB_ETC_DEFAULT="/etc/default/grub" -# Update a kernel parameter in default grub conf -# -# If a value is specified, it will be inserted in the end. Otherwise it -# would remove given kernel parameter. -# -# Note this function doesn't address the following cases, -# 1. The kernel ignores everything on the command line after a '--'. So -# simply adding the new entry to the end will fail if the cmdline -# contains a --. -# 2. If the value for a parameter contains spaces it can be quoted using -# double quotes, for example param="value with spaces". This will -# break the [^[:space:]"] regex for the value. -# 3. Dashes and underscores in the parameter name are equivalent. So -# some_parameter and some-parameter are identical. -# 4. Some parameters, e.g. efivar_ssdt, can be given multiple times. -# 5. Some kernel parameters, e.g. quiet, doesn't have value -# -# $1: the name of the kernel command line parameter -# $2: new value. If empty, given parameter would be removed -_update_kernel_arg_in_grub_etc_default() -{ - local _para=$1 _val=$2 _para_val - - if [[ $(uname -m) == s390x ]]; then - return - fi - - if [[ -n $_val ]]; then - _para_val="$_para=$_val" - fi - - # Update the command line /etc/default/grub, i.e. - # on the line that starts with 'GRUB_CMDLINE_LINUX=', - # 1) remove $para=$val if the it's the first arg - # 2) remove all occurences of $para=$val - # 3) insert $_para_val to end - # 4) remove duplicate spaces left over by 1) or 2) or 3) - # 5) remove space at the beginning of the string left over by 1) or 2) or 3) - # 6) remove space at the end of the string left over by 1) or 2) or 3) - sed -i -E "/^GRUB_CMDLINE_LINUX=/ { - s/"${_para}=[^[:space:]"]*/"/g; - s/[[:space:]]+${_para}=[^[:space:]"]*/ /g; - s/"$/ ${_para_val}"/ - s/[[:space:]]+/ /g; - s/(")[[:space:]]+/\1/g; - s/[[:space:]]+(")/\1/g; - }" "$GRUB_ETC_DEFAULT" -} - -# Read the kernel arg in default grub conf. - -# Note reading a kernel parameter that doesn't have a value isn't supported. -# -# $1: the name of the kernel command line parameter -_read_kernel_arg_in_grub_etc_default() -{ - sed -n -E "s/^GRUB_CMDLINE_LINUX=.*[[:space:]"]${1}=([^[:space:]"]*).*$/\1/p" "$GRUB_ETC_DEFAULT" -} - reset_crashkernel() { - local _opt _val _fadump_val _reboot _grubby_kernel_path _kernel _kernels + local _opt _val _reboot _grubby_kernel_path _kernel _kernels local _dump_mode _new_dump_mode local _has_changed _needs_reboot local _old_ck _new_ck @@ -1555,13 +1495,12 @@ reset_crashkernel() derror "Option --fadump only valid on PPC" exit 1 fi - _val=${_opt#*=} - if _dump_mode=$(get_dump_mode_by_fadump_val $_val); then - _fadump_val=$_val - else + _new_fadump=${_opt#*=} + if ! _dump_mode=$(get_dump_mode_by_fadump_val $_new_fadump); then derror "failed to determine dump mode" exit fi + [[ "$_new_fadump" == off ]] && _new_fadump="" ;; --kernel=*) _val=${_opt#*=} @@ -1598,29 +1537,6 @@ reset_crashkernel()
[[ $(uname -m) != ppc64le ]] && _dump_mode=kdump
- # If the dump mode is determined, we can also know the default crashkernel value - if [[ -n $_dump_mode ]]; then - _crashkernel=$(kdump_get_arch_recommend_crashkernel "$_dump_mode") - fi - - # If --kernel-path=ALL, update GRUB_CMDLINE_LINUX in /etc/default/grub. - # - # An exception case is when the ppc64le user doesn't specify the fadump value. - # In this case, the dump mode would be determined by parsing the kernel - # command line of the kernel(s) to be updated thus don't update GRUB_CMDLINE_LINUX. - # - # The following code has been simplified because of what has been done early, - # - set the dump mode as kdump for non-ppc64le cases - # - retrieved the default crashkernel value for given dump mode - if [[ $_grubby_kernel_path == ALL && -n $_dump_mode ]]; then - _update_kernel_arg_in_grub_etc_default crashkernel "$_crashkernel" - # remove the fadump if fadump is disabled - if [[ $_fadump_val == off ]]; then - _fadump_val="" - fi - _update_kernel_arg_in_grub_etc_default fadump "$_fadump_val" - fi - # If kernel-path not specified, either # - use KDUMP_KERNELVER if it's defined # - use current running kernel @@ -1638,15 +1554,13 @@ reset_crashkernel() _has_changed="" if [[ -z $_dump_mode ]]; then _new_dump_mode=$(get_dump_mode_by_kernel "$_kernel") - _new_ck=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") _new_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) else _new_dump_mode=$_dump_mode - _new_ck=$_crashkernel - _new_fadump=$_fadump_val fi
_old_ck=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) + _new_ck=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") _old_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) if _update_kernel_cmdline "$_kernel" fadump "$_old_fadump" "$_new_fadump"; then _has_changed="Updated fadump=$_new_fadump"
When override_resettable was introduced in 2013 with 4b850d2 ("Check if block device as dump target is resettable") it was forgotten to add the new option to check_config (today the function is called parse_config). So if a user would have set override_resettable check_config would have returned an error ("Invalid kdump config option override_resettable") and starting the kdump service would have failed. As there has been no bug report in the last ~10 years it is safe to assume that the option was never used. Thus simply remove the option.
Fixes: 4b850d2 ("Check if block device as dump target is resettable") Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- gen-kdump-conf.sh | 6 ------ kdump.conf.5 | 8 ------- mkdumprd | 55 ----------------------------------------------- 3 files changed, 69 deletions(-)
diff --git a/gen-kdump-conf.sh b/gen-kdump-conf.sh index 4abcd39..a8e7fdd 100755 --- a/gen-kdump-conf.sh +++ b/gen-kdump-conf.sh @@ -157,12 +157,6 @@ generate() # force_no_rebuild and force_rebuild options are mutually # exclusive and they should not be set to 1 simultaneously. # -# override_resettable <0 | 1> -# - Usually an unresettable block device can't be a dump target. -# Specifying 1 when you want to dump even though the block -# target is unresettable -# By default, it is 0, which will not try dumping destined to fail. -# # dracut_args <arg(s)> # - Pass extra dracut options when rebuilding kdump initrd. # diff --git a/kdump.conf.5 b/kdump.conf.5 index ec28552..9117aa7 100644 --- a/kdump.conf.5 +++ b/kdump.conf.5 @@ -218,14 +218,6 @@ force_no_rebuild and force_rebuild options are mutually exclusive and they should not be set to 1 simultaneously. .RE
-.B override_resettable <0 | 1> -.RS -Usually an unresettable block device can't be a dump target. Specifying 1 means -that even though the block target is unresettable, the user wants to try dumping anyway. -By default, it's set to 0, which will not try something destined to fail. -.RE - - .B dracut_args <arg(s)> .RS Kdump uses dracut to generate initramfs for second kernel. This option diff --git a/mkdumprd b/mkdumprd index 4feac28..31c4b76 100644 --- a/mkdumprd +++ b/mkdumprd @@ -24,7 +24,6 @@ fi
SSH_KEY_LOCATION=$DEFAULT_SSHKEY SAVE_PATH=$(get_save_path) -OVERRIDE_RESETTABLE=0
extra_modules="" dracut_args=(--add kdumpbase --quiet --hostonly --hostonly-cmdline --hostonly-i18n --hostonly-mode strict --hostonly-nics '' --aggressive-strip -o "plymouth resume ifcfg earlykdump") @@ -309,56 +308,6 @@ handle_default_dump_target() check_size fs "$_target" }
-# $1: function name -for_each_block_target() -{ - local dev majmin - - for dev in $(get_kdump_targets); do - [[ -b $dev ]] || continue - majmin=$(get_maj_min "$dev") - check_block_and_slaves "$1" "$majmin" && return 1 - done - - return 0 -} - -#judge if a specific device with $1 is unresettable -#return false if unresettable. -is_unresettable() -{ - local path device resettable=1 - - path="/sys/$(udevadm info --query=all --path="/sys/dev/block/$1" | awk '/^P:/ {print $2}' | sed -e 's/(cciss[0-9]+/).*/\1/g' -e 's//block/.*$//')/resettable" - if [[ -f $path ]]; then - resettable="$(< "$path")" - [[ $resettable -eq 0 ]] && [[ $OVERRIDE_RESETTABLE -eq 0 ]] && { - device=$(udevadm info --query=all --path="/sys/dev/block/$1" | awk -F= '/DEVNAME/{print $2}') - derror "Error: Can not save vmcore because device $device is unresettable" - return 0 - } - fi - - return 1 -} - -#check if machine is resettable. -#return true if resettable -check_resettable() -{ - local _target _override_resettable - - _override_resettable=$(kdump_get_conf_val override_resettable) - OVERRIDE_RESETTABLE=${_override_resettable:-$OVERRIDE_RESETTABLE} - if [ "$OVERRIDE_RESETTABLE" != "0" ] && [ "$OVERRIDE_RESETTABLE" != "1" ]; then - perror_exit "override_resettable value '$OVERRIDE_RESETTABLE' is invalid" - fi - - for_each_block_target is_unresettable && return - - return 1 -} - check_crypt() { local _dev @@ -370,10 +319,6 @@ check_crypt() done }
-if ! check_resettable; then - exit 1 -fi - if ! check_crypt; then dwarn "Warning: Encrypted device is in dump path, which is not recommended, see kexec-kdump-howto.txt for more details." fi
Currently the dracut modules are first prepared in a temporary directory before they are moved to modules.d. All the preparation work can be done by a single call to 'install' per file. Thus get rid off the indirection and install the dracut modules directly to modules.d.
While at it merge the three macros to remove the prefix into one.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kexec-tools.spec | 47 ++++++++++++++++++----------------------------- 1 file changed, 18 insertions(+), 29 deletions(-)
diff --git a/kexec-tools.spec b/kexec-tools.spec index 897b6ff..a75c02c 100644 --- a/kexec-tools.spec +++ b/kexec-tools.spec @@ -228,39 +228,28 @@ mkdir -p $RPM_BUILD_ROOT/usr/share/makedumpfile/eppic_scripts/ install -m 644 makedumpfile-%{mkdf_ver}/eppic_scripts/* $RPM_BUILD_ROOT/usr/share/makedumpfile/eppic_scripts/ %endif
-%define remove_dracut_prefix() %(echo -n %1|sed 's/.*dracut-//g') -%define remove_dracut_early_kdump_prefix() %(echo -n %1|sed 's/.*dracut-early-kdump-//g') -%define remove_dracut_fadump_prefix() %(echo -n %1|sed 's/.*dracut-fadump-//g') +%define dracutdir %{_prefix}/lib/dracut/modules.d +%define remove_prefix() %(echo -n %2|sed 's/.*%1-//g')
# deal with dracut modules -mkdir -p -m755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase -cp %{SOURCE100} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE100}} -cp %{SOURCE101} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE101}} -cp %{SOURCE102} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE102}} -cp %{SOURCE104} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE104}} -cp %{SOURCE106} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE106}} -cp %{SOURCE107} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE107}} -chmod 755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE100}} -chmod 755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99kdumpbase/%{remove_dracut_prefix %{SOURCE101}} -mkdir -p -m755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99earlykdump -cp %{SOURCE108} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99earlykdump/%{remove_dracut_prefix %{SOURCE108}} -cp %{SOURCE109} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99earlykdump/%{remove_dracut_early_kdump_prefix %{SOURCE109}} -chmod 755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99earlykdump/%{remove_dracut_prefix %{SOURCE108}} -chmod 755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99earlykdump/%{remove_dracut_early_kdump_prefix %{SOURCE109}} +mkdir -p -m755 $RPM_BUILD_ROOT/%{dracutdir}/99kdumpbase +install -m 755 %{SOURCE100} $RPM_BUILD_ROOT/%{dracutdir}/99kdumpbase/%{remove_prefix dracut %{SOURCE100}} +install -m 755 %{SOURCE101} $RPM_BUILD_ROOT/%{dracutdir}/99kdumpbase/%{remove_prefix dracut %{SOURCE101}} +install -m 644 %{SOURCE102} $RPM_BUILD_ROOT/%{dracutdir}/99kdumpbase/%{remove_prefix dracut %{SOURCE102}} +install -m 644 %{SOURCE104} $RPM_BUILD_ROOT/%{dracutdir}/99kdumpbase/%{remove_prefix dracut %{SOURCE104}} +install -m 644 %{SOURCE106} $RPM_BUILD_ROOT/%{dracutdir}/99kdumpbase/%{remove_prefix dracut %{SOURCE106}} +install -m 644 %{SOURCE107} $RPM_BUILD_ROOT/%{dracutdir}/99kdumpbase/%{remove_prefix dracut %{SOURCE107}} + +mkdir -p -m755 $RPM_BUILD_ROOT/%{dracutdir}/99earlykdump +install -m 755 %{SOURCE108} $RPM_BUILD_ROOT/%{dracutdir}/99earlykdump/%{remove_prefix dracut %{SOURCE108}} +install -m 755 %{SOURCE109} $RPM_BUILD_ROOT/%{dracutdir}/99earlykdump/%{remove_prefix dracut-early-kdump %{SOURCE109}} + %ifarch ppc64 ppc64le -mkdir -p -m755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99zz-fadumpinit -cp %{SOURCE200} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99zz-fadumpinit/%{remove_dracut_fadump_prefix %{SOURCE200}} -cp %{SOURCE201} $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99zz-fadumpinit/%{remove_dracut_fadump_prefix %{SOURCE201}} -chmod 755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99zz-fadumpinit/%{remove_dracut_fadump_prefix %{SOURCE200}} -chmod 755 $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99zz-fadumpinit/%{remove_dracut_fadump_prefix %{SOURCE201}} +mkdir -p -m755 $RPM_BUILD_ROOT/%{dracutdir}/99zz-fadumpinit +install -m 755 %{SOURCE200} $RPM_BUILD_ROOT/%{dracutdir}/99zz-fadumpinit/%{remove_prefix dracut-fadump %{SOURCE200}} +install -m 755 %{SOURCE201} $RPM_BUILD_ROOT/%{dracutdir}/99zz-fadumpinit/%{remove_prefix dracut-fadump %{SOURCE201}} %endif
- -%define dracutlibdir %{_prefix}/lib/dracut -#and move the custom dracut modules to the dracut directory -mkdir -p $RPM_BUILD_ROOT/%{dracutlibdir}/modules.d/ -mv $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/* $RPM_BUILD_ROOT/%{dracutlibdir}/modules.d/ - %post # Initial installation %systemd_post kdump.service @@ -363,7 +352,7 @@ fi %config %{_udevrulesdir} %{_udevrulesdir}/../kdump-udev-throttler %endif -%{dracutlibdir}/modules.d/* +%{dracutdir}/* %dir %{_localstatedir}/crash %dir %{_sysconfdir}/kdump %dir %{_sysconfdir}/kdump/pre.d
rpmbuild throws a warning with
line 80: It's not recommended to have unversioned Obsoletes: Obsoletes: diskdumputils netdump kexec-tools-eppic
In that diskdump and netdump were last used in RHEL4 and kexec-tools-eppic was removed with Fedora 22. There is no supported update path in which a current package could replace one of these three. Thus simply drop the Obsoletes.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kexec-tools.spec | 3 --- 1 file changed, 3 deletions(-)
diff --git a/kexec-tools.spec b/kexec-tools.spec index a75c02c..ef75fd0 100644 --- a/kexec-tools.spec +++ b/kexec-tools.spec @@ -75,9 +75,6 @@ BuildRequires: zlib-devel elfutils-devel glib2-devel bzip2-devel ncurses-devel b BuildRequires: pkgconfig intltool gettext BuildRequires: systemd-rpm-macros BuildRequires: automake autoconf libtool -%ifarch %{ix86} x86_64 ppc64 ppc s390x ppc64le -Obsoletes: diskdumputils netdump kexec-tools-eppic -%endif
%ifnarch s390x Requires: systemd-udev%{?_isa}
_crashkernel_add currently always assumes the good case, i.e. that the value of the crashkernel parameter has the correct syntax and that the delta added is a number. Both doesn't have to be true when the values are provided by users. Thus add some additional checks.
Furthermore require the delta to have a explicit unit, i.e. no longer assume that is in megabytes, i.e. 100 -> 100M.
Signed-off-by: Philipp Rudo prudo@redhat.com Reviewed-by: Pingfan Liu piliu@redhat.com --- kdump-lib.sh | 163 ++++++++++++++++++++++++++++------------- spec/kdump-lib_spec.sh | 38 ++++++---- 2 files changed, 138 insertions(+), 63 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 3f2ab6b..d0f10eb 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -853,62 +853,125 @@ has_aarch64_smmu() ls /sys/devices/platform/arm-smmu-* 1> /dev/null 2>&1 }
-# $1 crashkernel="" -# $2 delta in unit of MB -_crashkernel_add() +is_memsize() { [[ "$1" =~ ^[+-]?[0-9]+[KkMmGg]?$ ]]; } + +# range defined for crashkernel parameter +# i.e. <start>-[<end>] +is_memrange() +{ + is_memsize "${1%-*}" || return 1 + [[ -n ${1#*-} ]] || return 0 + is_memsize "${1#*-}" +} + +to_bytes() +{ + local _s + + _s="$1" + is_memsize "$_s" || return 1 + + case "${_s: -1}" in + K|k) + _s=${_s::-1} + _s="$((_s * 1024))" + ;; + M|m) + _s=${_s::-1} + _s="$((_s * 1024 * 1024))" + ;; + G|g) + _s=${_s::-1} + _s="$((_s * 1024 * 1024 * 1024))" + ;; + *) + ;; + esac + echo "$_s" +} + +memsize_add() { - local _ck _add _entry _ret - local _range _size _offset - - _ck="$1" - _add="$2" - _ret="" - - if [[ "$_ck" == *@* ]]; then - _offset="@${_ck##*@}" - _ck=${_ck%@*} - elif [[ "$_ck" == *,high ]] || [[ "$_ck" == *,low ]]; then - _offset=",${_ck##*,}" - _ck=${_ck%,*} + local -a units=("" "K" "M" "G") + local i a b + + a=$(to_bytes "$1") || return 1 + b=$(to_bytes "$2") || return 1 + i=0 + + (( a += b )) + while :; do + [[ $(( a / 1024 )) -eq 0 ]] && break + [[ $(( a % 1024 )) -ne 0 ]] && break + [[ $(( ${#units[@]} - 1 )) -eq $i ]] && break + + (( a /= 1024 )) + (( i += 1 )) + done + + echo "${a}${units[$i]}" +} + +_crashkernel_parse() +{ + local ck entry + local range size offset + + ck="$1" + + if [[ "$ck" == *@* ]]; then + offset="@${ck##*@}" + ck=${ck%@*} + elif [[ "$ck" == *,high ]] || [[ "$ck" == *,low ]]; then + offset=",${ck##*,}" + ck=${ck%,*} else - _offset='' + offset='' fi
- while read -d , -r _entry; do - [[ -n "$_entry" ]] || continue - if [[ "$_entry" == *:* ]]; then - _range=${_entry%:*} - _size=${_entry#*:} + while read -d , -r entry; do + [[ -n "$entry" ]] || continue + if [[ "$entry" == *:* ]]; then + range=${entry%:*} + size=${entry#*:} else - _range="" - _size=${_entry} + range="" + size=${entry} fi
- case "${_size: -1}" in - K) - _size=${_size::-1} - _size="$((_size + (_add * 1024)))K" - ;; - M) - _size=${_size::-1} - _size="$((_size + _add))M" - ;; - G) - _size=${_size::-1} - _size="$((_size * 1024 + _add))M" - ;; - *) - _size="$((_size + (_add * 1024 * 1024)))" - ;; - esac - - [[ -n "$_range" ]] && _ret+="$_range:" - _ret+="$_size," - done <<< "$_ck," - - _ret=${_ret%,} - [[ -n "$_offset" ]] && _ret+=$_offset - echo "$_ret" + echo "$size;$range;" + done <<< "$ck," + echo ";;$offset" +} + +# $1 crashkernel command line parameter +# $2 size to be added +_crashkernel_add() +{ + local ck delta ret + local range size offset + + ck="$1" + delta="$2" + ret="" + + while IFS=';' read -r size range offset; do + if [[ -n "$offset" ]]; then + ret="${ret%,}$offset" + break + fi + + [[ -n "$size" ]] || continue + if [[ -n "$range" ]]; then + is_memrange "$range" || return 1 + ret+="$range:" + fi + + size=$(memsize_add "$size" "$delta") || return 1 + ret+="$size," + done < <( _crashkernel_parse "$ck") + + echo "${ret%,}" }
# get default crashkernel @@ -958,7 +1021,7 @@ kdump_get_arch_recommend_crashkernel() #4k kernel, mlx5 consumes extra 124M memory, and choose 150M has_mlx5 && ((_delta += 150)) fi - _ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta") + _ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "${_delta}M") elif [[ $_arch == "ppc64le" ]]; then if [[ $_dump_mode == "fadump" ]]; then _ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G" diff --git a/spec/kdump-lib_spec.sh b/spec/kdump-lib_spec.sh index 81f0f86..814f961 100644 --- a/spec/kdump-lib_spec.sh +++ b/spec/kdump-lib_spec.sh @@ -49,21 +49,33 @@ Describe 'kdump-lib' End
Describe "_crashkernel_add()" - Context "when the input parameter is '1G-4G:256M,4G-64G:320M,64G-:576M'" - delta=100 + Context "For valid input values" Parameters - "1G-4G:256M,4G-64G:320M,64G-:576M" "1G-4G:356M,4G-64G:420M,64G-:676M" - "1G-4G:256M,4G-64G:320M,64G-:576M@4G" "1G-4G:356M,4G-64G:420M,64G-:676M@4G" - "1G-4G:1G,4G-64G:2G,64G-:3G@4G" "1G-4G:1124M,4G-64G:2148M,64G-:3172M@4G" - "1G-4G:10000K,4G-64G:20000K,64G-:40000K@4G" "1G-4G:112400K,4G-64G:122400K,64G-:142400K@4G" - "300M,high" "400M,high" - "300M,low" "400M,low" - "500M@1G" "600M@1G" + "1G-4G:256M,4G-64G:320M,64G-:576M" "100M" "1G-4G:356M,4G-64G:420M,64G-:676M" + "1G-4G:256M,4G-64G:320M,64G-:576M@4G" "100M" "1G-4G:356M,4G-64G:420M,64G-:676M@4G" + "1G-4G:1G,4G-64G:2G,64G-:3G@4G" "100M" "1G-4G:1124M,4G-64G:2148M,64G-:3172M@4G" + "1G-4G:10000K,4G-64G:20000K,64G-:40000K@4G" "100M" "1G-4G:112400K,4G-64G:122400K,64G-:142400K@4G" + "1,high" "1" "2,high" + "1K,low" "1" "1025,low" + "1M@1G" "1k" "1025K@1G" + "500M@1G" "-100m" "400M@1G" + "1099511627776" "0" "1024G" End - It "should add delta to the values after ':'" - - When call _crashkernel_add "$1" "$delta" - The output should equal "$2" + It "should add delta to every value after ':'" + When call _crashkernel_add "$1" "$2" + The output should equal "$3" + End + End + Context "For invalid input values" + Parameters + "1G-4G:256M.4G-64G:320M" "100M" + "foo" "1" + "1" "bar" + End + It "shall return an error" + When call _crashkernel_add "$1" "$2" + The output should equal "" + The status should be failure End End End
The two systems are IA64 based which is no longer supported by Fedora and was only supported in RHEL up to RHEL5. So it is safe to simply drop the special handling. In case it is still wanted nevertheless the special handling should be added to kdump-lib.sh:prepare_cmdline rather than editing the sysconfig in the spec file.
Signed-off-by: Philipp Rudo prudo@redhat.com --- kexec-tools.spec | 22 ---------------------- 1 file changed, 22 deletions(-)
diff --git a/kexec-tools.spec b/kexec-tools.spec index ef75fd0..e51d496 100644 --- a/kexec-tools.spec +++ b/kexec-tools.spec @@ -258,28 +258,6 @@ servicelog_notify --remove --command=/usr/lib/kdump/kdump-migrate-action.sh 2>/d servicelog_notify --add --command=/usr/lib/kdump/kdump-migrate-action.sh --match='refcode="#MIGRATE" and serviceable=0' --type=EVENT --method=pairs_stdin >/dev/null %endif
-# This portion of the script is temporary. Its only here -# to fix up broken boxes that require special settings -# in /etc/sysconfig/kdump. It will be removed when -# These systems are fixed. - -if [ -d /proc/bus/mckinley ] -then - # This is for HP zx1 machines - # They require machvec=dig on the kernel command line - sed -e's/(^KDUMP_COMMANDLINE_APPEND.*)("$)/\1 machvec=dig"/' \ - /etc/sysconfig/kdump > /etc/sysconfig/kdump.new - mv /etc/sysconfig/kdump.new /etc/sysconfig/kdump -elif [ -d /proc/sgi_sn ] -then - # This is for SGI SN boxes - # They require the --noio option to kexec - # since they don't support legacy io - sed -e's/(^KEXEC_ARGS.*)("$)/\1 --noio"/' \ - /etc/sysconfig/kdump > /etc/sysconfig/kdump.new - mv /etc/sysconfig/kdump.new /etc/sysconfig/kdump -fi -
%postun %systemd_postun_with_restart kdump.service
Hi Philipp,
I find this patch set fails the unit tests. Can you fix the test failures? Thanks!
[root@08f80338bc07 kexec-tools-fed]# shellspec Running: /bin/sh [bash 5.2.2(1)-release] .........................................................................................................................FFFFFFFFFF........FF.FF.FF.FFFFF.FF.FFFFF.FF.FFF.
Examples: 1) kdumpctl _update_kernel_arg_in_grub_etc_default() when the given parameter is in different positions should update the kernel parameter correctly When call _update_kernel_arg_in_grub_etc_default crashkernel 333M
1.1) The contents of file /tmp/default_grub should include crashkernel=333M"
expected "GRUB_CMDLINE_LINUX="crashkernel=222M fadump=on rhgb quiet"" to include "crashkernel=333M""
# spec/kdumpctl_general_spec.sh:179
...
Finished in 8.36 seconds (user 4.66 seconds, sys 6.13 seconds) 170 examples, 33 failures
On Thu, Aug 31, 2023 at 04:02:55PM +0200, Philipp Rudo wrote:
Hi everybody,
this series got spit from the CLI rework series as they are independent and can be merged earlier to reduce the overall series size. While most of the patches were part of the rfc v1 of the CLI rework series patches 9-13 are new.
Thanks Philipp
v1 -> v2
- Added new patch 13, which I've noticed during reviewing the patch Coiby
posted to split up the package.
- Improved the commit message of patch 10 with comments from Pingfan.
- Added Pingfan's Reviewed-by to all patches contained in v1.
rfc -> v1:
- Rebased to latest rawhide
- Patch 1 added two more fixes.
- Patch 2,4&8 improved the commit message
- Patch 3 dropped definition of _is_valid_kver as it was already present on
latest rawhide
- Added new patches 9-12
Philipp Rudo (13): Fix various shellcheck findings kdump-lib: make is_zstd_command_available more generic kdump-lib: simplify _get_kdump_kernel_version kdumpctl: drop _get_current_running_kernel_path kdumpctl: drop condrestart subcommand kdumpctl: simplify _update_kernel_cmdline kdumpctl: Prevent option --fadump on non-PPC in reset_crashkernel kdumpctl: Stop updating grub config in reset_crashkernel kdump.conf: Remove option override_resettable spec: Clean up handling of dracut files spec: Silence unversioned Obsolete warning kdump-lib: Harden _crashkernel_add spec: Drop special handling for IA64 machines
gen-kdump-conf.sh | 6 -- kdump-lib.sh | 179 +++++++++++++++++++++---------- kdump.conf.5 | 8 -- kdumpctl | 236 +++++++++++++---------------------------- kexec-tools.spec | 72 ++++--------- mkdumprd | 57 +--------- mkfadumprd | 2 +- spec/kdump-lib_spec.sh | 38 ++++--- 8 files changed, 237 insertions(+), 361 deletions(-)
-- 2.41.0
Hi Coiby,
right... should have tested it myself...
I'll prepare a v3.
Thanks! Philipp
On Fri, 1 Sep 2023 14:00:20 +0800 Coiby Xu coxu@redhat.com wrote:
Hi Philipp,
I find this patch set fails the unit tests. Can you fix the test failures? Thanks!
[root@08f80338bc07 kexec-tools-fed]# shellspec Running: /bin/sh [bash 5.2.2(1)-release] .........................................................................................................................FFFFFFFFFF........FF.FF.FF.FFFFF.FF.FFFFF.FF.FFF. Examples: 1) kdumpctl _update_kernel_arg_in_grub_etc_default() when the given parameter is in different positions should update the kernel parameter correctly When call _update_kernel_arg_in_grub_etc_default crashkernel 333M 1.1) The contents of file /tmp/default_grub should include crashkernel=333M" expected "GRUB_CMDLINE_LINUX="crashkernel=222M fadump=on rhgb quiet"" to include "crashkernel=333M"" # spec/kdumpctl_general_spec.sh:179 ... Finished in 8.36 seconds (user 4.66 seconds, sys 6.13 seconds) 170 examples, 33 failuresOn Thu, Aug 31, 2023 at 04:02:55PM +0200, Philipp Rudo wrote:
Hi everybody,
this series got spit from the CLI rework series as they are independent and can be merged earlier to reduce the overall series size. While most of the patches were part of the rfc v1 of the CLI rework series patches 9-13 are new.
Thanks Philipp
v1 -> v2
- Added new patch 13, which I've noticed during reviewing the patch Coiby
posted to split up the package.
- Improved the commit message of patch 10 with comments from Pingfan.
- Added Pingfan's Reviewed-by to all patches contained in v1.
rfc -> v1:
- Rebased to latest rawhide
- Patch 1 added two more fixes.
- Patch 2,4&8 improved the commit message
- Patch 3 dropped definition of _is_valid_kver as it was already present on
latest rawhide
- Added new patches 9-12
Philipp Rudo (13): Fix various shellcheck findings kdump-lib: make is_zstd_command_available more generic kdump-lib: simplify _get_kdump_kernel_version kdumpctl: drop _get_current_running_kernel_path kdumpctl: drop condrestart subcommand kdumpctl: simplify _update_kernel_cmdline kdumpctl: Prevent option --fadump on non-PPC in reset_crashkernel kdumpctl: Stop updating grub config in reset_crashkernel kdump.conf: Remove option override_resettable spec: Clean up handling of dracut files spec: Silence unversioned Obsolete warning kdump-lib: Harden _crashkernel_add spec: Drop special handling for IA64 machines
gen-kdump-conf.sh | 6 -- kdump-lib.sh | 179 +++++++++++++++++++++---------- kdump.conf.5 | 8 -- kdumpctl | 236 +++++++++++++---------------------------- kexec-tools.spec | 72 ++++--------- mkdumprd | 57 +--------- mkfadumprd | 2 +- spec/kdump-lib_spec.sh | 38 ++++--- 8 files changed, 237 insertions(+), 361 deletions(-)
-- 2.41.0
-
Coiby Xu -
Philipp Rudo