On Jun 16 2021, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Wed, Jun 16, 2021 at 8:25 PM Nikolaus Rath <nikolaus@xxxxxxxx> wrote: >> >> Hi Amir, >> >> On Wed, 16 Jun 2021, at 16:03, Amir Goldstein wrote: >> > Per request from Nikolaus, I modified the passthrough_hp example >> > to reuse inodes on last close+unlink, so it now hits the failure in the >> > new test with upstream kernel and it passes the test with this kernel fix. >> > >> > Thanks, >> > Amir. >> > >> > [2] https://github.com/libfuse/libfuse/pull/612 >> >> Actually, I am no longer sure this was a good idea. Having the libfuse test suite detect >> problems that with the kernel doesn't seem to helpful.. I think the testsuite should >> identify problems in libfuse. Currently, having the tests means that users might be >> hesitant to update to the newer libfuse because of the failing test - when in fact there >> is nothing wrong with libfuse at all. >> > > I suppose you are right. > I could take the tesy_syscalls test to xfstest, but fuse support for > xfstests is still WIP. > >> I assume the test will start failing on some future kernel (which is why it passed CL), >> and then start passing again for some kernel after that? > > I was not aware that it passes CI. > There are no test results available on github. Arg. Looks like something is broken there. I mistook the absence of results for a passing result. > I am not aware of any specific kernel version where the test should pass, > but the results also depend on the underlying filesystem. > > If your underlying filesystem is btrfs, it does not reuse inode numbers > at all, so the test will not fail. > > For me the test fails on ext4 and xfs on LTS kernel 5.10. > As I wrote in PR: > "...Fails the modified test_syscalls in this PR on upstream kernel" > > If you revert the last commit the test would pass on upstream kernel: > 80f2b8b ("passthrough_hp: excercise reusing inode numbers") > > We could make behavior of passthrough_hp example depend > on some minimal kernel protocol version or new kernel capability like > FUSE_SETXATTR_EXT if Miklos intends to merge the fix for the coming > kernel release or we could just make that new test optional via pytest option. > > After all, regardless of the kernel bug, this adds test coverage that was > missing, so it also covers a possible future regression in libfuse. > > Let me know if you want me to implement any of the listed options. I don't want an old kernel to result in libfuse unit tests failing, but I think it's a good idea to cover this case in some form. Would you be able to make the test conditional on a recent enough kernel version? Or, if that's too much work, print an error message that explains that there is a kernel bug but do fail the test? Best, -Nikolaus -- GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F »Time flies like an arrow, fruit flies like a Banana.«
