Rebase Modal patches onto upstream v0.18.0 - #11
Open
dflemstr wants to merge 336 commits into
Open
Conversation
Fixed a bug where the documentation was missing from the `NO_XATTR` constant on a non-linux OS. Using a macro to prevent duplication.
Clippy says it's better to write long constants with underscore separators: [https://rust-lang.github.io/rust-clippy/rust-1.51.0/index.html#unreadable_literal]
Promoted some important items to `public(crate)` for easier testing and future reorganization.
Fixed a bug where the passthrough test would fail on macOS simply because the target file doesn't exist. It can still fail, but the test failure should be related to the feature being tested (i.e., passthrough).
It's safer to define the block size as u32 (its true native type) and cast up to u64 as needed for file attr.
These `check_*` helper functions don't borrow anything from self, so they are better expressed as associated functions.
This `write_directory_content` function copies but doesn't move the entry data, so it is better as a borrowed parameter.
Clippy says that functions with the potential to error or panic should be labeled by a markdown section `#` in the doc string. [missing_errors_doc](https://rust-lang.github.io/rust-clippy/rust-1.51.0/index.html#missing_errors_doc) I've done my best to write some kind of reasonable statements; there is room for improvement.
Repaired incorrect doc strings which were likely the result of copy and paste errors.
* Enable cross-compiling with non-Linux host * build.rs always needs pkg-config at build time * clippy build.rs
* Enable pure FUSE mount path on BSD * Add constant for BSD mount helper * Fix FreeBSD mount flag constant and imports * Handle mount_fusefs helper invocation on BSD * Preserve fuse device fd for mount_fusefs
More types, less unsafe. In a few places errors were ignored, but should be handled. That would be separate change, so I left it as is, preserving current behavior.
This is example, it does not matter much, but clearner since the project uses nix.
MSRV is 1.85, above 1.77.
When reading sources, make more clear what code is public API and what is internals.
- reserve capacity - use next_multiple_of - resize instead of memcpy for padding - documentation assertion
If cloexec flag is cleared in parent process, another thread may fork/exec and inherit the descriptor.
mount(2) on FreeBSD https://man.freebsd.org/cgi/man.cgi?mount(2) does not mention the flag. AFAIU the flag was removed a while ago. Remove the flag, error if the flag is specified. This change is done to migrate code to `nix`, where there's no constant `MNT_NODEV` on FreeBSD.
Adds methods to convert BackingIds from/to their raw `backing_id` values, which allows for marshalling of backing file references across process boundaries.
The kernel rejects all `FOPEN_PASSTHROUGH` replies for an inode that's already open in passthrough mode if the given `backing_id` does not match the current `backing_id`. Clarify this in the relevant method documentation.
Expose the the `run` method while fixing an options check blindspot.
Maintaining it is too much work. If someone else would like to, please do!
The fuse_mount_mount_fusefs function (FreeBSD mount path) was not receiving or applying the SessionACL parameter, so the allow_other mount option was never passed to mount_fusefs. This caused the FreeBSD kernel to return EPERM for all operations by non-owner users (e.g. uid 65534 in pjdfstest), since only the mount owner was allowed access. Pass the acl parameter through and convert it to the allow_other mount option, matching the behavior of the Linux fusermount path. https://claude.ai/code/session_017h551yq57ivFUqcJyAmAfv
Currently, `ReplyEntry::entry` forces the same TTL for both the directory entry and its attributes. In our VFS in certain situations we need to audit all access, including stat, so we set attribute cache duration to zero. Setting entry cache to zero at the same time completely destroys performance, so I would like to add an ability to configure these timeouts separately.
Removed duplicate cargo test command from CI workflow.
This allows comparing Errno values in tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7W6G38VFdKggqEMQDAMAF
Modal serves FUSE over a virtiofs virtqueue as well as /dev/fuse, which needs extension points this crate otherwise keeps internal: - A reply transport. `ReplySender` becomes an opaque public type with a `custom` constructor backed by the new `CustomReplySender` trait, so replies and notifications can go anywhere. `Notifier` and `PollNotifier` carry a `ReplySender` rather than a `ChannelSender` for the same reason. - A dispatch entry point. `RequestWithSender::new` and `dispatch` are public and take a `DispatchContext`, which holds just the filesystem, ACL and session owner instead of a whole `/dev/fuse`-backed event loop. - A handshake entry point. `handshake_request` processes one FUSE_INIT message from raw bytes and any sender, reporting through `HandshakeOutcome` whether the kernel will send another. `Session::handshake` now drives it in a loop. - `ForgetOne::new`, and a `ForgetOne` re-export, so that filesystems which adapt or forward requests can rewrite batch forget entries. - `Uid` is re-exported so callers building a `DispatchContext` need not depend on a matching version of nix. Also size event loop read buffers from the negotiated `max_write` rather than the protocol maximum, which otherwise costs 16 MiB per event loop thread.
The kernel reports POLLERR on /dev/fuse when the connection is dead, so is_mounted() returned true for an unmounted filesystem and never false. This defeated the guard in umount_impl() that prevents unmounting the mountpoint a second time after the filesystem was already unmounted externally, which could unmount an unrelated filesystem that had since been mounted at the same path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TfPeUUeQKYjo26Y9mQG9sA
The pure-Rust mount backend left the unmount to the fusermount helper whenever auto_unmount was requested, but that helper only acts once the mounting process exits. Unmounting from within that process therefore did nothing: the mountpoint was left behind as a dangling "transport endpoint is not connected", and because the connection stayed alive, session teardown gave up waiting and detached the still-running session thread. Unmount in umount_impl() as libfuse's fuse_session_unmount() does, and close the socket afterwards so the helper exits with no mount left to clean up. auto_unmount keeps serving its purpose as a safety net for a process that dies without unmounting. Fixes cberner#407 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5akmePV1VnD9YRQLZH4gv
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
13 participants
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.
Summary
Rebuilds Modal's fuser patches on top of upstream
v0.18.0, replacing the set carried onfs/pin-bcff57c(upstream0.14.0plus local patches).Upstream has since absorbed most of what we carried: the multi-threaded session loop is now
Config::n_threads/Config::clone_fd,destroyhandling lives inFilesystemHolder, and dispatch is already factored out of the read loop. What remains are the extension points Modal needs to serve FUSE over a virtiofs virtqueue rather than/dev/fuse, which upstream keeps internal:ReplySenderbecomes an opaque public type with acustomconstructor backed by a newCustomReplySendertrait.NotifierandPollNotifiercarry aReplySenderinstead of aChannelSenderso notifications work over any transport too.RequestWithSender::newanddispatchare public and take aDispatchContext— the filesystem, ACL and session owner — instead of a/dev/fuse-backedSessionEventLoop.handshake_requestprocesses a single FUSE_INIT message from raw bytes against any sender and reports throughHandshakeOutcomewhether the kernel will send another.Session::handshakenow drives it in a loop.ForgetOne::newplus aForgetOnere-export, so filesystems that adapt or forward requests can rewrite batch forget entries.ForgetOneappears in the publicFilesystem::batch_forgetsignature but was not exported.Uidre-export, so callers building aDispatchContextneed not depend on a matching version of nix.One optimization is also preserved: event loop read buffers are sized from the negotiated
max_writerather than the protocol maximum, which upstream allocates at 16 MiB per event loop thread.Note that
libfuseis no longer a default feature upstream, so this builds the pure-Rust mount path by default — which is what Modal wants, as it removes thelibfusecode path that GHSA-cvmj-47v9-35m9 affects.Test plan
cargo buildclean, no warningscargo test— 53 passedcargo clippy --all-targetscleanMade with Cursor