sysroot: Support for directories instead of symbolic links in boot part#3359
sysroot: Support for directories instead of symbolic links in boot part#3359igoropaniuk wants to merge 4 commits intoostreedev:mainfrom
Conversation
|
Hi @igoropaniuk. Thanks for your PR. I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
bb7a7cb to
5be249e
Compare
|
This is based on the initial work done in the #1967 PR (looks like it stalled) |
|
Thanks so much for picking this back up!! I am very interested in systemd-boot support. But this is just one part of the picture as far as BLS type 1 spec. Note that that spec added the srel file which I think we should use to dispatch on. IOW if we find that file, it is an error if This is a tangentially related topic but I'd like to ask you: Have you investigated https://github.com/containers/bootc/ ? That's where most of my development work today is going. I will continue to help maintain ostree (and bootc hard depends on it today) but medium term I do plan to slowly sever that dependency (as we head towards composefs in particular, xref https://github.com/containers/composefs-rs/ for some cool PoC work). And especially that would mean here that we may end up doing BLS and UKI support in Rust in bootc, not here. |
|
@igoropaniuk Based on your contributions I'd like to offer you reviewer-level access to ostree. This would mean both the right and some responsibility to review/merge PRs made by others. |
Sounds great, I will be happy to help with reviews!
Actually I had, but our existing OTA solution is based on Uptane (Aktualizr fork as a client) + OSTree, so my primary focus is on this software stack for now. |
5be249e to
11f4940
Compare
|
@cgwalters I've added additional commit for |
11f4940 to
76ffcf8
Compare
|
@cgwalters I've added additional tests to cover proposed changes. Let me know if there is anything else for me to do in the scope of this change |
76ffcf8 to
04fd02b
Compare
|
@cgwalters gentle ping, let me know if there is anything else for me to do |
cgwalters
left a comment
There was a problem hiding this comment.
So sorry for the delay here and thanks for working on this again!
341f55a to
c0788dc
Compare
|
I pushed a rebased version of this to https://github.com/cgwalters/ostree/pull/new/symlinks-directories-support - I think I resolved the conflicts OK, but there's definitely some stuff here I want to look at more closely. |
|
I'd like to bring my issues with #1967 here: I've thought about this several times as I would really like to have this in Endless to support our systemd-boot systems. What always makes me anxious is trying to figure out how to handle compatibility with the vast majority of our systems that have symlink based deployments. There are 2 main issues I'm concerned with:
So, to me this requires a couple additional pieces of implementation and policy.
Does this PR consider any of these points? |
|
@igoropaniuk can you rebase on main and address the current comments ? |
|
Looking at the PR I would say
yes
yes (entries.srel containing "type1\n")
No, and this prevent downgrade, so at first I would personally prefer to use directories only when required (/boot is vfat) |
There were several attempts to include support for systemd-boot in ostree upstream, but a common solution is still pending to land, based on ostreedev/ostree#1967 and ostreedev/ostree#3359. Include the patches currently used in meta-lmp (validated and tested for a while already) based on the OSTREE_BOOTLOADER option (systemd-boot) to isolate the changes and reduce changes of regressions for systems relying on symlinks. Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
There were several attempts to include support for systemd-boot in ostree upstream, but a common solution is still pending to land, based on ostreedev/ostree#1967 and ostreedev/ostree#3359. Include the patches currently used in meta-lmp (validated and tested for a while already) based on the OSTREE_BOOTLOADER option (systemd-boot) to isolate the changes and reduce changes of regressions for systems relying on symlinks. Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
d950b3e to
e9f0b82
Compare
fd700ee to
6333c05
Compare
Allow manipulating and updating /boot/loader entries under a normal directory, as well as using symbolic links. In directory mode, a fixed staging directory loader.tmp is used instead of alternating between loader.0 and loader.1. The atomic swap is done via renameat2(RENAME_EXCHANGE) between loader.tmp and the live loader directory. After the exchange the stale loader.tmp is removed. This avoids the need to track which numbered directory is active via an ostree_bootversion file. The current bootversion is derived from the ostree= kernel argument found in the live loader/entries/*.conf files rather than from a dedicated tracking file. During deployment, _ostree_sysroot_read_boot_loader_configs() prefers loader.tmp/entries when it exists (i.e. while staging is in progress) so that bootloader write_config callbacks read the new BLS entries rather than the stale live ones. Non-ostree BLS entries (e.g. from a dual-boot setup) are copied from the live loader/entries/ into the staging loader.tmp/entries/ so they are preserved across each atomic swap. loader.conf is also copied from the live loader/ into loader.tmp/ so bootloaders like systemd-boot that rely on it continue to work after each swap. /boot/loader as a normal directory is needed by systemd-boot support, and can be stored under the EFI ESP vfat partition. Based on the original implementation done by Valentin David [1]. [1] ostreedev#1967 Signed-off-by: Ricardo Salveti <ricardo@foundries.io> Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
Add support for standard-conformance marker file loader/entries.srel. There might be implementations of boot loading infrastructure that are also using the /loader/entries/ directory, but install files that do not follow the [1] specification. In order to minimize confusion, a boot loader implementation may place the file /loader/entries.srel next to the /loader/entries/ directory containing the ASCII string type1 (followed by a UNIX newline). Tools that need to determine whether an existing directory implements the semantics described here may check for this file and contents: if it exists and contains the mentioned string, it shall assume a standards-compliant implementation is in place. If it exists but contains a different string it shall assume other semantics are implemented. If the file does not exist, no assumptions should be made. [1] https://uapi-group.org/specifications/specs/boot_loader_specification/#type-1-boot-loader-entry-keys Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
In directory mode the staging area for the new boot configuration is boot/loader.tmp, not boot/loader.N. The sysroot deploy code already creates loader.tmp and writes BLS entries there before invoking _ostree_bootloader_write_config(), but the syslinux, grub2 (non-EFI), and uboot backends each hard-coded boot/loader.N/<config-file> as the output path, causing an open(O_TMPFILE) failure because the boot/loader.N/ directory does not exist in directory mode. Fix each affected backend to detect whether boot/loader is a real directory (directory mode) or a symlink (classic mode) via glnx_fstatat_allow_noent(), and write to boot/loader.tmp/<config-file> in the former case. The ostree-grub-generator script derives its BLS entries directory from the parent directory of the grub.cfg output path, so pointing it at boot/loader.tmp/grub.cfg automatically causes it to read the new entries from boot/loader.tmp/entries/ - the correct staging location.
Add tests for boot/loader directory. Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
6333c05 to
3018fd2
Compare
|
@cgwalters @champtar addressed all comments, rebased on the latest main All tests are passing too. |
Allow manipulating and updating /boot/loader entries under a normal directory, as well as using symbolic links.
For directories this uses
renameat2to do atomic swap of the loader directory in the boot partition. It fallsback to non-atomic rename. This stays atomic on filesystems supporting links but also provide a non-atomic behavior when filesystem does not provide any atomic alternative./boot/loader as a normal directory is needed by systemd-boot support, and can be stored under the EFI ESP vfat partition.
Tests were duplicated for simplicity reasons.
Based on the original implementation done by Valentin David [1].
[1] #1967