-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
grub installer: remove old kernels before copying new ones. #26165
Conversation
@abbradar, thanks for your PR! By analyzing the history of the files in this pull request, we identified @wkennington, @edolstra and @symphorien to be potential reviewers. |
|
I dug a little around my last statement -- seems it's half-false. It won't be garbage collected but its profile will be removed (collector won't touch it because of
A proper way to fix this would be to not remove kernels from |
Ugh, there is still a problem left however. Suppose you'd do this:
On the third switch even though kernel from The only way this one can be fixed IMO is by always adding a boot entry that says "Generation booted at $date" which points to current |
For this, first remember all files that need to be installed. Then remove all files that are not in the list and copy new ones.
After more thought this was a false alarm: after |
The current form of But regardless of that, I think you still need to do either do the "Generation booted at $date" thing or some other flag file in
Also there is this case to end up with only garbage-collected generations in
|
@dezgeg I feel that the first case is the same that I was initially worried about, but in the end I think it's handled correctly because on final If I understand correctly |
@dezgeg Sorry for poking: any comments on the last reply? |
No problem... I think the second case is ok but the first one has a problem of
|
@dezgeg But I assumed that |
Sure, nix won't garbage-collect it, but the GRUB generator doesn't care about kernels that aren't referenced by the system profile generations (and step 2 removed the generation). |
Indeed, I've confirmed this problem in a VM. It currently exists in our systemd-boot installer. One solution would be to postpone removal of files from |
@abbradar Did you already have a chance to look at this? Just got bitten by it, too ;-) |
Are there any updates on this pull request, please? |
I didn't look at this for a long time. Let's close this for now. |
For this, first remember all files that need to be installed.
Then remove all files that are not in the list and copy new ones.
Motivation for this change
Fixes #23926.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)Tested in two VMs -- with and without split
/boot
.