composefs: Backwards compatibility#2128
composefs: Backwards compatibility#2128Johan-Liebert1 wants to merge 4 commits intobootc-dev:mainfrom
Conversation
Instead of reading the in memory filesystem to get /usr/lib/os-release get it from the mounted EROFS. This is also prep for providing backwards compatibility due to our newly introduced prefix `bootc_composefs-` where we'll need to create new boot entries and we can get the `os_id` from the mounted root Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a backward compatibility layer to migrate existing boot entries, including EFI binaries and BLS configurations, to a new naming convention using a custom prefix. The migration is automatically triggered during storage initialization when a legacy system state is detected. Review feedback identifies critical logic errors in the staging of BLS and GRUB entries where existing prefixed entries are inadvertently excluded, which would result in data loss. Furthermore, the rollback mechanism for failed renames incorrectly utilizes EXCHANGE flags for files that have already been moved, and the integration of the migration logic into the core storage initialization path is flagged as a potential stability risk that could make the tool unusable upon failure.
crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs
Outdated
Show resolved
Hide resolved
crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs
Outdated
Show resolved
Hide resolved
| if meta_json.is_none() { | ||
| let cmdline = composefs_booted()? | ||
| .ok_or_else(|| anyhow::anyhow!("Could not get booted composefs cmdline"))?; | ||
| prepend_custom_prefix(&storage, &cmdline).await?; | ||
| } |
There was a problem hiding this comment.
Triggering a potentially heavy and risky filesystem operation like prepend_custom_prefix inside BootedStorage::new is quite aggressive. Since BootedStorage::new is called at the start of almost every bootc command (including status), any failure during the prefix migration (e.g., due to an unexpected file in the ESP or a transient I/O error) will make the bootc tool completely unusable for the user. Consider logging a warning on failure instead of returning an error, or restricting this migration to specific operations like upgrade.
There was a problem hiding this comment.
Yeah, this is a point of contention on where this operation should be performed
While finishing up GC, we had come up with the idea of prepending our boot binaries (UKI PEs, BLS directories) with a certain prefix and we ended up hard requiring these prefixes. If someone has an older version of bootc which they used to install their system with, then upgrade to a new version, many if not all of the important operations would cease to work. This basically handles the backwards compatibility of new binaries on older systems by prepending our custom prefix to all existing boot binaries Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Check if the repo has meta.json file and if not apply our fix of prepending custom prefix to our bootloader entries and boot binaries Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Johan-Liebert1
force-pushed
the
backwards-compat
branch
from
ca6ebb6 to
f6cd249
Compare
|
Some potential conflicts here with #2044 One thing I think would clearly really help is standing up some mocked-unit testing for the boot layout. I have some PoC code incoming |
|
@Johan-Liebert1 can you look at squashing in https://github.com/cgwalters/bootc/tree/composefs-gc-tests ? |
Johan-Liebert1
force-pushed
the
backwards-compat
branch
from
e4ba88e to
139f378
Compare
Add some basic infra to mock up enough of an installed root to use in unit tests - specifically targeted for the bootloader logic. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
Johan-Liebert1
force-pushed
the
backwards-compat
branch
from
139f378 to
256a16d
Compare
|
OK I rolled the commits here into #2044 |
2 participants
composefs/boot: Get os_id from mounted EROFS
Instead of reading the in memory filesystem to get /usr/lib/os-release
get it from the mounted EROFS. This is also prep for providing backwards
compatibility due to our newly introduced prefix
bootc_composefs-where we'll need to create new boot entries and we can get the
os_idfrom the mounted root
composefs: Handle backwads compatibility with older versions
While finishing up GC, we had come up with the idea of prepending our
boot binaries (UKI PEs, BLS directories) with a certain prefix and we
ended up hard requiring these prefixes.
If someone has an older version of bootc which they used to install
their system with, then upgrade to a new version, many if not all of the
important operations would cease to work.
This basically handles the backwards compatibility of new binaries on
older systems by prepending our custom prefix to all existing boot binaries
composefs: Check for meta.json
Check if the repo has meta.json file and if not apply our fix of
prepending custom prefix to our bootloader entries and boot binaries
Since we depend on the existence of
meta.jsonin the composefs repo, we need #2044 first. After that's merged, we'll need to update the underlying repo and add some tests