Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails xfstests generic/035, directory file handle is lost #55

Closed
rfjakob opened this issue Sep 22, 2015 · 11 comments
Closed

Fails xfstests generic/035, directory file handle is lost #55

rfjakob opened this issue Sep 22, 2015 · 11 comments

Comments

@rfjakob
Copy link
Contributor

rfjakob commented Sep 22, 2015

loopbackfs (actually, anything that implements the nodefs API) fails xfstests generic/035.

What this test does is simple:

open("/testdir/20740/dir2", O_RDONLY) = 3
rename("/testdir/20740/dir1", "/testdir/20740/dir2") = 0
fstat(3, 0x7ffd603192d0)                = -1 ENOENT (No such file or directory)

This fails because unlike handles that point to files, handles that point to directories are not kept open until the user closes them.

nodefs.OpenDir() should work like Open() and pass the file handle up.

@rfjakob
Copy link
Contributor Author

rfjakob commented Sep 22, 2015

By the way, the good news is that tests 002 to 034 pass!
(or are skipped because they need a block device)

@hanwen
Copy link
Owner

hanwen commented Sep 24, 2015

your explanation sounds plausible, but I can't repro.

func TestOpenDirFStat(t *testing.T) {
tc := NewTestCase(t)
defer tc.Cleanup()

if err := os.Mkdir(tc.origSubdir, 0777); err != nil {
    t.Fatalf("Mkdir failed: %v", err)
}

fd, err := syscall.Open(tc.mountSubdir, syscall.O_RDONLY, 0755);
if err != nil {
    t.Fatalf("Open(%q): %v", tc.mountSubdir, err)
}
defer syscall.Close(int(fd))

if err := os.Rename(tc.mountSubdir, tc.mountSubdir + ".renamed"); err != nil {
    t.Fatalf("rename(%q): %v", tc.mountSubdir, err)
}

var result syscall.Stat_t
if err := syscall.Fstat(int(fd), &result); err != nil {
    t.Fatalf("dir.stat after rename: %v", err)
}

}

gives me

$ go test -v -run OpenDirFStat
=== RUN TestOpenDirFStat
2015/09/24 11:13:31 Dispatch: INIT, NodeId: 0. data: _fuse.InitIn: {7.23 Ra 0x20000 SPLICE_WRITE,POSIX_LOCKS,EXPORT_SUPPORT,BIG_WRITES,DONT_MASK,SPLICE_READ,ASYNC_READ,ATOMIC_O_TRUNC,READDIRPLUS_AUTO,SPLICE_MOVE,FLOCK_LOCKS,AUTO_INVAL_DATA,READDIRPLUS,0x38000}
2015/09/24 11:13:31 Serialize: INIT code: OK value: {7.21 Ra 0x20000 AUTO_INVAL_DATA,READDIRPLUS,BIG_WRITES,ASYNC_READ 0/0 Wr 0x10000}
2015/09/24 11:13:31 Dispatch: ACCESS, NodeId: 1. data: {r}
2015/09/24 11:13:31 Serialize: ACCESS code: OK value:
2015/09/24 11:13:31 Dispatch: LOOKUP, NodeId: 1. names: [.Trash] 7 bytes
2015/09/24 11:13:31 Serialize: LOOKUP code: 2=no such file or directory value: {0 G0 E0.000000000 A0.000000000 {M00 SZ=0 L=0 0:0 B0_0 i0:0 A 0.000000000 M 0.000000000 C 0.000000000}}
2015/09/24 11:13:31 Dispatch: LOOKUP, NodeId: 1. names: [.Trash-1000] 12 bytes
2015/09/24 11:13:31 Serialize: LOOKUP code: 2=no such file or directory value: {0 G0 E0.000000000 A0.000000000 {M00 SZ=0 L=0 0:0 B0_0 i0:0 A 0.000000000 M 0.000000000 C 0.000000000}}
2015/09/24 11:13:31 Dispatch: LOOKUP, NodeId: 1. names: [subdir] 7 bytes
2015/09/24 11:13:31 Serialize: LOOKUP code: OK value: {3 G0 E0.100000000 A0.100000000 {M040777 SZ=40 L=2 1000:1000 B0_4096 i0:3 A 1443086011.450558775 M 1443086011.450558775 C 1443086011.450558775}}
2015/09/24 11:13:31 Dispatch: OPENDIR, NodeId: 3.
2015/09/24 11:13:31 Serialize: OPENDIR code: OK value: {Fh 2 }
2015/09/24 11:13:31 Dispatch: LOOKUP, NodeId: 1. names: [subdir.renamed] 15 bytes
2015/09/24 11:13:31 Serialize: LOOKUP code: 2=no such file or directory value: {0 G0 E0.000000000 A0.000000000 {M00 SZ=0 L=0 0:0 B0_0 i0:0 A 0.000000000 M 0.000000000 C 0.000000000}}
2015/09/24 11:13:31 Dispatch: RENAME, NodeId: 1. data: {1} names: [subdir subdir.renamed] 22 bytes
2015/09/24 11:13:31 Serialize: RENAME code: OK value:
2015/09/24 11:13:31 Dispatch: GETATTR, NodeId: 3. data: {Fh 0}
2015/09/24 11:13:31 Serialize: GETATTR code: OK value: {A0.100000000 {M040777 SZ=40 L=2 1000:1000 B0_4096 i0:3 A 1443086011.451558799 M 1443086011.450558775 C 1443086011.451558799}}
2015/09/24 11:13:31 Dispatch: RELEASEDIR, NodeId: 3. data: {Fh 2 0x8000 L0}
2015/09/24 11:13:31 Serialize: RELEASEDIR code: OK value:
2015/09/24 11:13:31 Dispatch: BATCH_FORGET, NodeId: 0. data: {2} 32 bytes
--- PASS: TestOpenDirFStat (0.02s)
PASS
ok github.com/hanwen/go-fuse/fuse/test 0.024s

