Skip to content

Add support for mounting upper layers with images#303

Merged
cgwalters merged 3 commits into
mainfrom
support-upper
Jun 4, 2026
Merged

Add support for mounting upper layers with images#303
cgwalters merged 3 commits into
mainfrom
support-upper

Conversation

@alexlarsson
Copy link
Copy Markdown
Contributor

This is useful from the cli if you want to add a writable layer to a composefs mount. This is better than doing it in a separate overlayfs layer as that would increase the overlayfs stacking depth, etc.

@alexlarsson
Copy link
Copy Markdown
Contributor Author

I was chatting a bit with some people at the embedded recipes conf about using composefs without bootc (for a bit custom stuff) and they would need this to be able to couple composefs images with one or more writable layers.

Copy link
Copy Markdown
Collaborator

@Johan-Liebert1 Johan-Liebert1 left a comment

Choose a reason for hiding this comment

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

Some minor suggestions :)

Comment thread crates/composefs-ctl/src/lib.rs
Comment thread crates/composefs/src/mount.rs Outdated
@alexlarsson alexlarsson force-pushed the support-upper branch 2 times, most recently from 98425d8 to 0eead6a Compare May 29, 2026 08:25
@alexlarsson
Copy link
Copy Markdown
Contributor Author

Also, I didn't look at it in detail, but I think composefs-setup-root could use this instead of manually doing the overlayfs mount.

@Johan-Liebert1
Copy link
Copy Markdown
Collaborator

I don't remember well, but I think the "non initramfs" part of composefs-setup-root doesn't work as intended. I could be wrong though

@alexlarsson
Copy link
Copy Markdown
Contributor Author

We have a reverse dependency issue in bootc, where it needs to pass composefs::mount::NO_OVERLAY to mount. What is our current API stability guarantees here? Do i add a new mount_with_overlay() for this, or do we just fix bootc later?

@alexlarsson
Copy link
Copy Markdown
Contributor Author

Seems easiest to go with mount_with_overlay(), so i added that.

@alexlarsson
Copy link
Copy Markdown
Contributor Author

I have no idea what this failure is about:

    ---- privileged_test_digest_equivalence_debian_bootc ----
    command exited with non-zero code `bcvk ephemeral run-ssh --memory 10G localhost/composefs-rs-test-c9s:latest -- cfsctl-integration-tests --exact privileged_test_digest_equivalence_debian_bootc`: 255


    failures:
        privileged_test_digest_equivalence_debian_bootc

    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 60 filtered out; finished in 265.11s

@cgwalters
Copy link
Copy Markdown
Collaborator

I was chatting a bit with some people at the embedded recipes conf about using composefs without bootc (for a bit custom stuff)

Would it help for us to support building bootc without the ostree dep? I would like to be able to do that.

@cgwalters
Copy link
Copy Markdown
Collaborator

We have a reverse dependency issue in bootc, where it needs to pass composefs::mount::NO_OVERLAY to mount. What is our current API stability guarantees here? Do i add a new mount_with_overlay() for this, or do we just fix bootc later?

I think We can break API at any point, but it's also useful to have a bit of backpressure to avoid doing so unnecessarily. On the counterpoint, #225 does break API and in that PR I pointed the bootc revdep CI at a patch to adapt bootc to it.

@alexlarsson
Copy link
Copy Markdown
Contributor Author

I was chatting a bit with some people at the embedded recipes conf about using composefs without bootc (for a bit custom stuff)

Would it help for us to support building bootc without the ostree dep? I would like to be able to do that.

Nah, this was for "Flipper one" and the requirement they had was quite custom, focusing on allowing users to tweak and break their systems. I don't think it will ever be a good fit for bootc.

@cgwalters
Copy link
Copy Markdown
Collaborator

cgwalters commented May 29, 2026

I have no idea what this failure is about:

It's likely bootc-dev/bcvk#153 - a rerun made it pass.

cgwalters
cgwalters previously approved these changes May 29, 2026
Copy link
Copy Markdown
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Looks sane to me, but I would really like to build up the pattern of having integration tests again because I think they're easy to add.

Comment thread crates/composefs/src/mount.rs Outdated
Comment thread crates/composefs/src/mount.rs Outdated
)?)
}

/// Options for adding a writable overlay layer to a composefs mount.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's more possible options here - volatile is a big one!

I'm not saying we need to do that now, but we could make this #[non_exhaustive] to allow us to more easily add more options later.

Or we could just support pub options: Option<&'str>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made this a more generic non_exhaustive MountOptions.

@cgwalters
Copy link
Copy Markdown
Collaborator

One thing to bear in mind on this one too is the intersection with #304 - in the general case there what I think would work well is if we supported fd passing - for the mount operation we'd just hand back a detached fd by default.

@alexlarsson
Copy link
Copy Markdown
Contributor Author

I updated this to use a more generic MountOption that we can later extend.

This is useful from the cli if you want to add a writable layer to
a composefs mount. This is better than doing it in a separate overlayfs
layer as that would increase the overlayfs stacking depth, etc.

Note: This adds the overlay option as a separate mount_with_options()
as currently bootc calls repo.mount() which breaks API. We don't seem
to have any callers of repo.mount_at() so the options can be added
there.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
@alexlarsson alexlarsson force-pushed the support-upper branch 2 times, most recently from 4fb9573 to a3031b5 Compare June 4, 2026 12:09
@alexlarsson
Copy link
Copy Markdown
Contributor Author

I rebased on main and added support for varlink to this

Signed-off-by: Alexander Larsson <alexl@redhat.com>
@cgwalters
Copy link
Copy Markdown
Collaborator

OK but you weren't replying to my "needs tests" thread. I think the token cost of doing these things via LLMs is so cheap relative to the value they provide, so I spent some tokens and pushed a commit with a test case.

The previous commit added --upperdir, --workdir, and --read-write flags
to `cfsctl mount`; add some test cases.

Assisted-by: OpenCode (Claude Sonnet 4.6)
Signed-off-by: Colin Walters <walters@verbum.org>
Copy link
Copy Markdown
Contributor Author

@alexlarsson alexlarsson left a comment

Choose a reason for hiding this comment

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

lgtm

@cgwalters cgwalters added this pull request to the merge queue Jun 4, 2026
Merged via the queue into main with commit ed0da92 Jun 4, 2026
18 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.

3 participants