-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(fuse): Expose MFS as FUSE mount point #10781
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
base: master
Are you sure you want to change the base?
Whoa there!
You have triggered an abuse detection mechanism.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
Conversation
Core functionality should be ready, need to write some tests and change up the documentation. |
a85c978
to
5c7fa80
Compare
222d4dc
to
d1b4cc1
Compare
Tests ready now, PR should be good for review. |
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 didn't review everything at the moment.
@gsergey418 were you able to test mounting /mfs
and interact with that directory (read/write)?
I didn't manage to test myself, simply mounting /ipfs
and /ipns
(using master) broke fuse in my debian VM test setup.
Would you mind updating fuse.md to reflect these changes?
@guillaumemichel Yeah, it mounts and umounts successfully on my machine and I can interact with it like a normal filesystem (ls, cat, touch, mkdir, stat, etc...). Reading and writing seems to work correctly and keeps the files intact. I've changed the variable names like you requested and added documentation. |
I think it would be good to add some way of displaying the file's CID, perhaps through xattrs, if you want full MFS functionality. |
Triage note: thank you, due to holiday season, we may be slower to respond, will look into this after tackling priority work for 0.35.0-rc1 |
Thanks @gsergey418 for the PR. I have tested this branch on a fresh Debian Bookworm install, and it seems that most operations work great in fuse. E.g files added with
I didn't manage to display a file's CID using xattrs $ xattr /mfs/myDir/hello.txt
[Errno 95] Operation not supported: b'/mfs/myDir/hello.txt'
$ xattr /home/guissou/.ipfs/config
$ ... The latest command works on a file outside of MFS (but I guess the file doesn't have any xattr attribute). Is this feature working yet? It seems that in the mounted MFS (fuse), files are readonly. It is possible to override a file, but not to edit it. It that expected? What would it take for files to be mutable? |
@guillaumemichel Thank you for the review. I will look into it. |
@guillaumemichel The xattr was there, you just couldn't list available xattrs because the structures didn't implement the fs.NodeListXattrer interface. I've added the support for it, so it should work now.
|
fuse/mfs/mfs_unix.go
Outdated
@@ -187,6 +187,12 @@ func (dir *Dir) Create(ctx context.Context, req *fuse.CreateRequest, resp *fuse. | |||
return &file, &handler, nil | |||
} | |||
|
|||
// List dir xattr. | |||
func (dir *Dir) Listxattr(ctx context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { | |||
resp.Append("ipfs_cid") |
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.
"ipfs_cid"
should be defined as a const instead of being hardcoded in multiple places.
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 works great!
@gsergey418 could you also add some sharness tests to make sure /mfs
works as expected (e.g by adding a file using ipfs add --to-files
and verify that it appears in /mfs
, copying a file using cp myFile /mfs/myFile
and verifying with ipfs files ls
that the file exists). It would be great to test all commands (touch
, ls
, mkdir
, cp
, mv
, rm
), and ideally also the xattrs.
Let us know if you have issues with dependencies in the tests setup (fuse
, xattrs
)
I've noticed some problems with setting file permission which is done by some programs like sed and vim, which was most likely causing the immutability problem. Another problem with the file not being overwritten on mv. I'll fix those and let you know. |
@guillaumemichel The thing with vim and sed was they were trying to change file metadata (mode, gid, uid, mtime, etc.) and couldn't do it. Right now mfs.File and mfs.Directory support only changing the mode and mtime of a filesystem node. Another struggle is that you can't fool these programs into thinking that they successfully changed the permissions as they request file attributes afterwards. You could hack it so it would remember the changed attributes in memory and return them with Attr(), or come up with a separate mechanism of persisting them, which, for both of those solutions, would be too much overhead. You could also change the MFS code itself, which is also sort of extreme. I suggest for now, we don't implement the chmod/chown functionality (except maybe for mode/mtime) and inform users to avoid manipulating files like that. Anyways, have a good weekend. |
@gsergey418 thanks for the investigation. It makes sense, we can leave it as is and inform users in the appropriate docs about what isn't supported yet. It seems like the long term solution would be to edit MFS directly to support file metadata modifications, but we can tackle it later.
You too 😄 |
Fixes #10710
Expose the MFS root as a FUSE filesystem mounted at /mfs.