hanwen / go-fuse

FUSE bindings for Go
Other
2.04k stars 327 forks source link

Regression: xfstests generic/035 fails because Nlink is forced to 1 #137

Closed rfjakob closed 4 years ago

rfjakob commented 7 years ago

This seems to have been introduced in 3e9dd8f9e17171580aa481e9bee415a8546535d7 and causes xfstests generic/035 to fail with this:

 QA output created by 035
 overwriting regular file:
+nlink is 1, should be 0
 overwriting directory:

I suggest a revert.

rfjakob commented 7 years ago

What xfstests generic/035 does is 1) create two files, a and b 2) open b 3) rename a over b 4) fstat b and check nlink (should be zero)

C code is this: https://github.com/rfjakob/fuse-xfstests/blob/gocryptfs/src/t_rename_overwrite.c

hanwen commented 5 years ago

I don't think you could ever get that test to pass due to https://github.com/libfuse/libfuse/issues/62 - the kernel doesn't issue a FH for GetAttr calls.

hanwen commented 5 years ago

Correction: this test can pass if you build an in-memory node based FS. For loopback, you don't get FH, so there is no path to stat.

rfjakob commented 5 years ago

I see your point, and I also remember adding that comment in 8b95cbfda9742b2781cb8481fd04fa2101762529 .

However, xfstests generic/035 at the moment looks like this:

$ cat fuse-xfstests/results/generic/035.out.bad
QA output created by 035
overwriting regular file:
nlink is 1, should be 0
overwriting directory:
t_rename_overwrite: fstat(3): No such file or directory

And with 3e9dd8f9e17171580aa481e9bee415a8546535d7 reverted it looks like this:

$ cat fuse-xfstests/results//generic/035.out.bad
QA output created by 035
overwriting regular file:
overwriting directory:
t_rename_overwrite: fstat(3): No such file or directory

I'll have to check the xfstests code. Maybe it actually gets an error from the fstat but does not report it.

rfjakob commented 5 years ago

I'm not sure how, but it looks like the fstat succeeds: code

I have added a debug printf (commit), and the fstat result looks sane:

QA output created by 035
overwriting regular file:
res=0 dev=55 ino=2759954 mode=100644 nlink=0 uid=0
overwriting directory:
t_rename_overwrite: fstat(3): No such file or directory
res=-1 dev=0 ino=0 mode=0 nlink=0 uid=0
rfjakob commented 5 years ago

Debug log:

rx 94: OPEN i5 {O_RDONLY,0x8000} # open file2
tx 94:     OK, {Fh 2 }
rx 96: RENAME i3 {i3} ["file1" "file2"] 12b
tx 96:     OK
rx 98: GETATTR i5 {Fh 0}
tx 98:     OK, {tA=1s {M0100644 SZ=0 L=0 0:0 B0*4096 i0:2756255 A 1551010887.001112 M 1551010887.001112 C 1551010887.002746}}

This works because of

file = n.Inode().AnyFile()

https://github.com/hanwen/go-fuse/blob/fb9b7327a6ab6a80f4a61aab97f021ebcea25142/fuse/pathfs/pathfs.go#L591

hanwen commented 4 years ago

Jakob, is this an issue with the v2 API ? If not, I think we can close this.

hanwen commented 4 years ago

@rfjakob - paging jakob

rfjakob commented 4 years ago

Will retest.

rfjakob commented 4 years ago

No, the nlink is 1, should be 0 is gone!