Skip to content

Commit

Permalink
Merge pull request #3875 from MichaelEischer/fix-fuse-context-cancel
Browse files Browse the repository at this point in the history
mount: Fix input/output errors for canceled syscalls
  • Loading branch information
MichaelEischer committed Sep 10, 2022
2 parents 2363e5c + dd7cd5b commit be9ccc1
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 19 deletions.
11 changes: 11 additions & 0 deletions changelog/unreleased/issue-3567
Original file line number Diff line number Diff line change
@@ -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
27 changes: 19 additions & 8 deletions internal/fuse/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package fuse

import (
"context"
"errors"
"os"
"path/filepath"
"sync"
Expand Down Expand Up @@ -33,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{
Expand All @@ -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) {
Expand All @@ -57,13 +68,13 @@ 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
}

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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -195,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
Expand Down
4 changes: 2 additions & 2 deletions internal/fuse/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions internal/fuse/fuse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/fuse/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion internal/fuse/other.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
10 changes: 5 additions & 5 deletions internal/fuse/snapshots_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -97,17 +97,17 @@ 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
}

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
}
Expand All @@ -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
}

Expand Down

0 comments on commit be9ccc1

Please sign in to comment.