Various fixes #18
Merged
Various fixes #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
prudo1
force-pushed
the
fixes/various_fixes_202406
branch
from
0708227
to
6da1c1d
Compare
licliu
approved these changes
Jul 3, 2024
|
LGTM. |
The sed expression misses a 's' for search and replace... Fixes: 0f6ad91 ("kdump-lib: fix prepare_cmdline") Signed-off-by: Philipp Rudo <prudo@redhat.com>
When handling fadump there are three cases we need to consider 1) When running on non-ppc64le systems In this case _dump_mode=kdump and _new_fadump='' always. In other words we don't need to care updating the fadump parameter on the kernel command line. We could always remove it like the code did so far. But should we remove it when we never set it? In particular as that might overwrite a change explicitly made by the user (for whatever reason) 2) When running on ppc64le and the user provided --fadump option In this case _new_fadump and _dump_mode are set accordingly to what the user provided. Thus we need to update both the crashkernel (in case fadump was turned on/off) and the fadump (in case the fadump mode changed) parameters. 3) When running on ppc64le but the user did not provide --fadump option In this case both _new_fadump='' and _dump_mode=''. In this case we take over the previously set fadump parameter. Which means that we don't need to update the fadump parameter at all. We do need to check whether the _new_dump_mode is fadump or kdump though so we use the correct (new) default crashkernel value. In the three cases only the second one needs to update the fadump parameter. Reflect this discussion in code. This also fixes a bug that always prints kdump: Updated fadump= for kernel=$kernel. Please reboot the system for the change to take effect. when the crashkernel= parameter is unchanged as well as reboots the system, if --reboot is provided. Even for non-ppc architectures. Fixes: 140da74 ("rewrite reset_crashkernel to support fadump and to used by RPM scriptlet") Signed-off-by: Philipp Rudo <prudo@redhat.com>
There is no description for parameter --reboot for reset-crashkernel. Thus add one. Suggested-by: Lichen Liu <lichliu@redhat.com> Signed-off-by: Philipp Rudo <prudo@redhat.com>
The kexec-dmesg.log is only saved after the initial dump attempt but not for the failure_action. So in case the initial dump attempt failed and the failure_action is dump_to_rootfs the kexec-dmesg.log is missing. Fix that by calling save_log also after executing the failure_action. Fixes: 3d70f8b ("logger: save log after all kdump progress finished") Signed-off-by: Philipp Rudo <prudo@redhat.com>
When creating the kdump-utils subpackage the files for earlykdump were incorrectly renamed to kdump.sh and kdump-module-setup.sh rather than the expected early-kdump.sh and module-setup.sh. These incorrect file names then got transferred to the newly created Makefile with fe372af ("Upstream kdump-utils"). With those incorrect file names dracut fails when trying to add the earlykdump module with # dracut --add earlykdump kdump.img dracut[E]: Module 'earlykdump' cannot be installed. Fix the file names to allow installing the earlykdump module. Fixes: 372b4c6 ("Add kdump-utils subpackage") Signed-off-by: Philipp Rudo <prudo@redhat.com>
The option --dt-no-old-root only impacts the kexec_load system call. Since we switched to use kexec_file_load as default system call and remove support for the kexec_load system call for RHEL10 there is no more need to have it set per default. Thus remove it. Note: Keep the option for ppc64(be) as it doesn't set option -s per default. So kexec-tools will fall back to kexec_load in case kexec_file_load is not configured. Signed-off-by: Philipp Rudo <prudo@redhat.com>
The 'file' utility is no longer installed per default. In addition there was an update to it so it now reports the file type as application/vnd.microsoft.portable-executable. Thus fall back to use objdump to avoid adding yet an other dependency for kdump-utils and deal with different versions of 'file'. Note: This has the small drawback that objdump is arch specific. I.e. examining a aarch64 UKI on a x86_64 machine will return an 'file format not recognized' error. Signed-off-by: Philipp Rudo <prudo@redhat.com>
This rewrite removes an unneeded global variable and the need to manually keep the command and debug print in sync. In addition it allows to easily check for unsupported options and print an error message. This becomes important as the kexec_load system call (option -c) will be disabled in the RHEL10 but not the Fedora kernel. So having this patch allows to have a simple RHEL-only patch without deviating from upstream too much. Signed-off-by: Philipp Rudo <prudo@redhat.com>
prudo1
force-pushed
the
fixes/various_fixes_202406
branch
from
6da1c1d
to
52407b1
Compare
|
fixed conflicts due to merged PR #12 . No code change. |
|
Thanks for resolving the conflicts! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the last test cycle and the following discussion several issues were found. This series fixes most of them.
Note: The last commit of the series depends on PR #12. Unfortunately I couldn't find a way to tell GitHub this connection. Thus this PR also contains the identical commit to the one from PR #12.
Depends on PR #12