-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support newer protocol versions #122
Comments
I looked a bit through the versions, and this might not be as much work as we think. Here's the protocol version changelog (from
There's also not that much logic based on the minor version: /fs/fuse $ rg minor
cuse.c
321: arg->major != FUSE_KERNEL_VERSION || arg->minor < 11) {
325: fc->minor = arg->minor;
338: devt = MKDEV(arg->dev_major, arg->dev_minor);
443: arg->minor = FUSE_KERNEL_MINOR_VERSION;
603: .minor = CUSE_MINOR,
dir.c
769: if (fc->no_rename2 || fc->minor < 23)
1268: if (fc->minor < 18)
1279: if (fc->minor < 18)
1412: if (fc->minor >= 23) {
fuse_i.h
720: /** Negotiated minor version */
721: unsigned minor;
dev.c
593: if (fc->minor < 4 && args->in.h.opcode == FUSE_STATFS)
596: if (fc->minor < 9) {
612: if (fc->minor < 12) {
633: /* Needs to be done after fuse_get_req() so that fc->minor is valid */
1293: if (fc->minor < 16 || fiq->forget_list_head.next->next == NULL)
2362: .minor = FUSE_MINOR,
inode.c
855: if (arg->minor < 13)
889: if (arg->minor >= 6) {
895: if (arg->minor >= 17) {
904: if (arg->minor >= 9) {
952: fc->minor = arg->minor;
953: fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
966: arg->minor = FUSE_KERNEL_MINOR_VERSION;
file.c
965: if (ff->fc->minor < 9)
2480: if (fc->minor < 16) { Looks like most additional features are actually enabled through flags instead: fs/fuse $ rg arg-\>flags
cuse.c
330: cc->unrestricted_ioctl = arg->flags & CUSE_UNRESTRICTED_IOCTL;
444: arg->flags |= CUSE_UNRESTRICTED_IOCTL;
inode.c
891: if (arg->flags & FUSE_ASYNC_READ)
893: if (!(arg->flags & FUSE_POSIX_LOCKS))
896: if (!(arg->flags & FUSE_FLOCK_LOCKS))
899: if (!(arg->flags & FUSE_POSIX_LOCKS))
902: if (arg->flags & FUSE_ATOMIC_O_TRUNC)
906: if (arg->flags & FUSE_EXPORT_SUPPORT)
909: if (arg->flags & FUSE_BIG_WRITES)
911: if (arg->flags & FUSE_DONT_MASK)
913: if (arg->flags & FUSE_AUTO_INVAL_DATA)
915: if (arg->flags & FUSE_DO_READDIRPLUS) {
917: if (arg->flags & FUSE_READDIRPLUS_AUTO)
920: if (arg->flags & FUSE_ASYNC_DIO)
922: if (arg->flags & FUSE_WRITEBACK_CACHE)
924: if (arg->flags & FUSE_PARALLEL_DIROPS)
926: if (arg->flags & FUSE_HANDLE_KILLPRIV)
930: if ((arg->flags & FUSE_POSIX_ACL)) {
935: if (arg->flags & FUSE_CACHE_SYMLINKS)
937: if (arg->flags & FUSE_ABORT_ERROR)
939: if (arg->flags & FUSE_MAX_PAGES) {
968: arg->flags |= FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_ATOMIC_O_TRUNC | I don't think we should assume a version provides a specific set of flags by default, but rather check at runtime based on what flags were agreed upon during initialization. libfuse seems to do it this way. I wonder if OSXFUSE doesn't support the same set of flags for a version? That'd be good motivation to do it this way. I'm mostly concerned with structs changing shape between versions. If we gate struct changes behind a build-time version flag, it implies only supporting equal or newer server versions. If we want to support older server versions, we'd need runtime logic to handle different struct versions and I don't see a point to have build-time flags in that case. You could imagine an application supporting old FUSE versions, but also taking advantage of newer FUSE features if they are available. I don't think that would be possible with build-time versioning. I assume these structs only grow in size between versions, and we could put the new fields behind Thoughts? |
Thanks for digging into that. I think the major feature change that requires major API changes are notifications (and things like kernel cache invalidations, which are built on top of that), right? But exposing the various flags seems easy, and that'd be pretty good to have (to be able to enable parallel dir os for example). Does libfuse do anything special about struct shape changes? I'd assume not and that the protocol changes have considered this in a backwards-compatible manner. |
As far as I remember, 7.8 was the most common version supported by all platforms back those days. I agree that this is ancient and that we should move on to a newer version. The fuse-abi crate already has feature gates for each ABI version up to 7.19. Adding more versions shouldn't be hard. To actually use a newer ABI, we just need switch on the feature flag and add the corresponding implementation to the fuse crate. To keep this more structured, I'd personally do this step by step for every minor version so that it's easy to verify that we don't miss a thing that was introduced with a version. However, rust-fuse is acting a little different than libfuse. Libfuse is happy to talk to any older kernel driver down to 7.8 (which may also be the reason why I chose 7.8 back then). Libfuse then does things differently at runtime based on the ABI version. Rust-fuse instead requires the minimum version and currently rejects to work with older kernel drivers. Which means we either need to find a way to handle different versions at runtime or settle on a widely supported version (on Mac, the latest seems to be 7.19). I'd suggest to catch up to 7.19 first, see how it works out, then think about what we can do to optionally support newer versions. The fuse_kernel types only grow with each version (or use up previously reserved fields), which is convenient for libfuse since structs can be partially read (from old kernel drivers) and still be used if the unused fields are ignored at runtime (based on the negotiated version). This however doesn't play well with strict typing and safety in Rust. Using Option fields might be a solution, but would also make the deserialization of replies (which is time-critical for performance) more complex. It also wouldn't play well with the convention of 1:1 mapping of types in ffi crates. Having different types for every ABI version would however become a nightmare quickly. |
This issue is to start a discussion to see whatever is needed to support newer kernel protocol versions. I'm particularly interested in this for two reasons: first, to support parallelization, which shouldn't be too difficult; and, second but more complex, to support cache invalidations.
Now, the question is: do we need to worry about maintaining compatibility with the older protocol versions? Why?
The current library supports protocol version 7.8, but this is ancient. Looking at the history of
fuse_kernel.h
, the oldest changes recorded are from 2008 and they already document 7.11, which means 7.8 is likely some years older. I don't think it's worth supporting such old versions, if only because I doubt the Rust toolchain could be used to target the corresponding old Linux distributions.That said, this doesn't mean "ignore all older versions" because some protocol versions are quite recent and supporting their differences is probably a necessity. But... that also depends on whoever the users of this library are. If they don't care about older versions (I, as a user, do not for example)... maybe the library shouldn't either until the demand appears, for simplicity reasons.
So the question is: what should be the minimum protocol version that we are willing to support? Once that is fixed, we can see how the newer features should optionally be exposed. But I think targeting 7.8 as the minimum version is a mistake.
CC @kornholi
The text was updated successfully, but these errors were encountered: