Implement KDUMP_AUTO_RESIZE #123
Conversation
ptesarik
force-pushed
the
master
branch
2 times, most recently
from
5989095
to
4f2d578
Compare
|
I appreciate any suggestions how the test cases can be fixed. For the default installation, it is expected that the values are different, because I'd like to enable But I don't have any clue about the autoyast failure. I think that auto-resize should be disabled if the autoyast profile specifies an explicit value, and there should be a way to disable auto-resize even if no explicit value is specified, but that's starting to be too much for my poor yast development skills. |
Never mind, I think I have figured it out. See my commit 8b78a68 (last on this branch). |
ptesarik
force-pushed
the
master
branch
4 times, most recently
from
06416f8
to
0fe2577
Compare
If high memory is supported, do not add the extra widgets to a top-level array, but rather make its own array that can then be then inserted in the VBox as necessary. No functional change (intended).
Put all fields that control allocated memory into a VBox, which can then be enabled/disabled as a single entity.
Kdump-1.0.1 introduced a new option. Make it available in the YaST module.
The number is confusing, because it only shows how much memory is available at early boot (before kdump-early.service is started). It is impossible to know exactly how much memory will be allocated at that time. Rather than making a wild guess, I prefer to avoid showing any value, so people at least realize that something works very differently when this option is selected.
The whole point of KDUMP_AUTO_RESIZE is that users do not have to set any values. Let YaST calculate a sensible allocation automatically. The current logic is to take the size of the largest available memory block, or half of total RAM, whichever is smaller. For Xen Dom0, we take the default values, because the Xen hypervisor cannot shrink the crashkernel area after boot.
If FADUMP is enabled, then the crashkernel area is reserved by the firmware and it cannot be resized. Make sure that the Auto Resize checkbox is always disabled if FADUMP is active.
|
FYI I've rebased all commits once again, adding my correct GPG signature. |
There was a problem hiding this comment.
Thanks Petr for this PR! To be honest, I don't know this YaST module too much. Anyway, having a look to the code and tests, it looks quite ok to me.
Acoording to the tests, I see this new setting should also work with AutoYaST. In that case, you have to add the setting to the scheme definition here https://github.com/yast/yast-kdump/blob/master/src/autoyast-rnc/kdump.rnc#L32.
Moreover, it would be great to open a new PR for the AutoYaST documentation in order to document this new setting: https://documentation.suse.com/sles/15-SP3/html/SLES-all/cha-configuration-installation-options.html#CreateProfile-kdump-saving-summary
Maybe @imobachgs as AutoYaST expert could suggest some more things to adapt.
Yes, please, add the |
|
@ptesarik I see it does not move for two weeks, do you need some help with implementing autoyast support? If so, feel free to contact me on IRC or slack. |
|
Hi Josef, thanks for asking. This PR has been preempted by other projects with higher priority. I can get back to it early next week (Feb 28). |
The default proposal for kernel reservation is now either max_high, or half of total memory, whichever is smaller. Adjust the test suite accordingly. Also, make sure that auto resize is turned off if crash_kernel should not be changed.
If auto resize is off, the kernel command line should contain the default values, not the maximum values.
No functional change, but looks nicer.
|
Er, I have found some time to look at this PR again. So, I added one line to the rnc file in commit 99eb3ba, but now I feel stupid. Were you indeed waiting just for this one line? |
|
@ptesarik sorry, it gets a bit out of my radar. it is really a bit too late for SP4, but master is open for SP5 and TW. So far it looks good for me. Just please add entry to changelog and bump version and we can merge it. Thanks and sorry |
|
Hi @ptesarik, @jreidinger How can I help for moving the PR on and get it merged? |
|
@dgdavid well, as said, if you solve conflicts that appear meanwhile and add changelog entry and bump version, then nothing is in the way to merge it |
|
Ok, I've tried it. But it needs someone else review :) All existing test are passing, but we should ensure that intended changes are ok after merging conflicts found because of #126 @ptesarik, @imobachgs, friendly ping 😉 |
dgdavid
force-pushed
the
master
branch
2 times, most recently
from
a5d142b
to
a9504d1
Compare
See yast/ci-ruby-container#19 and yast/yast-theme#161 Co-Authored-By: Ladislav Slezák <lslezak@suse.cz>
|
Thanks for the re-review @jreidinger. I'll merge it now 🤞 |
|
✔️ Public Jenkins job #39 successfully finished |
|
✔️ Internal Jenkins job #28 successfully finished |
Kdump-1.0.1 adds a new option to resize the crash kernel reservation automatically at boot. It's non-trivial to use it properly, but the YaST module can be of great help!
This would have been the ultimate goal of jsc#SLE-18441 "Auto scale crashkernel size with hardware changes", but yeah, I know it's probably too late now. If nothing else, Tumbleweed may benefit from this work.