install: Only remove bootloader dirs from the ESP - #2475
ericcurtin wants to merge 1 commit into
Conversation
github-actions
Bot
added
area/install
|
This seems like a safer default than what was suggested in the TODO @cgwalters "TODO: we should also support not wiping the ESP." Disclosure: very much not AI writing this comment :) |
| /// the device firmware payload (`vendorfw/`) and the U-Boot EFI variable store | ||
| /// (`ubootefi.var`) on the ESP; wiping them leaves the machine unbootable. | ||
| #[context("Removing bootloader content from EFI system partition")] | ||
| fn clean_esp_bootloader_dirs(efidir: &Dir) -> Result<()> { |
There was a problem hiding this comment.
People might also have custom bootloader config files in ESP/loader. I think we should read the config files and only remove the ones we own and the boot binaries referenced by them.
There was a problem hiding this comment.
Maybe but I wouldn't mind a second opinion on that... For example... Would the pre-existing boot entries be completely broken after this? I'm open to it, but a second opinion could be interesting
There was a problem hiding this comment.
It's more or less related to #2243. I don't think it's a blocker per se, but if we're going the distance to do this, then maybe we should just handle all the cases.
Would the pre-existing boot entries be completely broken after this?
we're deleting the pre-existing boot entries... so, yes.
There was a problem hiding this comment.
Sorry, maybe I didn't explain myself well enough... Even if we left the pre-bootc boot entries around... Would they even boot at that point? Would they even be useful after the conversion to a bootc system? (genuine question, haven't checked how destructive "bootc install to-existing-root" is to userspace, initramfs, kernel, etc. This is useful in Asahi because of the files required above needed to boot, not because of rollback to pre-bootc case)
There was a problem hiding this comment.
Would they even boot at that point?
if we leave both the .conf file and the PE binary it points to, inside of the ESP, then there's no reason why that entry won't work. These entries probably won't be booting a kernel but maybe could be something firmware related
There was a problem hiding this comment.
Agreed it'd be better, but keeping this PR minimal; added a TODO referencing #2243 to be more selective within EFI/ and loader/ as a follow-up.
Yes it's clearly safer. However, it's worth explaining the rationale behind the current default, which is that it was designed to support in-place reprovisioning from a default cloud image. In those cases, we can be generally sure we want to remove everything else. One way we could reasonably weaken this default is via equivalent of But I don't like that much. The thing is, if we only remove boot loader entires we are going to be leaking e.g. shim/grub or other things that we do expect to get reinitialized from the container state. So one approach is to make this the OS problem - in fact kind of what's going on here with system-reinstall-bootc is we have custom package-manager plugins to support cleaning up the old OS state (which in some cases we do want to carry along!). Hmm, it might be that that would actually just work today on def-managed systems, if we |
I guess in the x86_64/aarch64 UEFI case, if we consider nothing else.
Could we just overwrite these things with new ones? Since either way whether if we destroy everything or overwrite binaries, it's destructive to rollback to the pre-bootc state either way.
There's a narrative in Fedora Asahi community (well one single person) that bootc is only interested in by the book UEFI platforms. I don't believe that to be true because there has been success in Android Bootloader-style platforms and suggestions of BIOS compatibility. The opinionated people (well again a single person) in Asahi is looking hard for reasons to not move forward with an Atomic variant. But if we don't have a clear path forward here for the |
I'm sure I know who that person is and they're obviously wrong; the bootloader space is very complicated but we definitely support classic x86 BIOS, and there's support for s390x zipl and as you know aboot in ostree, which bootc can enable too. (That said the Android Boot situation is quite complex)
Not necessarily, we would like to support that.
I think there's a pretty clear path per above: aim to keep the "reprovision cloud image" working, which could possibly be a change in default to |
553cf2d to
861ace1
Compare
to-existing-root emptied the whole ESP, which can hold content bootc cannot recreate. On Asahi this includes m1n1/boot.bin, vendorfw/ and ubootefi.var; wiping them leaves the machine unbootable. Only remove EFI/ and loader/, which we reinstall. Factor out remove_dir_no_xdev() to share the recursive removal. Generated-by: AI Signed-off-by: Eric Curtin <eric.curtin@docker.com>
861ace1 to
969d628
Compare
|
Cloud images only have |
This branch has not been deployed
4 participants
to-existing-rootemptied the whole ESP. On Asahi (Apple Silicon) the ESP also holdsm1n1/boot.bin(stage 2 m1n1 + U-Boot),vendorfw/andubootefi.var; wiping them leaves the machine unbootable. This was hit converting Fedora Asahi Remix 43 tofedora-asahi-remix-atomic-desktops/base-atomic:44.Only remove
EFI/andloader/(reinstalled by bootupd/systemd-boot), preserve everything else. Cloud images, which only haveEFI/on the ESP, see no change. Being more selective withinEFI/andloader/(other OSes, custom entries, old Type #1 kernels) is left as a TODO, see #2243.Testing: unit test, plus a check in
test-multi-device-esp.nuthat seededm1n1/boot.binsurvives and a staleEFI/staleis removed.Generated-by: AI
Problem diagnosed and reproduced on real hardware by the submitter; code and tests are LLM generated.