Skip to content

Conversation

@cgwalters
Copy link
Collaborator

This implements readonly mounting of /sysroot for composefs systems, matching the behavior that ostree systems already have. Previously, composefs left /sysroot mounted read-write, which was inconsistent and meant the readonly tests had to be skipped for composefs.

The implementation uses a direct libc::syscall wrapper for mount_setattr since rustix doesn't yet provide this API. The MOUNT_ATTR_RDONLY flag is applied recursively to three mount points during initramfs setup:

  • The composefs rootfs image mount (becomes / after switch-root)
  • The test root filesystem mount (used in testing scenarios)
  • The sysroot clone mount (becomes /sysroot in the booted system)

With this change, the readonly /sysroot tests in test-status.nu now run for both ostree and composefs systems without conditional checks.

Assisted-by: Claude Code (Sonnet 4.5)

@bootc-bot bootc-bot bot requested a review from gursewak1997 November 14, 2025 20:01
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces readonly mounting for /sysroot in composefs systems, aligning its behavior with ostree systems. This is a good improvement for consistency and enables previously skipped readonly tests. The implementation correctly uses a libc::syscall wrapper for mount_setattr as it's not yet available in rustix. My review includes suggestions for refactoring to reduce code duplication and for consistency in using constants from dependencies. I also found a minor redundancy in one of the test scripts.

@cgwalters cgwalters enabled auto-merge (rebase) November 14, 2025 22:27
@Johan-Liebert1
Copy link
Collaborator

We would need to update this for bootc update and bootc switch as they do write to /sysroot

@cgwalters
Copy link
Collaborator Author

We would need to update this for bootc update and bootc switch as they do write to /sysroot

We should now be consistently remounting /sysroot writable in a private mountns for both ostree and composefs paths. I'll double check this though.

@cgwalters
Copy link
Collaborator Author

We should now be consistently remounting /sysroot writable in a private mountns for both ostree and composefs paths. I'll double check this though.

Yep! This is covered by c0b9cde

@Johan-Liebert1
Copy link
Collaborator

Johan-Liebert1 commented Nov 18, 2025

Hmm... I still get a Read-only file system (os error 30) while switching

error: Switching: Composefs Switching: Pulling composefs repository: Pulling composefs repo: Unable to pull container image docker://192.168.122.1:5000/bootc-bls-upgrade: Failed to pull confi
g Descriptor { media_type: ImageConfig, digest: Digest { algorithm: Sha256, value: "sha256:47a7ba74f0fb105fc620cff9f2fc3ff3f823f5dca93d056c311751a0414e645a", split: 6 }, size: 13403, urls: No
ne, annotations: None, platform: None, artifact_type: None, data: None }: Read-only file system (os error 30)

@Johan-Liebert1
Copy link
Collaborator

Johan-Liebert1 commented Nov 18, 2025

We don't really remount /sysroot as rw in the composefs case. Only in the ostree case we call ostree_sysroot_lock which remounts /sysroot and /boot as rw. In composefs case we probably don't need a lock as composefs-rs does have an advisory lock via flock for all repository related operations, so I think just remounting as RW should be enough

This implements readonly mounting of /sysroot for composefs systems,
matching the behavior that ostree systems already have. Previously,
composefs left /sysroot mounted read-write, which was inconsistent
and meant the readonly tests had to be skipped for composefs.

The implementation uses a direct `libc::syscall` wrapper for
`mount_setattr` since rustix doesn't yet provide this API. The
`MOUNT_ATTR_RDONLY` flag is applied to three mount
points during initramfs setup:
- The composefs rootfs image mount (becomes `/` after switch-root)
- The test root filesystem mount (used in testing scenarios)
- The sysroot clone mount (becomes `/sysroot` in the booted system)

With this change, the readonly /sysroot tests in test-status.nu
now run for both ostree and composefs systems without conditional
checks.

Assisted-by: Claude Code (Sonnet 4.5)
Co-authored-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters merged commit e0475cd into bootc-dev:main Nov 19, 2025
45 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants