Thanks very much for the contribution @jankratochvil !

This fails CI, it's not immediately obvious to be why, would need to be fixed before merged, this script:

https://github.com/inotify-tools/inotify-tools/blob/master/build_and_test_multi.sh

re-creates what the CI does.

Tests fail rapidly, might be worth poking @amir73il

I am missing context.
Are you saying that fanotify was never enabled and tests never ran?
This sounds strange to me.
I am pretty sure that the fanotify tests were running and pass when we merged fanotity support.
@ericcurtin is that not the case?

I thought so, I haven't had a moment to look at this in detail yet.

Since the initial commit 0971a97 without an explicit configure --enable-fanotify the fanotify was disabled. So the testing scripts did never test it.
And in this commit there was build_and_test.sh but not build_and_test_multi.sh.
When build_and_test_multi.sh got introduced fanotify was not being tested as being disabled by default.
And fanotify for some reason (kernel bug? I did not investigate) does not work in containers.

rerunnning the checks

I think the existing errors are false positives only.

@jankratochvil what do you mean by "fanotify does not work in containers"?
it should definitely work in privileged containers, which is what build_and_test_multi.sh uses.
if you see otherwise please report a bug with more details.
It looks to me like the tests are failing for one of the reasons below:

1. If the test is run as root, mount_filesystem is missing mkfs.$fstype -F img (to force mkfs on image file)
2. If the test is not run as root, the check if fanotify_supported --filesystem never gets to check privileges as the helper claims to do. the --filesystem needs to check if the user running the test is root

I have a vague memory about @ericcurtin running the tests manually with sudo before merging, because the CI does not run them as root, but I may be wrong.

In any case, the tests fsnotifywait --fanotify (without --filesystem) should be able to run and pass inside or outside of containers.

I have a pretty simple patch to allow fanotify --filesystem watch inside rootless containers (amir73il/linux@2e6f12e) but that support is not upstream and nobody asked me to expedite it.

If rootless fanotify --filesystem support would be merged, the test would have to change to mount a tmpfs
Not sure why I chose ext2 for the test - possibly so the test could run on kernels before support for fanotify+tmpfs (amir73il/linux@59cda49)

In case you guys haven't noticed, I ported this codebase to C++ also, but I plan on using a restricted set of C++ the "-nodefaultlibs -lc -fno-exceptions" kind so we don't gain any extra runtime dependencies.

I am running everything as root (in KVM) and I get:

Couldn't watch test-symlink: Operation not supported
fanotify_mark(3, FAN_MARK_ADD|FAN_MARK_DONT_FOLLOW, FAN_ACCESS|FAN_MODIFY|FAN_ATTRIB|FAN_CLOSE_WRITE|FAN_CLOSE_NOWRITE|FAN_OPEN|FAN_MOVED_FROM|FAN_MOVED_TO|FAN_CREATE|FAN_DELETE|FAN_DELETE_SELF|FAN_MOVE_SELF|FAN_ONDIR|FAN_EVENT_ON_CHILD, AT_FDCWD, "test-symlink") = -1 EOPNOTSUPP (Operation not supported)

It has been run by

host# id
uid=0(root) gid=0(root) groups=0(root)
host# id=$(sudo docker run --privileged -d -it fedora:38 /bin/sh);sudo docker exec -it $id /bin/sh -c "mkdir -p $PWD";sudo docker cp $PWD $id:$PWD/..;sudo docker exec -it $id /bin/bash
container# id
uid=0(root) gid=0(root) groups=0(root)
container# (rm -rf mydir;mkdir mydir;cd mydir;rm -f test-symlink;touch test && ln -s test test-symlink && {(sleep 1 && touch -h test-symlink)&} && ../src/fsnotifywait --fanotify --quiet --no-dereference --timeout 2 test-symlink;echo $?)

It has been tested on:

• FAIL kernel-6.3.12-200.fc38.x86_64
• FAIL kernel-6.5.0-0.rc1.20230711git3f01e9fed845.12.fc39.x86_64
  The second kernel should already include: fanotify: support reporting non-decodeable file handles

So I do not know what else to blame than kernel.

Ah it's overlayfs ? This needs more code that is not upstream https://github.com/amir73il/linux/commits/ovl_encode_fid

Can you run the test on a directory that is shared from the host and not on overlayfs? Or mount a filesystem for running the test.
You can use the same helper that is used to mount ext2 fs for --filesystem test.

Ah it's overlayfs ?

Yes.

Can you run the test on a directory that is shared from the host and not on overlayfs? Or mount a filesystem for running the test. You can use the same helper that is used to mount ext2 fs for --filesystem test.

That is what happens by default when using containers. Maybe it could also run a test in non-containerized mode to have better testing coverage? Personally I just wanted to enable the fanotify functionality when you did already implement it. I have tried to modify .github/workflows/build.yml but it did disable all the tests.

Are there container filesystems we can use that supports this other than overlayfs? If no, we can run outside containers, just like to do everything in containers to be able to easily migrate CI or run locally etc.

Are there container filesystems we can use that supports this other than overlayfs? If no, we can run outside containers, just like to do everything in containers to be able to easily migrate CI or run locally etc.

There are several way to fix this. I will see if we can generalize the test later

Are there container filesystems we can use that supports this other than overlayfs? If no, we can run outside containers, just like to do everything in containers to be able to easily migrate CI or run locally etc.

There are several way to fix this. I will see if we can generalize the test later

@ericcurtin I created an improved version of fanotify_supported(). please see https://github.com/amir73il/inotify-tools/commits/fsnotify. This branch includes the fanotify build fixes by @jankratochvil, but not his Fedora fixes which are unrelated to fanotify

@amir73il cool, feel free to open a PR for that, that one looks looks good to me. This one does too! Merging...

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information