Skip to content

initramfs: Mount /sysroot readonly for composefs by default#1769

Merged
cgwalters merged 1 commit intobootc-dev:mainfrom
cgwalters:sysroot-ro-cfs
Nov 19, 2025
Merged

initramfs: Mount /sysroot readonly for composefs by default#1769
cgwalters merged 1 commit intobootc-dev:mainfrom
cgwalters:sysroot-ro-cfs

Conversation

@cgwalters
Copy link
Collaborator

@cgwalters cgwalters commented Nov 14, 2025

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
gemini-code-assist[bot]
gemini-code-assist bot reviewed Nov 14, 2025
View reviewed changes
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

Johan-Liebert1 commented Nov 17, 2025

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

@cgwalters
Copy link
Collaborator Author

cgwalters commented Nov 17, 2025

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

cgwalters commented Nov 17, 2025

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

@cgwalters cgwalters requested a review from jeckersb November 17, 2025 21:01
@Johan-Liebert1
Copy link
Collaborator

Johan-Liebert1 commented Nov 18, 2025 •
edited
Loading

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 •
edited
Loading

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

cgwalters
cgwalters commented Nov 18, 2025
View reviewed changes
@cgwalters @Johan-Liebert1
initramfs: Mount /sysroot readonly for composefs by default
5248715 
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

Reviewers

@Johan-Liebert1 Johan-Liebert1 Johan-Liebert1 approved these changes

@gursewak1997 gursewak1997 Awaiting requested review from gursewak1997

@jeckersb jeckersb Awaiting requested review from jeckersb

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cgwalters @Johan-Liebert1