Commit 3f7bb2c2 authored by Jakob Unterwurzacher's avatar Jakob Unterwurzacher Committed by Han-Wen Nienhuys

fs: detect Inode.Path() hitting an orphaned inode

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 https://github.com/hanwen/go-fuse/issues/55
  PASS
  ok    github.com/hanwen/go-fuse/v2/fs 0.014s

Change-Id: I2eb6fd48a11df543c9b7daf62647cb9d8a892568
parent ae87e918
...@@ -6,7 +6,9 @@ package fs ...@@ -6,7 +6,9 @@ package fs
import ( import (
"context" "context"
"log"
"os" "os"
"strings"
"syscall" "syscall"
"testing" "testing"
"unsafe" "unsafe"
...@@ -133,3 +135,67 @@ func (fn *testTypeChangeIno) Lookup(ctx context.Context, name string, out *fuse. ...@@ -133,3 +135,67 @@ func (fn *testTypeChangeIno) Lookup(ctx context.Context, name string, out *fuse.
child := fn.NewInode(ctx, childFN, stable) child := fn.NewInode(ctx, childFN, stable)
return child, syscall.F_OK 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
}
...@@ -8,6 +8,7 @@ import ( ...@@ -8,6 +8,7 @@ import (
"context" "context"
"fmt" "fmt"
"log" "log"
"math/rand"
"sort" "sort"
"strings" "strings"
"sync" "sync"
...@@ -267,7 +268,11 @@ func (n *Inode) Operations() InodeEmbedder { ...@@ -267,7 +268,11 @@ func (n *Inode) Operations() InodeEmbedder {
return n.ops 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 { func (n *Inode) Path(root *Inode) string {
var segments []string var segments []string
p := n p := n
...@@ -276,12 +281,18 @@ func (n *Inode) Path(root *Inode) string { ...@@ -276,12 +281,18 @@ func (n *Inode) Path(root *Inode) string {
// We don't try to take all locks at the same time, because // We don't try to take all locks at the same time, because
// the caller won't use the "path" string under lock anyway. // the caller won't use the "path" string under lock anyway.
found := false
p.mu.Lock() p.mu.Lock()
// Select an arbitrary parent // Select an arbitrary parent
for pd = range p.parents { for pd = range p.parents {
found = true
break break
} }
p.mu.Unlock() p.mu.Unlock()
if found == false {
p = nil
break
}
if pd.parent == nil { if pd.parent == nil {
break break
} }
...@@ -290,9 +301,12 @@ func (n *Inode) Path(root *Inode) string { ...@@ -290,9 +301,12 @@ func (n *Inode) Path(root *Inode) string {
p = pd.parent 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? // NOSUBMIT - should replace rather than append?
segments = append(segments, ".deleted") segments = append(segments, deletedPlaceholder)
} }
i := 0 i := 0
......
...@@ -70,7 +70,7 @@ func (n *loopbackNode) root() *loopbackRoot { ...@@ -70,7 +70,7 @@ func (n *loopbackNode) root() *loopbackRoot {
} }
func (n *loopbackNode) path() string { func (n *loopbackNode) path() string {
path := n.Path(nil) path := n.Path(n.Root())
return filepath.Join(n.root().rootPath, path) return filepath.Join(n.root().rootPath, path)
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment