-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Kernel+Userland: Add FUSE support #23098
Conversation
This is a very interesting feature. I imagine we could port filesystems like exfat (which are licensed with a license that doesn't permit us to directly port it to our codebase), which is quite nice. |
b1ac63f
to
505052f
Compare
Latest push resolves the conflicts, and also improves the FUSE device so that it should now properly support multiple clients. |
93ed671
to
115f8c5
Compare
@supercomputer7 do you have any time to look at this? |
Sure. I will give this a review soon. |
|
||
fuse_out_header* header = bit_cast<fuse_out_header*>(response->data()); | ||
if (header->error) | ||
return Error::from_errno(-header->error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something smelly about this code. We follow a raw pointer here, and we negate the return code. Can we at least VERIFY
that the pointer is valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's a problem here, as the pointer is backed by a KBuffer
, and send_request_and_wait_for_a_reply()
guarantees that said buffer is at least large enough to contain a fuse_out_header
. The negation of the error code is needed because Error::from_errno()
expects a positive input, while errors in FUSE are always negative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better way than just casting it after the call site? maybe return fuse_out_header
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not quite as simple as just returning a fuse_out_header
would be, since often the response buffer should actually contain more data than just a header, such as when we send (for example) a FUSE_READ
or a FUSE_READDIR
request. Properly parsing the response also depends on the type of the original request, so its hard to make send_request_and_wait_for_a_reply()
have a more specific return type.
This is useful for parsing user-provided integers that should be interpreted as octals.
Just want to say I am really happy about the amendments you made to your patches - it looks much better now! :) |
Thanks! I haven't gotten around to using Discord yet, but I appreciate the review here :) |
This adds both the fuse device (used for communication between the kernel and the filesystem) and filesystem implementation itself.
This only contains the bare minimum amount of functionality required by libfuse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty sweet, thanks for expanding our Filesystem code! This looks like it's in a good enough state to iterate on in-tree. I'm a bit nervous about the Dual-licensed Defintions file, but given that one of the licenses was BSD 2 Clause, the same as our license, I think(?) your modifications on top of it are suitable to not get us in trouble. It might be a good idea in a follow-up to add a link or blurb about where the file is orignally from (Linux/BSD/Some other FOSS project/etc) and why we want to keep its license intact rather than using the SPDX wording.
At this point, there's practically full support for reading, and limited support for writing (modifying inodes should work for the most part, but there's no support for altering the directory structure.)
You'll need to be root for all interactions with the filesystem, as support for other uids and gids is currently nonexistent :p
To test this, you can use the following self-contained POC daemon:
source
Alternatively, you can use the example programs from the libfuse port, which I'll PR shortly.