Skip to content

Support mounting file systems & migrate devices to use mounts#995

Open
jaybosamiya-ms wants to merge 8 commits into
mainfrom
jayb/fs-via-backend-part2
Open

Support mounting file systems & migrate devices to use mounts#995
jaybosamiya-ms wants to merge 8 commits into
mainfrom
jayb/fs-via-backend-part2

Conversation

@jaybosamiya-ms

Copy link
Copy Markdown
Member

This PR introduces the Composer, which acts as a mount point for the new core file system design (see #887). It maintains a runtime list of dyn-safe Backend objects for each mount point, using as much walking as feasible under the mount points.

To exercise the Composer and demonstrate how it works, I've also switched Devices to stop being hardcoded to /dev inside the backend but instead be a flat list of devices that is mounted at /dev.

While this PR does not expose/use it this way yet, with this PR, finally LiteBox core now has some of the fundamental machinery needed to eventually support mount(2) family of syscalls in the Linux shim.

Future PRs will continue migrating more backends over, eventually allowing for the old layered file system to disappear, and replaced with this mount composition + (upcoming) overlay file system to support union mounting.

@jaybosamiya-ms jaybosamiya-ms marked this pull request as ready for review July 1, 2026 21:40
@wdcui

wdcui commented Jul 2, 2026

Copy link
Copy Markdown
Member

GPT-5.5 reported two potential issues. I will try to review this PR tomorrow.

  1.  litebox/src/fs/composer.rs:257  — Empty composers can panic on root stat.  Composer::builder().build()  succeeds with no mounts, but  root_handle() 
    returns a virtual root and  virtual_dir_status([])  then  expect s a precomputed virtual dir that was never inserted.
  2.  litebox/src/fs/composer.rs:435  — Ancestor paths of nested mounts shadow real directories from an enclosing mounted backend. With mounts like  / 
    and  /mnt/foo , walking  /mnt  switches to  Virtual  before asking the root backend, so real  /mnt  contents are hidden and mutations under  /mnt 
    fail as read-only instead of operating on the backing backend while merging mount children.

@wdcui wdcui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall the code looks good to me. Some minor comments below. Please also take a look at the potential issues reported by GPT-5.5. Thanks!

Comment thread litebox_runner_snp/src/main.rs Outdated
let dev_stdio = litebox::fs::resolver::Resolver::new(
litebox,
litebox::fs::devices::Devices::migration_helper_standalone_new(litebox),
litebox::fs::composer::Composer::builder()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we have similar code both here and default_fs() in litebox_shim_linux/src/lib.rs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's similar but a little different because the default_fs in the Linux shim does not deal with 9p. We can probably abstract some of it out, but that's outside the scope of this particular PR.

Comment thread litebox/src/fs/devices.rs
if dir.location != Location::Dev {
return Err(OpenError::PathError(PathError::NoSuchFileOrDirectory));
}
let _dir = dir.into_typed::<Self>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like into_typed may cause panic by expect. It's a little hidden,and it's obvious that into_typed would have this kind of behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it can panic, but only if you actively try to use a handle from one file system onto another. The expectation is that any "normal" use of the file system will not actually panic, since users are not supposed to be invoking backends directly. More concretely, if we have initialized the file systems correctly (in shim or runner), and no backends are actively trying to mess with each other, then no set of guest syscalls can cause a panic at these calls.

Essentially, I explicitly chose not to make these Option or Result, but instead intentionally short there.

Nonetheless, I added doc comments to each of these as to what their intended use case is.

@jaybosamiya-ms

jaybosamiya-ms commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Thanks Weidong! The two reported issues are reasonable, not critical in practice I think, but fixed since they were real issues. I also added a little more documentation about the internal usage of into_typed and such, based on you noticing their potential for panicking if you misuse them.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 SemverChecks 🤖 ⚠️ Potential breaking API changes detected ⚠️

Click for details
--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/inherent_method_missing.ron

Failed in:
  Devices::migration_helper_standalone_new, previously in file /home/runner/work/litebox/litebox/target/semver-checks/git-main/5f338675f2338fa71312e6b52926cfbe4810af9c/litebox/src/fs/devices.rs:149

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