[RFC] sysroot: Support for directories instead of symbolic links in boot part #3359
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. Instructions 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. |
github-actions
bot
added
the
area/prepare-root
igoropaniuk
force-pushed
the
symlinks-directories-support
branch
2 times, most recently
from
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. |
| if (TEMP_FAILURE_RETRY (symlinkat (".", sysroot->sysroot_fd, "boot/boot")) < 0) | ||
| if (errno != EPERM && errno != EEXIST) | ||
| return glnx_throw_errno_prefix (error, "symlinkat"); |
There was a problem hiding this comment.
This is the bit I'd like to have dispatch on the srel file in particular.
To be conformant, we shouldn't create a symlink even if we can - the spec just says that XBOOTLDR needs to be something "the firmware" can read, and maybe someone makes a firmware that handles ext4 or whatever.
To be clear there's no good reason I am aware of to make XBOOTLDR something other than VFAT today - but still, the srel file seems like the right way to dispatch.
There was a problem hiding this comment.
Oh and as a big bonus, dispatching on that file means we can unit test this code without setting up a special hack to make symlink writing fail with EPERM etc.
There was a problem hiding this comment.
@cgwalters makes sense, just to be sure we're on the same page:
"type 1"in srel file andboot/loaderis a symlink -> report error"type 1"in srel file andboot/loaderis a directory -> useprepare_new_bootloader_dir()- no srel file exists/or not
"type 1"andboot/loaderis a directory -> useprepare_new_bootloader_dir() - no srel file exists/or not
"type 1"andboot/loaderis a symlink -> useprepare_new_bootloader_link()
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. |
Allow manipulating and updating /boot/loader entries under a normal directory, as well as using symbolic links. For directories this uses `renameat2` to 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. Based on the original implementation done by Valentin David [1]. [1] ostreedev#1967 Signed-off-by: Ricardo Salveti <[email protected]> Signed-off-by: Jose Quaresma <[email protected]> Signed-off-by: Igor Opaniuk <[email protected]>
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 <[email protected]>
igoropaniuk
force-pushed
the
symlinks-directories-support
branch
from
5be249e to
11f4940
Compare
|
@cgwalters I've added additional commit for |
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