Skip to content

Commit

Permalink
fs: detect Inode.Path() hitting an orphaned inode
Browse files Browse the repository at this point in the history
Some ".deleted" logic was in place, but did not work
properly when the Inode was in the root directory.

Fix that by introducing the `found` variable and
add a test that verifies that it works.

Also, return `.go-fuse.$RANDOM/deleted` to make it
very unlikely that the placeholder name matches an
actual file or directory. Introducing the `/deleted`
subdir makes sure operations creating a file or
directory fail reliably with ENOENT.

Tests now look like this:

  $ go test ./fs -run TestPosix/RenameOpenDir -count 1  -v
  === RUN   TestPosix
  === RUN   TestPosix/RenameOpenDir
  [...]
  --- PASS: TestPosix (0.01s)
      --- SKIP: TestPosix/RenameOpenDir (0.01s)
          test.go:383: Fstat failed: no such file or directory. Known limitation - see #55
  PASS
  ok    github.com/hanwen/go-fuse/v2/fs 0.014s

Change-Id: I2eb6fd48a11df543c9b7daf62647cb9d8a892568
  • Loading branch information
rfjakob authored and hanwen committed Jan 3, 2020
1 parent ae87e91 commit 3f7bb2c
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 4 deletions.
66 changes: 66 additions & 0 deletions fs/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ package fs

import (
"context"
"log"
"os"
"strings"
"syscall"
"testing"
"unsafe"
Expand Down Expand Up @@ -133,3 +135,67 @@ func (fn *testTypeChangeIno) Lookup(ctx context.Context, name string, out *fuse.
child := fn.NewInode(ctx, childFN, stable)
return child, syscall.F_OK
}

// TestDeletedInodePath checks that Inode.Path returns ".deleted" if an Inode is
// disconnected from the hierarchy (=orphaned)
func TestDeletedInodePath(t *testing.T) {
rootNode := testDeletedIno{}
mnt, _, clean := testMount(t, &rootNode, &Options{Logger: log.New(os.Stderr, "", 0)})
defer clean()

// Open a file handle so the kernel cannot FORGET the inode
fd, err := os.Open(mnt + "/dir")
if err != nil {
t.Fatal(err)
}
defer fd.Close()

// Delete it so the inode does not have a path anymore
err = syscall.Rmdir(mnt + "/dir")
if err != nil {
t.Fatal(err)
}
rootNode.deleted = true

// Our Getattr implementation `testDeletedIno.Getattr` should return
// ENFILE when everything looks ok, EILSEQ otherwise.
var st syscall.Stat_t
err = syscall.Fstat(int(fd.Fd()), &st)
if err != syscall.ENFILE {
t.Error(err)
}
}

type testDeletedIno struct {
Inode

deleted bool
}

func (n *testDeletedIno) Lookup(ctx context.Context, name string, out *fuse.EntryOut) (*Inode, syscall.Errno) {
if n.Root().Operations().(*testDeletedIno).deleted {
return nil, syscall.ENOENT
}
if name != "dir" {
return nil, syscall.ENOENT
}
childNode := &testDeletedIno{}
stable := StableAttr{Mode: syscall.S_IFDIR, Ino: 999}
child := n.NewInode(ctx, childNode, stable)
return child, syscall.F_OK
}

func (n *testDeletedIno) Opendir(ctx context.Context) syscall.Errno {
return OK
}

func (n *testDeletedIno) Getattr(ctx context.Context, f FileHandle, out *fuse.AttrOut) syscall.Errno {
prefix := ".go-fuse"
p := n.Path(n.Root())
if strings.HasPrefix(p, prefix) {
// Return ENFILE when things look ok
return syscall.ENFILE
}
// Otherwise EILSEQ
return syscall.EILSEQ
}
20 changes: 17 additions & 3 deletions fs/inode.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"fmt"
"log"
"math/rand"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -267,7 +268,11 @@ func (n *Inode) Operations() InodeEmbedder {
return n.ops
}

// Path returns a path string to the inode relative to the root.
// Path returns a path string to the inode relative to `root`.
// Pass nil to walk the hierarchy as far up as possible.
//
// If you set `root`, Path() warns if it finds an orphaned Inode, i.e.
// if it does not end up at `root` after walking the hierarchy.
func (n *Inode) Path(root *Inode) string {
var segments []string
p := n
Expand All @@ -276,12 +281,18 @@ func (n *Inode) Path(root *Inode) string {

// We don't try to take all locks at the same time, because
// the caller won't use the "path" string under lock anyway.
found := false
p.mu.Lock()
// Select an arbitrary parent
for pd = range p.parents {
found = true
break
}
p.mu.Unlock()
if found == false {
p = nil
break
}
if pd.parent == nil {
break
}
Expand All @@ -290,9 +301,12 @@ func (n *Inode) Path(root *Inode) string {
p = pd.parent
}

if p == nil {
if root != nil && root != p {
deletedPlaceholder := fmt.Sprintf(".go-fuse.%d/deleted", rand.Uint64())
n.bridge.logf("warning: Inode.Path: inode i%d is orphaned, replacing segment with %q",
n.stableAttr.Ino, deletedPlaceholder)
// NOSUBMIT - should replace rather than append?
segments = append(segments, ".deleted")
segments = append(segments, deletedPlaceholder)
}

i := 0
Expand Down
2 changes: 1 addition & 1 deletion fs/loopback.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (n *loopbackNode) root() *loopbackRoot {
}

func (n *loopbackNode) path() string {
path := n.Path(nil)
path := n.Path(n.Root())
return filepath.Join(n.root().rootPath, path)
}

Expand Down

0 comments on commit 3f7bb2c

Please sign in to comment.