From a0c1ae9f900a34efd5f8dcb7a660225f70a60133 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 19 Aug 2022 20:26:35 +0200 Subject: [PATCH 1/2] mount: Correctly return context.Canceled for interrupted syscalls bazil/fuse expects us to return context.Canceled to signal that a syscall was successfully interrupted. Returning a wrapped version of that error however causes the fuse library to signal an EIO (input/output error). Thus unwrap context.Canceled errors before returning them. --- changelog/unreleased/issue-3567 | 11 +++++++++++ internal/fuse/dir.go | 15 +++++++++++++-- internal/fuse/file.go | 2 +- internal/fuse/snapshots_dir.go | 4 ++-- 4 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 changelog/unreleased/issue-3567 diff --git a/changelog/unreleased/issue-3567 b/changelog/unreleased/issue-3567 new file mode 100644 index 000000000..8b0bf5790 --- /dev/null +++ b/changelog/unreleased/issue-3567 @@ -0,0 +1,11 @@ +Bugfix: Improve handling of interrupted syscalls in `mount` command + +Accessing restic's fuse mount could result in "input / output" errors when +using programs in which syscalls can be interrupted. This is for example the +case for go programs. + +We have corrected the error handling for interrupted syscalls. + +https://github.com/restic/restic/issues/3567 +https://github.com/restic/restic/issues/3694 +https://github.com/restic/restic/pull/3875 diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index dcacaa96a..62e419969 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -5,6 +5,7 @@ package fuse import ( "context" + "errors" "os" "path/filepath" "sync" @@ -44,6 +45,16 @@ func newDir(ctx context.Context, root *Root, inode, parentInode uint64, node *re }, nil } +// returing a wrapped context.Canceled error will instead result in returing +// an input / output error to the user. Thus unwrap the error to match the +// expectations of bazil/fuse +func unwrapCtxCanceled(err error) error { + if errors.Is(err, context.Canceled) { + return context.Canceled + } + return err +} + // replaceSpecialNodes replaces nodes with name "." and "/" by their contents. // Otherwise, the node is returned. func replaceSpecialNodes(ctx context.Context, repo restic.Repository, node *restic.Node) ([]*restic.Node, error) { @@ -57,7 +68,7 @@ func replaceSpecialNodes(ctx context.Context, repo restic.Repository, node *rest tree, err := restic.LoadTree(ctx, repo, *node.Subtree) if err != nil { - return nil, err + return nil, unwrapCtxCanceled(err) } return tree.Nodes, nil @@ -91,7 +102,7 @@ func (d *dir) open(ctx context.Context) error { tree, err := restic.LoadTree(ctx, d.root.repo, *d.node.Subtree) if err != nil { debug.Log(" error loading tree %v: %v", d.node.Subtree, err) - return err + return unwrapCtxCanceled(err) } items := make(map[string]*restic.Node) for _, n := range tree.Nodes { diff --git a/internal/fuse/file.go b/internal/fuse/file.go index 571d5a865..6995107c4 100644 --- a/internal/fuse/file.go +++ b/internal/fuse/file.go @@ -105,7 +105,7 @@ func (f *openFile) getBlobAt(ctx context.Context, i int) (blob []byte, err error blob, err = f.root.repo.LoadBlob(ctx, restic.DataBlob, f.node.Content[i], nil) if err != nil { debug.Log("LoadBlob(%v, %v) failed: %v", f.node.Name, f.node.Content[i], err) - return nil, err + return nil, unwrapCtxCanceled(err) } f.root.blobCache.Add(f.node.Content[i], blob) diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index 34bcccc4d..c23e68a2d 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -58,7 +58,7 @@ func (d *SnapshotsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { // update snapshots meta, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) if err != nil { - return nil, err + return nil, unwrapCtxCanceled(err) } else if meta == nil { return nil, fuse.ENOENT } @@ -97,7 +97,7 @@ func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) meta, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) if err != nil { - return nil, err + return nil, unwrapCtxCanceled(err) } else if meta == nil { return nil, fuse.ENOENT } From dd7cd5b9b312e6893adfdad35c13d83bc77afa06 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 19 Aug 2022 20:29:33 +0200 Subject: [PATCH 2/2] fuse: remove unused context parameter --- internal/fuse/dir.go | 12 ++++++------ internal/fuse/file.go | 2 +- internal/fuse/fuse_test.go | 4 ++-- internal/fuse/link.go | 2 +- internal/fuse/other.go | 2 +- internal/fuse/snapshots_dir.go | 6 +++--- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index 62e419969..cfb2aa71d 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -34,7 +34,7 @@ func cleanupNodeName(name string) string { return filepath.Base(name) } -func newDir(ctx context.Context, root *Root, inode, parentInode uint64, node *restic.Node) (*dir, error) { +func newDir(root *Root, inode, parentInode uint64, node *restic.Node) (*dir, error) { debug.Log("new dir for %v (%v)", node.Name, node.Subtree) return &dir{ @@ -74,7 +74,7 @@ func replaceSpecialNodes(ctx context.Context, repo restic.Repository, node *rest return tree.Nodes, nil } -func newDirFromSnapshot(ctx context.Context, root *Root, inode uint64, snapshot *restic.Snapshot) (*dir, error) { +func newDirFromSnapshot(root *Root, inode uint64, snapshot *restic.Snapshot) (*dir, error) { debug.Log("new dir for snapshot %v (%v)", snapshot.ID(), snapshot.Tree) return &dir{ root: root, @@ -206,13 +206,13 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { } switch node.Type { case "dir": - return newDir(ctx, d.root, fs.GenerateDynamicInode(d.inode, name), d.inode, node) + return newDir(d.root, fs.GenerateDynamicInode(d.inode, name), d.inode, node) case "file": - return newFile(ctx, d.root, fs.GenerateDynamicInode(d.inode, name), node) + return newFile(d.root, fs.GenerateDynamicInode(d.inode, name), node) case "symlink": - return newLink(ctx, d.root, fs.GenerateDynamicInode(d.inode, name), node) + return newLink(d.root, fs.GenerateDynamicInode(d.inode, name), node) case "dev", "chardev", "fifo", "socket": - return newOther(ctx, d.root, fs.GenerateDynamicInode(d.inode, name), node) + return newOther(d.root, fs.GenerateDynamicInode(d.inode, name), node) default: debug.Log(" node %v has unknown type %v", name, node.Type) return nil, fuse.ENOENT diff --git a/internal/fuse/file.go b/internal/fuse/file.go index 6995107c4..505b9dca2 100644 --- a/internal/fuse/file.go +++ b/internal/fuse/file.go @@ -36,7 +36,7 @@ type openFile struct { cumsize []uint64 } -func newFile(ctx context.Context, root *Root, inode uint64, node *restic.Node) (fusefile *file, err error) { +func newFile(root *Root, inode uint64, node *restic.Node) (fusefile *file, err error) { debug.Log("create new file for %v with %d blobs", node.Name, len(node.Content)) return &file{ inode: inode, diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index df24f77af..a46f9ba15 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -119,7 +119,7 @@ func TestFuseFile(t *testing.T) { root := &Root{repo: repo, blobCache: bloblru.New(blobCacheSize)} inode := fs.GenerateDynamicInode(1, "foo") - f, err := newFile(context.TODO(), root, inode, node) + f, err := newFile(root, inode, node) rtest.OK(t, err) of, err := f.Open(context.TODO(), nil, nil) rtest.OK(t, err) @@ -163,7 +163,7 @@ func TestFuseDir(t *testing.T) { } parentInode := fs.GenerateDynamicInode(0, "parent") inode := fs.GenerateDynamicInode(1, "foo") - d, err := newDir(context.TODO(), root, inode, parentInode, node) + d, err := newDir(root, inode, parentInode, node) rtest.OK(t, err) // don't open the directory as that would require setting up a proper tree blob diff --git a/internal/fuse/link.go b/internal/fuse/link.go index 6953ecfb3..a69c06f85 100644 --- a/internal/fuse/link.go +++ b/internal/fuse/link.go @@ -20,7 +20,7 @@ type link struct { inode uint64 } -func newLink(ctx context.Context, root *Root, inode uint64, node *restic.Node) (*link, error) { +func newLink(root *Root, inode uint64, node *restic.Node) (*link, error) { return &link{root: root, inode: inode, node: node}, nil } diff --git a/internal/fuse/other.go b/internal/fuse/other.go index f7745172b..48de06b97 100644 --- a/internal/fuse/other.go +++ b/internal/fuse/other.go @@ -16,7 +16,7 @@ type other struct { inode uint64 } -func newOther(ctx context.Context, root *Root, inode uint64, node *restic.Node) (*other, error) { +func newOther(root *Root, inode uint64, node *restic.Node) (*other, error) { return &other{root: root, inode: inode, node: node}, nil } diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index c23e68a2d..79c8378d8 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -105,9 +105,9 @@ func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) entry := meta.names[name] if entry != nil { if entry.linkTarget != "" { - return newSnapshotLink(ctx, d.root, fs.GenerateDynamicInode(d.inode, name), entry.linkTarget, entry.snapshot) + return newSnapshotLink(d.root, fs.GenerateDynamicInode(d.inode, name), entry.linkTarget, entry.snapshot) } else if entry.snapshot != nil { - return newDirFromSnapshot(ctx, d.root, fs.GenerateDynamicInode(d.inode, name), entry.snapshot) + return newDirFromSnapshot(d.root, fs.GenerateDynamicInode(d.inode, name), entry.snapshot) } else { return NewSnapshotsDir(d.root, fs.GenerateDynamicInode(d.inode, name), d.inode, d.dirStruct, d.prefix+"/"+name), nil } @@ -127,7 +127,7 @@ type snapshotLink struct { var _ = fs.NodeReadlinker(&snapshotLink{}) // newSnapshotLink -func newSnapshotLink(ctx context.Context, root *Root, inode uint64, target string, snapshot *restic.Snapshot) (*snapshotLink, error) { +func newSnapshotLink(root *Root, inode uint64, target string, snapshot *restic.Snapshot) (*snapshotLink, error) { return &snapshotLink{root: root, inode: inode, target: target, snapshot: snapshot}, nil }