as you can see, the kernel sends the inode correctly, so the fstat succeeds.

@rfjakob
Copy link
Contributor Author

rfjakob commented Sep 24, 2015

Interesting. This is what the test does: https://github.com/rfjakob/fuse-xfstests/blob/gocryptfs/src/t_rename_overwrite.c
I cannot check right now, but I believe I can repro this every time using t_rename_overwrite.c . Will check again tonight.

@rfjakob
Copy link
Contributor Author

rfjakob commented Sep 24, 2015

I think your test is missing the second directory - t_rename_overwrite.c overwrites a directory with another directory.

  1. Create dir1, Create dir2
  2. Open dir2
  3. Rename dir1 over dir2
  4. Fstat dir2

This is what I get using t_rename_overwrite.c :

2015/09/24 21:45:18 Dispatch: LOOKUP, NodeId: 1. names: [dir2] 5 bytes
2015/09/24 21:45:18 Serialize: LOOKUP code: OK value: {3 G0 E1.000000000 A1.000000000 {M040775 SZ=4096 L=2 1026:1026 B8*4096 i0:3 A 1443123875.118179028 M 1443123875.118179028 C 1443123887.105220165}}
2015/09/24 21:45:18 Dispatch: OPENDIR, NodeId: 3. 
2015/09/24 21:45:18 Serialize: OPENDIR code: OK value: {Fh 2 }
2015/09/24 21:45:18 Dispatch: LOOKUP, NodeId: 1. names: [dir1] 5 bytes
2015/09/24 21:45:18 Serialize: LOOKUP code: OK value: {4 G0 E1.000000000 A1.000000000 {M040775 SZ=4096 L=2 1026:1026 B8*4096 i0:4 A 1443123904.606280228 M 1443123904.606280228 C 1443123904.606280228}}
2015/09/24 21:45:18 Dispatch: RENAME, NodeId: 1. data: {1} names: [dir1 dir2] 10 bytes
2015/09/24 21:45:18 Serialize: RENAME code: OK value: 
2015/09/24 21:45:18 Dispatch: GETATTR, NodeId: 3. data: {Fh 0} 
2015/09/24 21:45:18 Serialize: GETATTR code: 2=no such file or directory value: {A0.000000000 {M00 SZ=0 L=0 0:0 B0*0 i0:0 A 0.000000000 M 0.000000000 C 0.000000000}}
2015/09/24 21:45:18 Dispatch: RELEASEDIR, NodeId: 3. data: {Fh 2 0x8000  L0} 
2015/09/24 21:45:18 Serialize: RELEASEDIR code: OK value: 
2015/09/24 21:45:18 Dispatch: FORGET, NodeId: 3. data: {1}

t_rename_overwrite: fstat(3): No such file or directory

I can reproduce this every time.

@hanwen
Copy link
Owner

hanwen commented Sep 25, 2015

interesting. I didn't know rename for a directory could have an empty directory as target.

so, note that fstat doesnt have Fh set, but IIRC that also holds for non-directory fstat. I'll have a look to see if I can so something.

@rfjakob
Copy link
Contributor Author

rfjakob commented Sep 25, 2015 via email

@hanwen
Copy link
Owner

hanwen commented Sep 25, 2015

mv dir1 dir2 actually executes

rename("dir1", "dir2/dir1")

but I can repro the problem now.

@rfjakob
Copy link
Contributor Author

rfjakob commented Sep 25, 2015 via email

@rfjakob
Copy link
Contributor Author

rfjakob commented Sep 25, 2015 via email

rfjakob added a commit to rfjakob/go-fuse that referenced this issue Dec 26, 2019
The test seemed to pass because the inode number is overridden
in rawBridge.getattr, but looking at the permissions shows that
the wrong directory is stat()ed:

  $ go test ./fs -run TestPosix/RenameOpenDir -count 1  -v
  [...]
  17:49:46.454077 received ENODEV (unmount request), thread exiting
  17:49:46.454343 received ENODEV (unmount request), thread exiting
  --- PASS: TestPosix (0.01s)
      --- SKIP: TestPosix/RenameOpenDir (0.01s)
          test.go:392: got permissions 0755, want 0700. Known limitation - see hanwen#55
  PASS
  ok    github.com/hanwen/go-fuse/v2/fs 0.016s

Also, add a log message whenever the inode number is overridden,
this should (probably) not happen during normal operation. And
it actually only happens once in the test suite (in RenameOpenDir):

  $ go test ./... -count 1 -v 2>&1 | grep "overriding ino"
  14:48:44.143694 warning: rawBridge.getattr: overriding ino 188663 with 186314

See hanwen#55

Change-Id: I8b2ddb84c35a3b28b4f5e032e7113f8d484a5981
rfjakob added a commit to rfjakob/go-fuse that referenced this issue Dec 26, 2019
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 hanwen#55
  PASS
  ok    github.com/hanwen/go-fuse/v2/fs 0.014s

Change-Id: I2eb6fd48a11df543c9b7daf62647cb9d8a892568
gerritforge-ltd pushed a commit that referenced this issue Jan 3, 2020
The test seemed to pass because the inode number is overridden
in rawBridge.getattr, but looking at the permissions shows that
the wrong directory is stat()ed:

  $ go test ./fs -run TestPosix/RenameOpenDir -count 1  -v
  [...]
  17:49:46.454077 received ENODEV (unmount request), thread exiting
  17:49:46.454343 received ENODEV (unmount request), thread exiting
  --- PASS: TestPosix (0.01s)
      --- SKIP: TestPosix/RenameOpenDir (0.01s)
          test.go:392: got permissions 0755, want 0700. Known limitation - see #55
  PASS
  ok    github.com/hanwen/go-fuse/v2/fs 0.016s

Also, add a log message whenever the inode number is overridden,
this should (probably) not happen during normal operation. And
it actually only happens once in the test suite (in RenameOpenDir):

  $ go test ./... -count 1 -v 2>&1 | grep "overriding ino"
  14:48:44.143694 warning: rawBridge.getattr: overriding ino 188663 with 186314

See #55

Change-Id: I8b2ddb84c35a3b28b4f5e032e7113f8d484a5981
gerritforge-ltd pushed a commit that referenced this issue Jan 3, 2020
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
@rfjakob
Copy link
Contributor Author

rfjakob commented Feb 11, 2023

Note: xfstests generic/035 still fails, because the fstat cannot succeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants