#18 _update_crashkernel: bail if no existing crashkernel arg (#2243068)
Opened 7 months ago by adamwill. Modified 7 months ago
rpms/ adamwill/kexec-tools dont-enable-kdump-on-update  into  rawhide

file modified
+4
@@ -1581,6 +1581,10 @@ 

  	_kernel=$1

  	_dump_mode=$(get_dump_mode_by_kernel "$_kernel")

  	_old_ck=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel)

+ 	# don't add a crashkernel arg if we didn't already have one

+ 	if [[ -z "$_old_ck" ]]; then

+ 		return

+ 	fi

  	_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_ck=$(kdump_get_arch_recommend_crashkernel "$_dump_mode" "$_kver")

5b31b09 had a significant effect
not clearly disclosed in the commit message: it results in a
crashkernel arg being added on every kernel install or kexec-tools
package update, even if there was no existing crashkernel arg.

Previously in reset_crashkernel_after_update() (called in the
kexec-tools %posttrans), we looped over existing kernels and read
their crashkernel arg. If it was "auto", we did one thing. If it
was otherwise non-empty, we did another thing. If it was empty, we
did nothing.

Previously in reset_crashkernel_for_installed_kernel() (called in
the 92-crashkernel.install scriptlet, which is run by
kernel-install add every time a kernel is installed), we read
the crashkernel args for the newly-installed and currently-
running kernels and if they were non-equal, made the arg for the
newly-installed kernel the same as the one for the currently-
running kernel (this seems somewhat odd, but that's off-topic
here). If there was no existing arg for either kernel (which would
be the usual case if the user had not explicitly set one, or used
the anaconda option to enable kdump at install time), we would do
nothing.

After 5b31b09, both these functions were turned into fairly thin
wrappers around _update_crashkernel, which does not care if
there is any existing crashkernel arg. It just always makes
the crashkernel arg be the currently 'recommended' one, no
matter whether it's currently set at all or what it's set to.

The commit message explained:

"Another change brought by this simplification is kexec-tools will also
set up the kernel crashkernel parameter for a fresh install (previously
it's limited to osbuild)."

but that's not a clear enough explanation, I don't think, and the
behaviour is really not desired. We do not want crashkernel to be
set for every Fedora installation, which is the effect of
the current implementation. In the worst case - on a system with
1G of RAM - it eats nearly 20% of RAM!

This changes _update_crashkernel to only do anything if the
kernel being operated on has an existing crashkernel arg. This
should better match previous behaviour and user expectations.

Signed-off-by: Adam Williamson awilliam@redhat.com

I'm not really sure this is 'correct' - it's entirely possible there's a case where the newly-installed kernel won't have a crashkernel= parameter yet, but we somehow "know" that it ought to get one. But I at least wanted to post something to be reviewed, and I definitely don't think the current behaviour is acceptable.

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/97a2afc42c014d048d45bd128c2c085e

As written in the Bugzilla [1] I don't think that is the way to go. auto_reset_crashkernel is meant to always set the crashkernel if set to 'yes'. If this behavior is not desired then it is better to simply set it's default to 'no'.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2243068#c8

Metadata