various fixes for /ipfs fuse code #4194
Conversation
|
I had some hacking time on the plane yesterday. Yay. |
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
whyrusleeping
force-pushed
the
feat/fuse-fixes
branch
from
5f0791d
to
c8c2921
Compare
fuse/readonly/readonly_unix.go
Outdated
| switch nd := nd.(type) { | ||
| case *mdag.ProtoNode: | ||
| return &Node{Ipfs: s.Ipfs, Nd: nd}, nil | ||
| case *mdag.RawNode: |
There was a problem hiding this comment.
(nit) you can just write (I think?):
switch nd := nd.(type) {
case *mdag.ProtoNode, *mdag.RawNode:
return &Node{Ipfs: s.Ipfs, Nd: nd}, nil
default:
// ...
}
fuse/node/mount_unix.go
Outdated
| done <- struct{}{} | ||
| }() | ||
|
|
||
| <-done |
There was a problem hiding this comment.
Can we use a WaitGroup here while you're at it? This is a bit confusing at first glance.
fuse/readonly/readonly_unix.go
Outdated
| a.Mode = 0444 | ||
| a.Size = uint64(len(rawnd.RawData())) | ||
| a.Blocks = 1 | ||
| a.Uid = uint32(os.Getuid()) // TODO: should probably cache these calls. No sense making multiple syscalls for each attr call here |
There was a problem hiding this comment.
Can we not just claim that these files are owned by root (does fuse not let us do that)?
There was a problem hiding this comment.
We can, I guess that does work.
fuse/readonly/readonly_unix.go
Outdated
| // will be expensive to query each child. However most shells call an | ||
| // additional 'stat' on each item in a directory listing, its probably | ||
| // okay. | ||
| entries = append(entries, fuse.Dirent{Name: n, Type: fuse.DT_File}) |
There was a problem hiding this comment.
As someone who has written programs that rely on this being accurate... 😡. We may want to start inlining info like this into directories (but that's not very elegant).
There was a problem hiding this comment.
Also, if your argument is that most shells will call an additional stat, we might as well compute this up-front and cache it.
There was a problem hiding this comment.
Yeah, lets go ahead and do that.
| switch err { | ||
| case bserv.ErrNotFound: | ||
| return nil, ErrNotFound | ||
| default: |
There was a problem hiding this comment.
You're missing the case where err is nil (this is what's causing the segfaults in CI).
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
fuse/readonly/readonly_unix.go
Outdated
| n = link.Cid.String() | ||
| n = lnk.Cid.String() | ||
| } | ||
| // TODO: calling everything a DT_File here might cause issues. But it |
fuse/readonly/readonly_unix.go
Outdated
| } | ||
| } | ||
| default: | ||
| t = fuse.DT_Unknown |
There was a problem hiding this comment.
Maybe put this up top (i.e., t := fuse.DT_Unknown). We have several cases where we just log errors but don't assign anything (I assume it will default to unknown but that's not immediately obvious).
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
|
|
various fixes for /ipfs fuse code
Including: