lib: Fix bootc status on non-bootc systems#1851
Conversation
Previously, `BootedStorage::new()` unconditionally tried to open `/sysroot` before checking the environment type. This caused `bootc status` to fail on non-ostree/composefs systems. (We did work in containers and we had tests for that; but the container case is special cased even earlier) Fixes: https://issues.redhat.com/browse/RHEL-135687 Assisted-by: Claude Code (Opus 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue where bootc status would fail on non-bootc systems. The change refactors the code to only attempt opening /sysroot when on a bootc-managed system, which is the right approach. The addition of a new integration test to verify the fix in a simulated non-bootc environment is excellent. Overall, this is a solid improvement. I have one minor suggestion to improve the documentation for the new helper function to make its side-effects clearer for future maintainability.
| Composefs(BootedComposefs), | ||
| } | ||
|
|
||
| /// Open the physical root (/sysroot) and /run directories for a booted system. |
There was a problem hiding this comment.
The new helper function get_physical_root_and_run has an important side effect of remounting /sysroot as read-write. The current documentation doesn't mention this. It would be beneficial for future maintenance to make this explicit in the doc comment, and also to note the precondition that this should only be called within a private mount namespace to ensure system stability.
/// Open the physical root (/sysroot) and /run directories for a booted system,
/// remounting /sysroot as read-write.
///
/// This function should only be called inside a private mount namespace.
There was a problem hiding this comment.
open_dir_remount_rw(&d, ".".into())?
was there in the original code I don't think this is right.
Previously,
BootedStorage::new()unconditionally tried to open/sysrootbefore checking the environment type. This causedbootc statusto fail on non-ostree/composefs systems.(We did work in containers and we had tests for that; but
the container case is special cased even earlier)
Fixes: https://issues.redhat.com/browse/RHEL-135687
Assisted-by: Claude Code (Opus 4.5)