Add Filesystem::syncfs() for FUSE_SYNCFS - #738
Merged
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12cedba171
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The kernel sends this for syncfs(2), asking the filesystem to make everything it holds durable. Without it fuser answered the opcode with ENOSYS from the unknown-operation path, so a filesystem had no way to implement the call. Leaving syncfs() unimplemented still reports ENOSYS, and the kernel then stops sending it for the lifetime of the connection, so nothing changes for a filesystem that does not want it. SyncFs is deliberately not added to the list of operations exempt from the SessionACL check. That list is for requests the kernel issues on its own behalf with no caller; syncfs(2) has one, and its request carries that caller's credentials like any other. Since every access decision fuser makes rests on that, and it is easy to assume wrongly, a test now pins it down. Note that the kernel only propagates syncfs(2) on fuseblk and virtiofs connections - fc->sync_fs is set nowhere else - and fuser mounts neither. This is therefore reachable only for a session built with Session::from_fd on such a connection, which is what the trait method and the changelog say. Fixes #359 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5akmePV1VnD9YRQLZH4gv
cberner
force-pushed
the
claude/github-issues-triage-dv6dhi
branch
from
August 4, 2026 02:45
12cedba to
65d75fc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #359.
FUSE_SYNCFSis opcode 50, ABI 7.34, carryingstruct fuse_syncfs_in { uint64_t padding; }and answered with an empty reply. fuser reports 7.44, so nothing needs version gating on the receive side. Adds the opcode, the argument struct, anOperation::SyncFs, the dispatch, andFilesystem::syncfs().Leaving
syncfs()unimplemented reportsENOSYS, which is what the unknown-opcode path already produced, and the kernel then clearsfc->sync_fsand stops asking for the lifetime of the connection. So this changes nothing for a filesystem that does not want it.One thing worth knowing before merging
The kernel only propagates
syncfs(2)onfuseblkand virtiofs connections.fc->sync_fsis set in exactly two places --fuse_fill_super_common()underif (ctx->is_bdev), andvirtio_fs.c-- andfuse_sync_fs()returns 0 immediately when it is unset. fuser mounts filesystem typefuse, neverfuseblk, so on a filesystem fuser mounts the kernel never sends this andsyncfs(2)succeeds without a round trip.That leaves one reachable path: a session built with
Session::from_fdon afuseblkor virtiofs connection. The trait method and the changelog both say so, so nobody implements it expecting it to fire on an ordinary mount.It is still worth having -- it is protocol coverage, it is what the issue asked for, and
from_fdis public API -- but it is not something I can demonstrate end to end, and I would rather you knew that than found it later.Testing
ll::request::tests::syncfsandsyncfs_without_its_argument, in the byte-level style of the existing request tests, for both endiannesses. They cover the parse and that a request whose header declares no argument is rejected rather than parsed. Confirmed the second fails if the argument fetch is dropped.No e2e test, for the reason above: the kernel will not send this to anything fuser can mount, so a
fuser-testscase would assert nothing.cargo fmt, all threeclippyinvocations frommake pre,cargo doc,cargo test --allwith and withoutlibfuse, the musl build,cargo check --target x86_64-apple-darwin --features=macos-no-mount, andmake test_passthroughare green. Docker was unavailable here, somount_tests/pjdfs_tests/xfstestsdid not run locally; this adds an opcode none of them exercise.🤖 Generated with Claude Code
https://claude.ai/code/session_01H5akmePV1VnD9YRQLZH4gv
Generated by Claude Code