Skip to content

Add read-only mount support (#14) - #15

Merged
aniketmaurya merged 2 commits into
CelestoAI:mainfrom
craigpfeifer:feat/readonly-mount
Aug 5, 2026
Merged

aniketmaurya merged 2 commits into
CelestoAI:mainfrom
craigpfeifer:feat/readonly-mount

Conversation

@craigpfeifer

Copy link
Copy Markdown
Contributor

Closes #14.

What

Adds the ability to mount a smolfs volume read-only:

smolfs mount --readonly NAME PATH   # alias: --read-only

This is useful for data the current agent should not modify — model weights, training/evaluation data, reference data, or documents produced by another process.

Behavior

  • Mount: passes JuiceFS' --read-only flag; persists read_only on the volume record and clears it on unmount. The mount liveness probe uses a read-only check (read_dir) instead of the write probe when mounting read-only.
  • Writes: rejected by the filesystem with Read-only file system (EROFS) — enforced by the kernel/FUSE, not smolfs.
  • smolfs flush: returns a new VolumeReadOnly error on a read-only mount (there is nothing to flush).
  • smolfs status: adds a mode column showing ro/rw (or - when unmounted). JSON keeps a boolean read_only field, matching the existing dev convention.
  • SDKs: read_only exposed through the Python (read_only=False kwarg) and Node (readOnly?) bindings.

Read-only is a property of a mount (like mountpoint), set at mount time and cleared on unmount — it is not a volume-level setting at init.

Backward compatibility

Volume.read_only uses #[serde(default)], so existing volume TOML records without the field still deserialize (covered by a regression test).

Tests

  • Unit tests: mount flag included/omitted, serde-default legacy record, VolumeInfo mapping, and flush erroring on a read-only volume.
  • Integration test (crates/smolfs-juicefs/tests/readonly_mount.rs, gated behind SMOLFS_RUN_INTEGRATION=1): full lifecycle on a --dev volume — read-only mount, write rejected (EROFS), flush errors, unmount, read-write remount where writes and flush succeed again.

Verified locally: cargo test (unit + integration), cargo clippy -D warnings, cargo fmt --check, cargo build -p smolfs-cli, and maturin build for the Python wheel.

🤖 Generated with Claude Code

Mount a volume read-only with `smolfs mount --readonly NAME PATH`
(alias `--read-only`), for data the current agent should not modify
(model weights, training/eval data, reference documents).

- Pass JuiceFS `--read-only` at mount time; persist `read_only` on the
  volume record (serde default for backward compatibility) and clear it
  on unmount.
- Use a read-only liveness probe instead of the write probe when mounting
  read-only.
- `smolfs flush` errors with a new `VolumeReadOnly` error on a read-only
  mount; writes are rejected by the filesystem (EROFS).
- `smolfs status` shows a `mode` column (ro/rw); JSON keeps a boolean
  `read_only` field.
- Expose `read_only` through the Python and Node SDK bindings.
- Add unit tests (mount flag, serde default, VolumeInfo mapping, flush
  guard) and a gated integration test covering the full lifecycle.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@craigpfeifer

craigpfeifer commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor Author

There is one catch here : multiple clients can mount a single fs as ro OR rw only if they are using network metadata. Multiple clients cannot mount a single fs as ro or rw if using --dev.

What actually makes concurrent RO/RW possible

JuiceFS coordinates concurrent mounts through its metadata engine, not the object store. The object store (S3/local files) just holds the data blocks. So the deciding factor is:

Volume type Metadata Object store Concurrent multi-client?
Cloud Redis (networked) S3 / GCS ✅ Yes — across processes and machines
--dev SQLite (local file) Local files ⚠️ Single machine only, and not through smolfs cleanly

Cloud volume: Redis is a networked server built for many simultaneous clients, and S3 is a shared object store. Each client (each machine) points at the same metadata_url + bucket with its own SMOLFS_HOME, so each keeps its own mount record. Client A mounts --readonly, client B mounts read-write → works, exactly as intended. ✅

--dev volume: metadata is a local SQLite file and objects are local files, both derived from and living inside one SMOLFS_HOME (service.rs build_volume). By construction it's single-machine, and any two "clients" on that machine share the same registry — so you hit the single-mount-per-volume-record limitation (last mount clobbers the record). There's no cross-machine story at all because the paths are local.

One correction

It's not "S3 vs dev" — it's "networked metadata (Redis) vs local SQLite." If you configured a cloud volume with S3 storage but a local SQLite metadata URL, it would have the same single-machine limitation as dev. The Redis metadata engine is what provides the cross-client coordination; S3 is just the shared data side that makes it usable off one machine.

@aniketmaurya

Copy link
Copy Markdown
Contributor

@codex please review the PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6fca407235

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/smolfs-juicefs/tests/readonly_mount.rs
Comment thread crates/smolfs-juicefs/tests/readonly_mount.rs Outdated
- Parameterize the read-only mount lifecycle test over both the Redis and
  SQLite metadata engines (backed by the S3 object store), mirroring the
  minio_integration pattern, per AGENTS.md's requirement to cover both
  metadata backends when touching JuiceFS behavior. (P1)
- Run the read-only integration test in CI: add it to the JuiceFS
  integration step (which already provides Redis, MinIO/S3, and
  SMOLFS_RUN_INTEGRATION), and install fuse3 so the mount lifecycle can
  actually be exercised. (P2)

Verified locally against Redis + SQLite metadata with MinIO S3 storage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@craigpfeifer

Copy link
Copy Markdown
Contributor Author

Thanks @codex — both points addressed in 19159ca:

P1 — cover the Redis metadata path. readonly_mount.rs no longer hard-codes a dev/SQLite volume. It now parameterizes the full read-only lifecycle over both the Redis and SQLite metadata engines (backed by the S3 object store), mirroring minio_integration.rs and satisfying the AGENTS.md requirement to cover both backends when touching JuiceFS behavior.

P2 — run the test in CI. The read-only test is now part of the JuiceFS integration step (cargo test -p smolfs-juicefs --test minio_integration --test readonly_mount), which already provides Redis, MinIO/S3, and SMOLFS_RUN_INTEGRATION=1. Since this test actually mounts (unlike minio_integration, which only formats), I also added a step to install fuse3 so the mount lifecycle can run.

Verified locally against Redis + SQLite metadata with MinIO S3 storage (full lifecycle passes for both backends).

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@craigpfeifer

Copy link
Copy Markdown
Contributor Author

@codex please review the changes

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds end-to-end read-only mount support to SmolFS, surfacing the mode consistently across the CLI, core models/registry, JuiceFS backend invocation, and the Python/Node bindings.

Changes:

  • Adds a --readonly/--read-only mount option, passes JuiceFS --read-only, and uses a read-only liveness probe.
  • Persists Volume.read_only with #[serde(default)], clears it on unmount, and exposes it via status (table + JSON) and SDK VolumeInfo.
  • Adds unit + integration test coverage for read-only lifecycle behavior, and updates CI/docs accordingly.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
README.md Documents --readonly behavior and updates command reference text.
crates/smolfs-juicefs/tests/readonly_mount.rs New integration test covering read-only mount lifecycle across Redis + SQLite metadata backends.
crates/smolfs-juicefs/src/service.rs Implements read-only probing, persists/clears read_only, and rejects flush on RO.
crates/smolfs-juicefs/src/juicefs.rs Adds --read-only flag wiring to the JuiceFS mount spec + tests.
crates/smolfs-core/src/registry.rs Adds regression test ensuring legacy volume records deserialize without read_only.
crates/smolfs-core/src/models.rs Adds Volume.read_only, MountVolume.readonly, and propagates into VolumeInfo + tests.
crates/smolfs-core/src/error.rs Adds SmolFsError::VolumeReadOnly.
crates/smolfs-cli/src/main.rs Adds CLI flag, prints mount mode in status, and annotates mount output for RO.
bindings/python/src/lib.rs Adds read_only mount kwarg and exposes VolumeInfo.read_only.
bindings/node/src/lib.rs Adds read_only option and exposes VolumeInfo.read_only in N-API structs.
bindings/node/index.d.ts Updates TS types with readOnly option/field.
.github/workflows/ci.yml Installs FUSE and runs the new read-only integration test in CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 103 to +108
let volume = self.registry.read(name)?;
if volume.read_only {
return Err(SmolFsError::VolumeReadOnly {
name: name.to_string(),
});
}
@aniketmaurya
aniketmaurya merged commit 81e505f into CelestoAI:main Aug 5, 2026
7 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.

Feature Request : Mount smolfs as read-only

3 participants