Add Filesystem::tmpfile() for FUSE_TMPFILE - #740
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 354de0f9f3
ℹ️ 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".
| assert_eq!(x.mode(), 0o100_640); | ||
| assert_eq!(x.umask(), 0o22); | ||
| // O_TMPFILE is O_DIRECTORY | 0o20_000_000 on every architecture fuser builds for | ||
| assert_eq!(x.flags(), libc::O_RDWR | libc::O_TMPFILE); |
There was a problem hiding this comment.
The macOS and FreeBSD CI jobs compile all test targets (cargo build --all --all-targets), but this new always-compiled unit test references the Linux-only libc::O_TMPFILE constant. On those non-Linux targets the constant is not available, so cross-platform CI will fail before tests can run; please gate this test/assertion to Linux or compare against a portable literal wire value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and already fixed in b906df2 -- both jobs had failed with exactly this before the review landed, so we reached the same conclusion independently.
Gated the constants and the test on target_os = "linux" rather than comparing against a literal. O_TMPFILE exists on Linux alone and macFUSE never sends this opcode, so a Linux-only test is the honest description; a portable literal would have kept it compiling everywhere while testing a value that means nothing on those platforms.
The end-to-end test needed nothing: it is in a module that is already #[cfg(all(test, target_os = "linux"))].
The real mistake was in how I checked before pushing. I ran cargo check --target x86_64-apple-darwin --features=macos-no-mount, which builds only the lib, while mac-ci builds --all-targets. Now verified with --all-targets on both x86_64-apple-darwin and x86_64-unknown-freebsd, which are clean.
Generated by Claude Code
354de0f to
b906df2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b906df2f4e
ℹ️ 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".
| let opened = nix::fcntl::open( | ||
| &mountpoint, | ||
| nix::fcntl::OFlag::O_TMPFILE | nix::fcntl::OFlag::O_RDWR, | ||
| nix::sys::stat::Mode::from_bits_truncate(0o600), | ||
| ); |
There was a problem hiding this comment.
On Linux kernels whose negotiated FUSE protocol is older than 7.37, this O_TMPFILE open is rejected by the kernel with EOPNOTSUPP before fuser sees any request, so the later opened.expect(...) panics even though the library still supports older kernels (the session only rejects ABI versions below 7.6). Please skip based on negotiated ABI/kernel support rather than requiring FUSE_TMPFILE for every Linux test run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and worse than the version numbers suggest. Fixed in 1558748.
I checked which kernel this actually landed in rather than going by the ABI number: fuse_tmpfile is absent in v6.0 and present in v6.1. And the guard is not in fuse at all -- vfs_tmpfile() returns before touching the filesystem:
if (!dir->i_op->tmpfile)
return -EOPNOTSUPP;So on anything older than 6.1 the open fails without a request being sent, and the test would have failed for anyone on 5.15, 5.14 or similar, which fuser supports perfectly well. Not a niche case.
The test now reads Session::proto_version after the handshake and skips below 7.37, in the style of the existing session_ends_cleanly_after_abort skip. Since fuser advertises 7.44, the negotiated version is the kernel's own, so it is a faithful proxy for whether the inode operation exists. Verified both paths: the skip prints and returns cleanly when forced, and the real path still exercises the round trip on this kernel.
Generated by Claude Code
The kernel sends this for open() with O_TMPFILE, asking for a file that has no name and belongs to no directory until linkat() gives it one. Without it fuser answered the opcode with ENOSYS from the unknown-operation path, and since the kernel takes that as permanent, O_TMPFILE was reported to callers as EOPNOTSUPP for the life of the connection. That is still what happens for a filesystem that leaves the method unimplemented. On the wire this is a create request, so it carries a name, but the name is the placeholder "/" the VFS gives every anonymous dentry. It says nothing and is not passed on, which is also what libfuse's tmpfile handler does. Fixes #366 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5akmePV1VnD9YRQLZH4gv
b906df2 to
1558748
Compare
2 participants
Fixes #366.
FUSE_TMPFILEis opcode 51, ABI 7.37. The kernel sends it foropen()withO_TMPFILE, asking for a file with no name that belongs to no directory untillinkat()gives it one.On the wire it is a create request:
fuse_create_open()in the kernel builds it withopcode = FUSE_TMPFILEand otherwise identical arguments, so it carriesfuse_create_inplus a name and is answered withfuse_entry_out+fuse_open_out. The name is the placeholder/that the VFS gives every anonymous dentry, so it says nothing and is not passed to the filesystem -- libfuse'stmpfilehandler drops it for the same reason, taking only(req, parent, mode, fi).Unlike #359 this is reachable on an ordinary mount, so it comes with a real end-to-end test rather than only an ABI one.
Behaviour is unchanged for a filesystem that does not implement it:
ENOSYSis what the unknown-opcode path already returned, the kernel takes that as permanent, setsfc->no_tmpfileand reportsEOPNOTSUPPwithout asking again. That includes thesimpleexample, so nothing in pjdfs or xfstests changes.Testing
session::test::tmpfile_openmounts a filesystem, opens the mountpoint withO_TMPFILE | O_RDWR, and requires that the call reachesFilesystem::tmpfile()withS_IFREGset andO_TMPFILEin the flags, and that the descriptor the filesystem replies with gets back to the caller. Confirmed it fails without the dispatch arm, withEOPNOTSUPP-- which is exactly the old behaviour, so the test is pinning the change rather than the plumbing.ll::request::tests::tmpfilecovers the parse in the byte-level style of the neighbouring request tests, for both endiannesses.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.Not included
simpledoes not implementtmpfile. Doing so needs an inode with no name and no parent, pluslink()to later give it one, which is a feature of the example rather than of this change -- and without it the example's behaviour is byte-for-byte what it is today.🤖 Generated with Claude Code
https://claude.ai/code/session_01H5akmePV1VnD9YRQLZH4gv
Generated by Claude Code