Elara6331/itd
4
6
Fork 12
Code Issues 8 Pull Requests 1 Projects Releases 11 Wiki Activity

Added FUSE support #55

Merged
Elara6331 merged 65 commits from yannickulrich/itd:fuse into master 2023-03-25 22:23:52 +00:00
Conversation 4 Commits 65 Files Changed 8 +14 -15
3 changed files with 14 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 673383f795 - Show all commits

2
fuse.go
View File

@ -9,7 +9,7 @@ import (
"go.arsenm.dev/infinitime" "go.arsenm.dev/infinitime"
) )
func startFuse(ctx context.Context, dev *infinitime.Device) error { func startFUSE(ctx context.Context, dev *infinitime.Device) error {
yannickulrich marked this conversation as resolved Outdated
Elara6331 commented 2023-03-01 17:27:23 +00:00
Outdated
Review
Copy Link

This function should be called startFUSE instead to adhere to Go naming conventions

This function should be called `startFUSE` instead to adhere to Go naming conventions
yannickulrich commented 2023-03-01 19:08:01 +00:00
Outdated
Review
Copy Link

Thanks, I'm relatively new to Go, sorry for these issues

Done in 673383f

Thanks, I'm relatively new to Go, sorry for these issues Done in 673383f
// This is where we'll mount the FS // This is where we'll mount the FS
os.Mkdir(k.String("fuse.mountpoint"), 0755) os.Mkdir(k.String("fuse.mountpoint"), 0755)
yannickulrich marked this conversation as resolved Outdated
Elara6331 commented 2023-03-08 23:44:18 +00:00
Outdated
Review
Copy Link

Use os.MkdirAll here instead so that it creates parent directories, and handle the error (just return it).

Use `os.MkdirAll` here instead so that it creates parent directories, and handle the error (just return it).
yannickulrich commented 2023-03-11 13:29:49 +00:00
Outdated
Review
Copy Link

This is actually a bit more complicated than that. If the mountpoint already exists, fuse will crash. We also can't delete the mountpoint beforehand (rm: cannot remove '/tmp/itd/mnt': Transport endpoint is not connected). The best way to solve this should be calling the unmount function. How would you suggest going about doing this?

This is actually a bit more complicated than that. If the mountpoint already exists, fuse will crash. We also can't delete the mountpoint beforehand (`rm: cannot remove '/tmp/itd/mnt': Transport endpoint is not connected`). The best way to solve this should be calling the [unmount function](https://pkg.go.dev/github.com/hanwen/go-fuse/v2@v2.2.0/fuse#Server.Unmount). How would you suggest going about doing this?
Elara6331 commented 2023-03-11 18:30:43 +00:00
Outdated
Review
Copy Link

Yeah, this one is going to be a bit more complicated. The FUSE library does have a different unmount function that you could call before trying to mount the fs, but it's not exported, so we'll need to do a small hack to get access to it anyway. In the fusefs package, add a file called unmount.go with the following contents:

unmount.go 
package fusefs

import (
    _ "unsafe"

    "github.com/hanwen/go-fuse/v2/fuse"
)

func Unmount(mountPoint string) error {
    return unmount(mountPoint, &fuse.MountOptions{DirectMount: false})
}

// Unfortunately, the FUSE library does not export its unmount function,
// so this is required until that changes
//go:linkname unmount github.com/hanwen/go-fuse/v2/fuse.unmount
func unmount(mountPoint string, opts *fuse.MountOptions) error

Then, you can simply call that function at the top of startFUSE like so:

// Ignore the error because nothing might be mounted on the mountpoint
_ = fusefs.Unmount(k.String("fuse.mountpoint"))

I'll file an issue with the fuse library to export that function once this is merged.

The best way to solve this should be calling the unmount function.

I agree that this should be done. However, this doesn't solve the problem completely because if ITD panics for whatever reason, it won't get run and then the user will get a confusing message. Let me see what the best way would be to call this function on shutdown.

Yeah, this one is going to be a bit more complicated. The FUSE library does have [a different unmount function](https://github.com/hanwen/go-fuse/blob/0f728ba15b38579efefc3dc47821882ca18ffea7/fuse/mount_linux.go#L133) that you could call before trying to mount the fs, but it's not exported, so we'll need to do a small hack to get access to it anyway. In the `fusefs` package, add a file called `unmount.go` with the following contents: <details> <summary><code>unmount.go</code></summary> ```go package fusefs import ( _ "unsafe" "github.com/hanwen/go-fuse/v2/fuse" ) func Unmount(mountPoint string) error { return unmount(mountPoint, &fuse.MountOptions{DirectMount: false}) } // Unfortunately, the FUSE library does not export its unmount function, // so this is required until that changes //go:linkname unmount github.com/hanwen/go-fuse/v2/fuse.unmount func unmount(mountPoint string, opts *fuse.MountOptions) error ``` </details> Then, you can simply call that function at the top of `startFUSE` like so: ```go // Ignore the error because nothing might be mounted on the mountpoint _ = fusefs.Unmount(k.String("fuse.mountpoint")) ``` I'll file an issue with the fuse library to export that function once this is merged. > The best way to solve this should be calling the unmount function. I agree that this should be done. However, this doesn't solve the problem completely because if ITD panics for whatever reason, it won't get run and then the user will get a confusing message. Let me see what the best way would be to call this function on shutdown.
yannickulrich commented 2023-03-12 12:34:39 +00:00
Outdated
Review
Copy Link

Cool, thank you! I have implemented this 🙂

Cool, thank you! I have implemented this 🙂
root := fusefs.BuildRootNode(dev) root := fusefs.BuildRootNode(dev)

25
internal/fusefs/fuse.go
View File

@ -36,6 +36,9 @@ type ITNode struct {
path string path string
} }
var myfs *blefs.FS = nil
var inodemap map[string]uint64 = nil
yannickulrich marked this conversation as resolved Outdated
Elara6331 commented 2023-03-01 17:06:23 +00:00
Outdated
Review
Copy Link

This error needs to be handled, you can just return it in this case and update the code that calls this function to do the actual handling

This error needs to be handled, you can just return it in this case and update the code that calls this function to do the actual handling
yannickulrich commented 2023-03-01 19:09:05 +00:00
Outdated
Review
Copy Link

Done in a54ca7a

Done in a54ca7a
func BuildRootNode(dev *infinitime.Device) *ITNode { func BuildRootNode(dev *infinitime.Device) *ITNode {
inodemap = make(map[string]uint64) inodemap = make(map[string]uint64)
myfs, _ = dev.FS() myfs, _ = dev.FS()
@ -80,10 +83,6 @@ func BuildProperties(dev *infinitime.Device) {
} }
yannickulrich marked this conversation as resolved Outdated
Elara6331 commented 2023-03-01 17:07:46 +00:00
Outdated
Review
Copy Link

These variables should go above BuildRootNode because it's using them and it would be more readable that way. Also, Go doesn't require semicolons, you can remove those.

These variables should go above `BuildRootNode` because it's using them and it would be more readable that way. Also, Go doesn't require semicolons, you can remove those.
yannickulrich commented 2023-03-01 19:08:22 +00:00
Outdated
Review
Copy Link

Done in 673383f

Done in 673383f
var myfs *blefs.FS = nil;
var inodemap map[string]uint64 = nil;
var _ = (fs.NodeReaddirer)((*ITNode)(nil)) var _ = (fs.NodeReaddirer)((*ITNode)(nil))
// Readdir is part of the NodeReaddirer interface // Readdir is part of the NodeReaddirer interface
@ -206,7 +205,7 @@ func (n *ITNode) Lookup(ctx context.Context, name string, out *fuse.EntryOut) (*
for _, file := range n.lst { for _, file := range n.lst {
if file.path != n.path + "/" + name { if file.path != n.path + "/" + name {
continue; continue
} }
log.Info("LookUp successful").Str("path", file.path).Send() log.Info("LookUp successful").Str("path", file.path).Send()
@ -230,7 +229,7 @@ func (n *ITNode) Lookup(ctx context.Context, name string, out *fuse.EntryOut) (*
child := n.NewInode(ctx, operations, stable) child := n.NewInode(ctx, operations, stable)
return child, 0 return child, 0
} }
break; break
} }
log.Warn("LookUp failed").Str("path", n.path + "/" + name).Send() log.Warn("LookUp failed").Str("path", n.path + "/" + name).Send()
} }
@ -298,7 +297,7 @@ func (fh *bytesFileWriteHandle) Flush(ctx context.Context) (errno syscall.Errno)
go func() { go func() {
// For every progress event // For every progress event
for sent := range fp.Progress() { for sent := range fp.Progress() {
log.Info("Progress").Int("bytes", int(sent)).Int("of", len(fh.content)).Send(); log.Info("Progress").Int("bytes", int(sent)).Int("of", len(fh.content)).Send()
} }
yannickulrich marked this conversation as resolved Outdated
Elara6331 commented 2023-03-01 17:23:16 +00:00
Outdated
Review
Copy Link
- log.Info("Progress").Int("bytes", int(sent)).Int("of", len(fh.content)).Send()
+ log.Info("FUSE Write Progress").Int("bytes", int(sent)).Int("total", len(fh.content)).Send()
```diff - log.Info("Progress").Int("bytes", int(sent)).Int("of", len(fh.content)).Send() + log.Info("FUSE Write Progress").Int("bytes", int(sent)).Int("total", len(fh.content)).Send()
yannickulrich commented 2023-03-01 19:10:17 +00:00
Outdated
Review
Copy Link

Done in b5328ec

Done in b5328ec
}() }()
yannickulrich marked this conversation as resolved Outdated
Elara6331 commented 2023-03-08 23:39:58 +00:00
Outdated
Review
Copy Link

This is not correct. If no content has been written, you should still create the file because the user might want to create an empty file using a command like touch.

This is not correct. If no content has been written, you should still create the file because the user might want to create an empty file using a command like `touch`.
yannickulrich commented 2023-03-11 14:09:17 +00:00
Outdated
Review
Copy Link

Oh, good point, thank you. Should be fixed now.

Oh, good point, thank you. Should be fixed now.
@ -330,7 +329,7 @@ func (fh *bytesFileWriteHandle) Fsync(ctx context.Context, flags uint32) (errno
var _ = (fs.NodeGetattrer)((*ITNode)(nil)) var _ = (fs.NodeGetattrer)((*ITNode)(nil))
func (bn *ITNode) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOut) syscall.Errno { func (bn *ITNode) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOut) syscall.Errno {
log.Info("getattr").Str("path", bn.path).Send(); log.Info("getattr").Str("path", bn.path).Send()
out.Ino = bn.Ino out.Ino = bn.Ino
yannickulrich marked this conversation as resolved
Elara6331 commented 2023-03-01 17:24:20 +00:00
Review
Copy Link

These should be debug logs rather than info logs. Also, the message should be a bit more specific, something like "FUSE getattr". Same for all the similar logs.

These should be debug logs rather than info logs. Also, the message should be a bit more specific, something like "FUSE getattr". Same for all the similar logs.
yannickulrich commented 2023-03-01 19:09:58 +00:00
Review
Copy Link

Done in b5328ec

Done in b5328ec
out.Mtime = bn.self.modtime out.Mtime = bn.self.modtime
out.Ctime = bn.self.modtime out.Ctime = bn.self.modtime
@ -342,8 +341,8 @@ func (bn *ITNode) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOu
var _ = (fs.NodeSetattrer)((*ITNode)(nil)) var _ = (fs.NodeSetattrer)((*ITNode)(nil))
func (bn *ITNode) Setattr(ctx context.Context, fh fs.FileHandle, in *fuse.SetAttrIn, out *fuse.AttrOut) syscall.Errno { func (bn *ITNode) Setattr(ctx context.Context, fh fs.FileHandle, in *fuse.SetAttrIn, out *fuse.AttrOut) syscall.Errno {
log.Info("setattr").Str("path", bn.path).Send() log.Info("setattr").Str("path", bn.path).Send()
out.Size = 0; out.Size = 0
out.Mtime = 0; out.Mtime = 0
return 0 return 0
} }
@ -365,10 +364,10 @@ func (f *ITNode) Open(ctx context.Context, openFlags uint32) (fh fs.FileHandle,
} }
return fh, fuse.FOPEN_DIRECT_IO, 0 return fh, fuse.FOPEN_DIRECT_IO, 0
} else { } else {
log.Info("Opening file: read").Str("path", f.path).Send(); log.Info("Opening file: read").Str("path", f.path).Send()
fp, err := myfs.Open(f.path) fp, err := myfs.Open(f.path)
if err != nil { if err != nil {
log.Error("Opening file failed").Str("path", f.path).Err(err).Send(); log.Error("Opening file failed").Str("path", f.path).Err(err).Send()
return nil, 0, syscall.EROFS return nil, 0, syscall.EROFS
} }
@ -379,7 +378,7 @@ func (f *ITNode) Open(ctx context.Context, openFlags uint32) (fh fs.FileHandle,
go func() { go func() {
// For every progress event // For every progress event
for sent := range fp.Progress() { for sent := range fp.Progress() {
log.Info("Progress").Int("bytes", int(sent)).Int("of", int(f.self.size)).Send(); log.Info("Progress").Int("bytes", int(sent)).Int("of", int(f.self.size)).Send()
} }
}() }()

2
main.go
View File

@ -188,7 +188,7 @@ func main() {
} }
// Start fuse socket // Start fuse socket
if k.Bool("fuse.enabled") { if k.Bool("fuse.enabled") {
yannickulrich marked this conversation as resolved Outdated
Elara6331 commented 2023-03-01 17:26:43 +00:00
Outdated
Review
Copy Link

FUSE should be started before the socket (socket should always be last)

FUSE should be started before the socket (socket should always be last)
yannickulrich commented 2023-03-01 19:10:37 +00:00
Outdated
Review
Copy Link

Done in 3b96901

Done in 3b96901
err = startFuse(ctx, dev) err = startFUSE(ctx, dev)
if err != nil { if err != nil {
log.Error("Error starting fuse socket").Err(err).Send() log.Error("Error starting fuse socket").Err(err).Send()
} }