[LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1

[LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1

[Amir Goldstein]

Hi Cyril,

Following patches test kernel code merged to v5.7-rc1.

Patch 1,3 test fanotify minor bug fixes, which could be backported
to stable kernels.

Patch 2 makes test fanotify15 more robust, by making less assumptions,
in preparation for adding a test case.

Patch 4 adds a new test for the new event type FAN_MODIFY_DIR, which
carries a new event format (parent fid + name).

Thanks,
Amir.

Amir Goldstein (4):
  syscalls/fanotify09: Check merging of events on directories
  syscalls/fanotify15: Minor corrections
  syscalls/fanotify15: Add a test case for inode marks
  syscalls/fanotify: New test for FAN_MODIFY_DIR

 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/fanotify/.gitignore |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h |  15 +-
 .../kernel/syscalls/fanotify/fanotify09.c     |  46 +-
 .../kernel/syscalls/fanotify/fanotify15.c     | 128 ++++-
 .../kernel/syscalls/fanotify/fanotify16.c     | 441 ++++++++++++++++++
 6 files changed, 595 insertions(+), 37 deletions(-)
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify16.c

-- 
2.17.1

[LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories

[Amir Goldstein]

In a setup of mount mark and directory inode mark the FAN_ONDIR flag
set on one mark should not imply that all events in the other mark mask
are expected on directories as well.

Add a regression test case for commit 55bf882c7f13:
   fanotify: fix merging marks masks with FAN_ONDIR

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify09.c     | 46 +++++++++++++------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 0f6a9e864..68a4e5081 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -15,6 +15,10 @@
  * Test case #2 is a regression test for commit b469e7e47c8a:
  *
  *      fanotify: fix handling of events on child sub-directory
+ *
+ * Test case #3 is a regression test for commit 55bf882c7f13:
+ *
+ *      fanotify: fix merging marks masks with FAN_ONDIR
  */
 #define _GNU_SOURCE
 #include "config.h"
@@ -57,16 +61,25 @@ static int mount_created;
 static struct tcase {
 	const char *tname;
 	unsigned int ondir;
+	const char *testdir;
 	int nevents;
 } tcases[] = {
 	{
 		"Events on children with both inode and mount marks",
 		0,
+		DIR_NAME,
 		1,
 	},
 	{
 		"Events on children and subdirs with both inode and mount marks",
 		FAN_ONDIR,
+		DIR_NAME,
+		2,
+	},
+	{
+		"Events on files and dirs with both inode and mount marks",
+		FAN_ONDIR,
+		".",
 		2,
 	},
 };
@@ -125,6 +138,20 @@ static void cleanup_fanotify_groups(void)
 	}
 }
 
+static void event_res(int ttype, int group,
+		      struct fanotify_event_metadata *event)
+{
+	int len;
+	sprintf(symlnk, "/proc/self/fd/%d", event->fd);
+	len = readlink(symlnk, fdpath, sizeof(fdpath));
+	if (len < 0)
+		len = 0;
+	fdpath[len] = 0;
+	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d path=%s",
+		group, (unsigned long long)event->mask,
+		(unsigned)event->pid, event->fd, fdpath);
+}
+
 static void verify_event(int group, struct fanotify_event_metadata *event,
 			 uint32_t expect)
 {
@@ -139,15 +166,7 @@ static void verify_event(int group, struct fanotify_event_metadata *event,
 			(unsigned long long)event->mask, (unsigned)event->pid,
 			(unsigned)getpid(), event->fd);
 	} else {
-		int len;
-		sprintf(symlnk, "/proc/self/fd/%d", event->fd);
-		len = readlink(symlnk, fdpath, sizeof(fdpath));
-		if (len < 0)
-			len = 0;
-		fdpath[len] = 0;
-		tst_res(TPASS, "group %d got event: mask %llx pid=%u fd=%d path=%s",
-			group, (unsigned long long)event->mask,
-			(unsigned)event->pid, event->fd, fdpath);
+		event_res(TPASS, group, event);
 	}
 }
 
@@ -167,9 +186,9 @@ static void test_fanotify(unsigned int n)
 	 */
 	SAFE_FILE_PRINTF(fname, "1");
 	/*
-	 * generate FAN_CLOSE_NOWRITE event on a child subdir.
+	 * generate FAN_CLOSE_NOWRITE event on a testdir (subdir or ".")
 	 */
-	dirfd = SAFE_OPEN(DIR_NAME, O_RDONLY);
+	dirfd = SAFE_OPEN(tc->testdir, O_RDONLY);
 	if (dirfd >= 0)
 		SAFE_CLOSE(dirfd);
 
@@ -210,13 +229,12 @@ static void test_fanotify(unsigned int n)
 
 	/*
 	 * Then verify the rest of the groups did not get the MODIFY event and
-	 * did not get the FAN_CLOSE_NOWRITE event on subdir.
+	 * did not get the FAN_CLOSE_NOWRITE event on testdir.
 	 */
 	for (i = 1; i < NUM_GROUPS; i++) {
 		ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
 		if (ret > 0) {
-			tst_res(TFAIL, "group %d got event", i);
-			verify_event(i, event, FAN_CLOSE_NOWRITE);
+			event_res(TFAIL, i, event);
 			if (event->fd != FAN_NOFD)
 				SAFE_CLOSE(event->fd);
 			continue;
-- 
2.17.1

[LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories

[Petr Vorel]

Hi Amir,

> In a setup of mount mark and directory inode mark the FAN_ONDIR flag
> set on one mark should not imply that all events in the other mark mask
> are expected on directories as well.

> Add a regression test case for commit 55bf882c7f13:
>    fanotify: fix merging marks masks with FAN_ONDIR

Merged this one (with simple change: added {"linux-git", "55bf882c7f13"},).
Thanks for your work!

Kind regards,
Petr

[LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories

[Matthew Bobrowski]

On Tue, Apr 21, 2020 at 09:49:59AM +0300, Amir Goldstein wrote:
> +static void event_res(int ttype, int group,
> +		      struct fanotify_event_metadata *event)
> +{
> +	int len;
> +	sprintf(symlnk, "/proc/self/fd/%d", event->fd);
> +	len = readlink(symlnk, fdpath, sizeof(fdpath));
> +	if (len < 0)
> +		len = 0;
> +	fdpath[len] = 0;
> +	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d path=%s",
> +		group, (unsigned long long)event->mask,
> +		(unsigned)event->pid, event->fd, fdpath);
> +}

Nice helper, although it would be nice not to see all these statements
clunked together like this.

> -	 * generate FAN_CLOSE_NOWRITE event on a child subdir.
> +	 * generate FAN_CLOSE_NOWRITE event on a testdir (subdir or ".")
  	   ^ s/g/G :P

Reviewed-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>

/M

[LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories

[Amir Goldstein]

On Fri, May 1, 2020 at 10:17 AM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> On Tue, Apr 21, 2020 at 09:49:59AM +0300, Amir Goldstein wrote:
> > +static void event_res(int ttype, int group,
> > +                   struct fanotify_event_metadata *event)
> > +{
> > +     int len;
> > +     sprintf(symlnk, "/proc/self/fd/%d", event->fd);
> > +     len = readlink(symlnk, fdpath, sizeof(fdpath));
> > +     if (len < 0)
> > +             len = 0;
> > +     fdpath[len] = 0;
> > +     tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d path=%s",
> > +             group, (unsigned long long)event->mask,
> > +             (unsigned)event->pid, event->fd, fdpath);
> > +}
>
> Nice helper, although it would be nice not to see all these statements
> clunked together like this.
>
> > -      * generate FAN_CLOSE_NOWRITE event on a child subdir.
> > +      * generate FAN_CLOSE_NOWRITE event on a testdir (subdir or ".")
>            ^ s/g/G :P
>
> Reviewed-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
>

Thanks for the review Matthew, but this patch has already been merged,
so those cleanups could be done at a later time.
I will address you comments to fanotify15 and fanotify16, which are
still not merged, when you are done with review.

Thanks,
Amir.

[LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories

[Matthew Bobrowski]

Ah, right.

I'll finish off the review tomorrow when I have some more time.

Chat then!

/M

On Fri, 1 May 2020, 7:05 pm Amir Goldstein, <amir73il@gmail.com> wrote:

> On Fri, May 1, 2020 at 10:17 AM Matthew Bobrowski
> <mbobrowski@mbobrowski.org> wrote:
> >
> > On Tue, Apr 21, 2020 at 09:49:59AM +0300, Amir Goldstein wrote:
> > > +static void event_res(int ttype, int group,
> > > +                   struct fanotify_event_metadata *event)
> > > +{
> > > +     int len;
> > > +     sprintf(symlnk, "/proc/self/fd/%d", event->fd);
> > > +     len = readlink(symlnk, fdpath, sizeof(fdpath));
> > > +     if (len < 0)
> > > +             len = 0;
> > > +     fdpath[len] = 0;
> > > +     tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d
> path=%s",
> > > +             group, (unsigned long long)event->mask,
> > > +             (unsigned)event->pid, event->fd, fdpath);
> > > +}
> >
> > Nice helper, although it would be nice not to see all these statements
> > clunked together like this.
> >
> > > -      * generate FAN_CLOSE_NOWRITE event on a child subdir.
> > > +      * generate FAN_CLOSE_NOWRITE event on a testdir (subdir or ".")
> >            ^ s/g/G :P
> >
> > Reviewed-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> >
>
> Thanks for the review Matthew, but this patch has already been merged,
> so those cleanups could be done at a later time.
> I will address you comments to fanotify15 and fanotify16, which are
> still not merged, when you are done with review.
>
> Thanks,
> Amir.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200501/9b454f70/attachment-0001.htm>

[LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections

[Amir Goldstein]

- Fix calculation of events buffer size
- Read file events and dir events in two batches
- Generate FAN_MODIFY event explicitly with truncate() operation
  instead of FAN_ATTRIB event implicitly with create() operation
- FAN_MODIFY and FAN_DELETE_SELF may or may not be merged

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify15.c     | 58 ++++++++++++++-----
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index e0d513025..454441bfe 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -25,8 +25,14 @@
 #if defined(HAVE_SYS_FANOTIFY_H)
 #include <sys/fanotify.h>
 
-#define BUF_SIZE 256
-#define EVENT_MAX 256
+#define EVENT_MAX 10
+
+/* Size of the event structure, not including file handle */
+#define EVENT_SIZE (sizeof(struct fanotify_event_metadata) + \
+		    sizeof(struct fanotify_event_info_fid))
+/* Double events buffer size to account for file handles */
+#define EVENT_BUF_LEN (EVENT_MAX * EVENT_SIZE * 2)
+
 
 #define MOUNT_POINT "mntpoint"
 #define TEST_DIR MOUNT_POINT"/test_dir"
@@ -44,7 +50,7 @@ struct event_t {
 };
 
 static int fanotify_fd;
-static char events_buf[BUF_SIZE];
+static char events_buf[EVENT_BUF_LEN];
 static struct event_t event_set[EVENT_MAX];
 
 static void do_test(void)
@@ -55,23 +61,24 @@ static void do_test(void)
 	struct fanotify_event_metadata *metadata;
 	struct fanotify_event_info_fid *event_fid;
 
+
 	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
-				FAN_CREATE | FAN_DELETE | FAN_ATTRIB |
-				FAN_MOVED_FROM | FAN_MOVED_TO |
-				FAN_DELETE_SELF | FAN_ONDIR,
+				FAN_CREATE | FAN_DELETE | FAN_MOVE |
+				FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR,
 				AT_FDCWD, TEST_DIR) == -1) {
 		if (errno == ENODEV)
 			tst_brk(TCONF,
 				"FAN_REPORT_FID not supported on %s "
 				"filesystem", tst_device->fs_type);
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark(%d, FAN_MARK_ADD, FAN_CREATE | "
-			"FAN_DELETE | FAN_MOVED_FROM | FAN_MOVED_TO | "
-			"FAN_DELETE_SELF | FAN_ONDIR, AT_FDCWD, %s) failed",
+			"fanotify_mark(%d, FAN_MARK_ADD | FAN_MARK_FILESYSTEM, "
+			"FAN_CREATE | FAN_DELETE | FAN_MOVE | "
+			"FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR, "
+			"AT_FDCWD, %s) failed",
 			fanotify_fd, TEST_DIR);
 	}
 
-	/* Generate a sequence of events */
+	/* All dirent events on testdir are merged */
 	event_set[count].mask = FAN_CREATE | FAN_MOVED_FROM | FAN_MOVED_TO | \
 				FAN_DELETE;
 	event_set[count].handle.handle_bytes = MAX_HANDLE_SZ;
@@ -82,9 +89,22 @@ static void do_test(void)
 	fd = SAFE_CREAT(FILE1, 0644);
 	SAFE_CLOSE(fd);
 
+	/*
+	 * Event on child file is not merged with dirent events.
+	 */
+	event_set[count].mask = FAN_MODIFY;
+	event_set[count].handle.handle_bytes = MAX_HANDLE_SZ;
+	fanotify_get_fid(FILE1, &event_set[count].fsid,
+			 &event_set[count].handle);
+	count++;
+
+	SAFE_TRUNCATE(FILE1, 1);
 	SAFE_RENAME(FILE1, FILE2);
 
-	event_set[count].mask = FAN_ATTRIB | FAN_DELETE_SELF;
+	/*
+	 * FAN_DELETE_SELF may be merged with FAN_MODIFY event above.
+	 */
+	event_set[count].mask = FAN_DELETE_SELF;
 	event_set[count].handle.handle_bytes = MAX_HANDLE_SZ;
 	fanotify_get_fid(FILE2, &event_set[count].fsid,
 			 &event_set[count].handle);
@@ -92,6 +112,9 @@ static void do_test(void)
 
 	SAFE_UNLINK(FILE2);
 
+	/* Read file events from the event queue */
+	len = SAFE_READ(0, fanotify_fd, events_buf, EVENT_BUF_LEN);
+
 	/*
 	 * Generate a sequence of events on a directory. Subsequent events
 	 * are merged, so it's required that we set FAN_ONDIR once in
@@ -118,13 +141,12 @@ static void do_test(void)
 
 	SAFE_RMDIR(DIR2);
 
-	/* Read events from the event queue */
-	len = SAFE_READ(0, fanotify_fd, events_buf, BUF_SIZE);
+	/* Read dir events from the event queue */
+	len += SAFE_READ(0, fanotify_fd, events_buf + len, EVENT_BUF_LEN - len);
 
 	/* Process each event in buffer */
 	for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
-		FAN_EVENT_OK(metadata, len);
-		metadata = FAN_EVENT_NEXT(metadata,len), i++) {
+		FAN_EVENT_OK(metadata, len); i++) {
 		event_fid = (struct fanotify_event_info_fid *) (metadata + 1);
 		event_file_handle = (struct file_handle *) event_fid->handle;
 
@@ -141,7 +163,7 @@ static void do_test(void)
 				"Received unexpected file descriptor %d in "
 				"event. Expected to get FAN_NOFD(%d)",
 				metadata->fd, FAN_NOFD);
-		} else if (metadata->mask != event_set[i].mask) {
+		} else if (!(metadata->mask & event_set[i].mask)) {
 			tst_res(TFAIL,
 				"Got event: mask=%llx (expected %llx) "
 				"pid=%u fd=%d",
@@ -197,6 +219,10 @@ static void do_test(void)
 				*(unsigned long *)
 				event_file_handle->f_handle);
 		}
+		metadata->mask  &= ~event_set[i].mask;
+		/* No events left in current mask? Go for next event */
+		if (metadata->mask == 0)
+			metadata = FAN_EVENT_NEXT(metadata, len);
 	}
 
 	for (; i < count; i++)
-- 
2.17.1

[LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections

[Petr Vorel]

Hi Amir,

> - Fix calculation of events buffer size
> - Read file events and dir events in two batches
> - Generate FAN_MODIFY event explicitly with truncate() operation
>   instead of FAN_ATTRIB event implicitly with create() operation
> - FAN_MODIFY and FAN_DELETE_SELF may or may not be merged

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Whole patchset LGTM, but I'd prefer Jan or somebody else reviewed it.

Kind regards,
Petr

[LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections

[Cyril Hrubis]

Hi!
Looks good to me as well.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections

[Matthew Bobrowski]

On Tue, Apr 21, 2020 at 09:50:00AM +0300, Amir Goldstein wrote:
>  static void do_test(void)
> @@ -55,23 +61,24 @@ static void do_test(void)
>  	struct fanotify_event_metadata *metadata;
>  	struct fanotify_event_info_fid *event_fid;
>  
> +

^ Unnecessary white line entered here?

>  	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
> -				FAN_CREATE | FAN_DELETE | FAN_ATTRIB |
> -				FAN_MOVED_FROM | FAN_MOVED_TO |
> -				FAN_DELETE_SELF | FAN_ONDIR,
> +				FAN_CREATE | FAN_DELETE | FAN_MOVE |
> +				FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR,
>  				AT_FDCWD, TEST_DIR) == -1) {

...

> -	/* Generate a sequence of events */
> +	/* All dirent events on testdir are merged */
>  	event_set[count].mask = FAN_CREATE | FAN_MOVED_FROM | FAN_MOVED_TO | \
>  				FAN_DELETE;

Just a suggestion, perhaps we can modify the above line to the following:

	event_set[count].mask = FAN_CREATE | FAN_MOVE | FAN_DELETE;

Also, I believe that we can generally replace all instances of
FAN_MOVED_FROM | FAN_MOVED_TO with FAN_MOVE within this file.

The rest looks good to me.

Reviewed-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>

/M

[LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks

[Amir Goldstein]

Test reporting events with fid also with recusrive inode marks:
- Test events "on self" (FAN_DELETE_SELF) on file and dir
- Test events "on child" (FAN_MODIFY) on file

With recursive inode marks, verify that the FAN_MODIFY event reported
to parent "on child" is merged with the FAN_MODIFY event reported to
child.

The new test case is a regression test for commit f367a62a7cad:

    fanotify: merge duplicate events on parent and child

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify15.c     | 76 +++++++++++++++++--
 1 file changed, 69 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index 454441bfe..bb1069139 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -9,6 +9,10 @@
  *	Test file that has been purposely designed to verify
  *	FAN_REPORT_FID functionality while using newly defined dirent
  *	events.
+ *
+ * Test case #1 is a regression test for commit f367a62a7cad:
+ *
+ *      fanotify: merge duplicate events on parent and child
  */
 #define _GNU_SOURCE
 #include "config.h"
@@ -53,29 +57,51 @@ static int fanotify_fd;
 static char events_buf[EVENT_BUF_LEN];
 static struct event_t event_set[EVENT_MAX];
 
-static void do_test(void)
+static struct test_case_t {
+	struct fanotify_mark_type mark;
+	unsigned long mask;
+} test_cases[] = {
+	{
+		/* Watch filesystem including events "on self" */
+		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+		FAN_DELETE_SELF,
+	},
+	{
+		/* Watch directory including events "on children" */
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_EVENT_ON_CHILD,
+	},
+};
+
+static void do_test(unsigned int number)
 {
 	int i, fd, len, count = 0;
 
 	struct file_handle *event_file_handle;
 	struct fanotify_event_metadata *metadata;
 	struct fanotify_event_info_fid *event_fid;
+	struct test_case_t *tc = &test_cases[number];
+	struct fanotify_mark_type *mark = &tc->mark;
 
+	tst_res(TINFO,
+		"Test #%d: FAN_REPORT_FID with mark type: %s",
+		number, mark->name);
 
-	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
+
+	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
 				FAN_CREATE | FAN_DELETE | FAN_MOVE |
-				FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR,
+				FAN_MODIFY | FAN_ONDIR,
 				AT_FDCWD, TEST_DIR) == -1) {
 		if (errno == ENODEV)
 			tst_brk(TCONF,
 				"FAN_REPORT_FID not supported on %s "
 				"filesystem", tst_device->fs_type);
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark(%d, FAN_MARK_ADD | FAN_MARK_FILESYSTEM, "
+			"fanotify_mark(%d, FAN_MARK_ADD | %s, "
 			"FAN_CREATE | FAN_DELETE | FAN_MOVE | "
-			"FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR, "
+			"FAN_MODIFY | FAN_ONDIR, "
 			"AT_FDCWD, %s) failed",
-			fanotify_fd, TEST_DIR);
+			fanotify_fd, mark->name, TEST_DIR);
 	}
 
 	/* All dirent events on testdir are merged */
@@ -89,8 +115,21 @@ static void do_test(void)
 	fd = SAFE_CREAT(FILE1, 0644);
 	SAFE_CLOSE(fd);
 
+	/* Recursive watch file for events "on self" */
+	if (mark->flag == FAN_MARK_INODE &&
+	    fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag,
+			  FAN_MODIFY | FAN_DELETE_SELF,
+			  AT_FDCWD, FILE1) == -1) {
+		tst_brk(TBROK | TERRNO,
+			"fanotify_mark(%d, FAN_MARK_ADD | %s, "
+			"FAN_DELETE_SELF, AT_FDCWD, %s) failed",
+			fanotify_fd, mark->name, FILE1);
+	}
+
 	/*
 	 * Event on child file is not merged with dirent events.
+	 * FAN_MODIFY event reported on file mark should be merged with the
+	 * FAN_MODIFY event reported on parent directory watch.
 	 */
 	event_set[count].mask = FAN_MODIFY;
 	event_set[count].handle.handle_bytes = MAX_HANDLE_SZ;
@@ -131,6 +170,17 @@ static void do_test(void)
 
 	SAFE_MKDIR(DIR1, 0755);
 
+	/* Recursive watch subdir for events "on self" */
+	if (mark->flag == FAN_MARK_INODE &&
+	    fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag,
+			  FAN_DELETE_SELF | FAN_ONDIR,
+			  AT_FDCWD, DIR1) == -1) {
+		tst_brk(TBROK | TERRNO,
+			"fanotify_mark(%d, FAN_MARK_ADD | %s,"
+			"FAN_DELETE_SELF | FAN_ONDIR, AT_FDCWD, %s) failed",
+			fanotify_fd, mark->name, DIR1);
+	}
+
 	SAFE_RENAME(DIR1, DIR2);
 
 	event_set[count].mask = FAN_ONDIR | FAN_DELETE_SELF;
@@ -144,6 +194,17 @@ static void do_test(void)
 	/* Read dir events from the event queue */
 	len += SAFE_READ(0, fanotify_fd, events_buf + len, EVENT_BUF_LEN - len);
 
+	/*
+	 * Cleanup the mark
+	 */
+	if (fanotify_mark(fanotify_fd, FAN_MARK_FLUSH | mark->flag, 0,
+			  AT_FDCWD, TEST_DIR) < 0) {
+		tst_brk(TBROK | TERRNO,
+			"fanotify_mark (%d, FAN_MARK_FLUSH | %s, 0,"
+			"AT_FDCWD, '"TEST_DIR"') failed",
+			fanotify_fd, mark->name);
+	}
+
 	/* Process each event in buffer */
 	for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
 		FAN_EVENT_OK(metadata, len); i++) {
@@ -262,7 +323,8 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MOUNT_POINT,
 	.all_filesystems = 1,
-	.test_all = do_test,
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(test_cases),
 	.setup = do_setup,
 	.cleanup = do_cleanup
 };
-- 
2.17.1

[LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks

[Petr Vorel]

Hi Amir,

>  	for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
>  		FAN_EVENT_OK(metadata, len); i++) {
> @@ -262,7 +323,8 @@ static struct tst_test test = {
>  	.mount_device = 1,
>  	.mntpoint = MOUNT_POINT,
>  	.all_filesystems = 1,
> -	.test_all = do_test,
> +	.test = do_test,
> +	.tcnt = ARRAY_SIZE(test_cases),
>  	.setup = do_setup,
>  	.cleanup = do_cleanup
Again, missing (can be added during merge):
-	.cleanup = do_cleanup
+	.cleanup = do_cleanup,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "f367a62a7cad"},
+		{}
>  };

Apart from already mentioned FSID_VAL_MEMBER() LGTM, but again, somebody more
experienced in fanotify and/or filesystems should look into it.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

[LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks

[Petr Vorel]

Hi Amir,

> Apart from already mentioned FSID_VAL_MEMBER() LGTM, but again, somebody more
I'm sorry, FSID_VAL_MEMBER() was meant to be for fanotify16.c.
> experienced in fanotify and/or filesystems should look into it.


Kind regards,
Petr

[LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks

[Cyril Hrubis]

Hi!
Looks good to me, minus the things pointed out by Peter.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks

[Matthew Bobrowski]

On Tue, Apr 21, 2020 at 09:50:01AM +0300, Amir Goldstein wrote:
> +	tst_res(TINFO,
> +		"Test #%d: FAN_REPORT_FID with mark type: %s",
> +		number, mark->name);
>  
> -	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
> +

A nit, but there's an unnecessary extra whiteline here.

> +	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
>  				FAN_CREATE | FAN_DELETE | FAN_MOVE |
> -				FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR,
> +				FAN_MODIFY | FAN_ONDIR,
>  				AT_FDCWD, TEST_DIR) == -1) {
>  		if (errno == ENODEV)
>  			tst_brk(TCONF,
>  				"FAN_REPORT_FID not supported on %s "
>  				"filesystem", tst_device->fs_type);
>  		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark(%d, FAN_MARK_ADD | FAN_MARK_FILESYSTEM, "
> +			"fanotify_mark(%d, FAN_MARK_ADD | %s, "
>  			"FAN_CREATE | FAN_DELETE | FAN_MOVE | "
> -			"FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR, "
> +			"FAN_MODIFY | FAN_ONDIR, "
>  			"AT_FDCWD, %s) failed",
> -			fanotify_fd, TEST_DIR);
> +			fanotify_fd, mark->name, TEST_DIR);

I see that you've removed the FAN_DELETE_SELF mask here, although
should we consider adding tc->mask here too for the sake of
correctness?

The rest looks fine to me.

Reviewed-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>

/M

[LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks

[Amir Goldstein]

On Sat, May 2, 2020 at 10:10 AM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> On Tue, Apr 21, 2020 at 09:50:01AM +0300, Amir Goldstein wrote:
> > +     tst_res(TINFO,
> > +             "Test #%d: FAN_REPORT_FID with mark type: %s",
> > +             number, mark->name);
> >
> > -     if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
> > +
>
> A nit, but there's an unnecessary extra whiteline here.
>
> > +     if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
> >                               FAN_CREATE | FAN_DELETE | FAN_MOVE |
> > -                             FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR,
> > +                             FAN_MODIFY | FAN_ONDIR,
> >                               AT_FDCWD, TEST_DIR) == -1) {
> >               if (errno == ENODEV)
> >                       tst_brk(TCONF,
> >                               "FAN_REPORT_FID not supported on %s "
> >                               "filesystem", tst_device->fs_type);
> >               tst_brk(TBROK | TERRNO,
> > -                     "fanotify_mark(%d, FAN_MARK_ADD | FAN_MARK_FILESYSTEM, "
> > +                     "fanotify_mark(%d, FAN_MARK_ADD | %s, "
> >                       "FAN_CREATE | FAN_DELETE | FAN_MOVE | "
> > -                     "FAN_MODIFY | FAN_DELETE_SELF | FAN_ONDIR, "
> > +                     "FAN_MODIFY | FAN_ONDIR, "
> >                       "AT_FDCWD, %s) failed",
> > -                     fanotify_fd, TEST_DIR);
> > +                     fanotify_fd, mark->name, TEST_DIR);
>
> I see that you've removed the FAN_DELETE_SELF mask here, although
> should we consider adding tc->mask here too for the sake of
> correctness?

Sure, I added " | 0x%x" for the extra tc->mask and also
enhanced the TINFO in the beginning of the test case to disaply
more explicit text like this:
               "FAN_REPORT_FID on filesystem including FAN_DELETE_SELF",
               "FAN_REPORT_FID on directory with FAN_EVENT_ON_CHILD",

Thanks,
Amir.

[LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR

[Amir Goldstein]

- Watch dir modify events with name info
- Watch self delete events w/o name info
- Test inode and filesystem marks
- Check getting events for all file and dir names

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/fanotify/.gitignore |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h |  15 +-
 .../kernel/syscalls/fanotify/fanotify16.c     | 441 ++++++++++++++++++
 4 files changed, 455 insertions(+), 3 deletions(-)
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify16.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 9bb72beb2..4c25c9cb9 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -571,6 +571,7 @@ fanotify12 fanotify12
 fanotify13 fanotify13
 fanotify14 fanotify14
 fanotify15 fanotify15
+fanotify16 fanotify16
 
 ioperm01 ioperm01
 ioperm02 ioperm02
diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
index 68e4cc7aa..e7cf224fd 100644
--- a/testcases/kernel/syscalls/fanotify/.gitignore
+++ b/testcases/kernel/syscalls/fanotify/.gitignore
@@ -13,4 +13,5 @@
 /fanotify13
 /fanotify14
 /fanotify15
+/fanotify16
 /fanotify_child
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 6da7e765c..a05f4a372 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -41,6 +41,9 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 #ifndef FAN_REPORT_TID
 #define FAN_REPORT_TID		0x00000100
 #endif
+#ifndef FAN_REPORT_FID
+#define FAN_REPORT_FID		0x00000200
+#endif
 
 #ifndef FAN_MARK_INODE
 #define FAN_MARK_INODE		0
@@ -79,9 +82,8 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 #ifndef FAN_OPEN_EXEC_PERM
 #define FAN_OPEN_EXEC_PERM	0x00040000
 #endif
-
-#ifndef FAN_REPORT_FID
-#define FAN_REPORT_FID		0x00000200
+#ifndef FAN_DIR_MODIFY
+#define FAN_DIR_MODIFY		0x00080000
 #endif
 
 /*
@@ -106,6 +108,13 @@ typedef struct {
 #define __kernel_fsid_t lapi_fsid_t
 #endif /* __kernel_fsid_t */
 
+#ifndef FAN_EVENT_INFO_TYPE_FID
+#define FAN_EVENT_INFO_TYPE_FID		1
+#endif
+#ifndef FAN_EVENT_INFO_TYPE_DFID_NAME
+#define FAN_EVENT_INFO_TYPE_DFID_NAME	2
+#endif
+
 #ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_HEADER
 struct fanotify_event_info_header {
 	uint8_t info_type;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
new file mode 100644
index 000000000..0ec151841
--- /dev/null
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -0,0 +1,441 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 CTERA Networks. All Rights Reserved.
+ *
+ * Started by Amir Goldstein <amir73il@gmail.com>
+ *
+ * DESCRIPTION
+ *     Check FAN_DIR_MODIFY events with name info
+ */
+#define _GNU_SOURCE
+#include "config.h"
+
+#include <stdio.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/syscall.h>
+#include "tst_test.h"
+#include "fanotify.h"
+
+#if defined(HAVE_SYS_FANOTIFY_H)
+#include <sys/fanotify.h>
+#include <sys/inotify.h>
+
+#define EVENT_MAX 10
+
+/* Size of the event structure, not including file handle */
+#define EVENT_SIZE (sizeof(struct fanotify_event_metadata) + \
+		    sizeof(struct fanotify_event_info_fid))
+/* Tripple events buffer size to account for file handles and names */
+#define EVENT_BUF_LEN (EVENT_MAX * EVENT_SIZE * 3)
+
+
+#define BUF_SIZE 256
+
+static char fname1[BUF_SIZE], fname2[BUF_SIZE];
+static char dname1[BUF_SIZE], dname2[BUF_SIZE];
+static int fd_notify;
+
+struct fid_t {
+	__kernel_fsid_t fsid;
+	struct file_handle handle;
+	char buf[MAX_HANDLE_SZ];
+};
+
+struct event_t {
+	unsigned long long mask;
+	struct fid_t *fid;
+	char name[BUF_SIZE];
+};
+
+static struct event_t event_set[EVENT_MAX];
+
+static char event_buf[EVENT_BUF_LEN];
+
+#define DIR_NAME1 "test_dir1"
+#define DIR_NAME2 "test_dir2"
+#define FILE_NAME1 "test_file1"
+#define FILE_NAME2 "test_file2"
+#define MOUNT_PATH "fs_mnt"
+
+static struct test_case_t {
+	struct fanotify_mark_type mark;
+	unsigned long mask;
+	struct fanotify_mark_type sub_mark;
+	unsigned long sub_mask;
+} test_cases[] = {
+	{
+		/* Filesystem watch for dir modify and delete self events */
+		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+		FAN_DIR_MODIFY | FAN_DELETE_SELF | FAN_ONDIR,
+		{},
+		0,
+	},
+	{
+		/* Recursive watches for dir modify events */
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_DIR_MODIFY,
+		/* Watches for delete self event on subdir */
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_DIR_MODIFY | FAN_DELETE_SELF | FAN_ONDIR,
+	},
+};
+
+void save_fid(const char *path, struct fid_t *fid)
+{
+	int *fh = (int *)(fid->handle.f_handle);
+	int *fsid = fid->fsid.val;
+
+	fh[0] = fh[1] = fh[2] = 0;
+	fid->handle.handle_bytes = MAX_HANDLE_SZ;
+	fanotify_get_fid(path, &fid->fsid, &fid->handle);
+
+	tst_res(TINFO,
+		"fid(%s) = %x.%x.%x.%x.%x...",
+		path, fsid[0], fsid[1], fh[0], fh[1], fh[2]);
+}
+
+static void do_test(unsigned int number)
+{
+	int len = 0, i = 0, test_num = 0;
+	int tst_count = 0;
+	int fd;
+	struct test_case_t *tc = &test_cases[number];
+	struct fanotify_mark_type *mark = &tc->mark;
+	struct fanotify_mark_type *sub_mark = &tc->sub_mark;
+	struct fid_t root_fid, dir_fid, file_fid;
+
+	tst_res(TINFO,
+		"Test #%d: FAN_REPORT_FID with mark type: %s",
+		number, mark->name);
+
+
+	fd_notify = fanotify_init(FAN_REPORT_FID, 0);
+	if (fd_notify == -1) {
+		if (errno == EINVAL) {
+			tst_brk(TCONF,
+				"FAN_REPORT_FID not supported by kernel");
+			return;
+		}
+		tst_brk(TBROK | TERRNO,
+			"fanotify_init(FAN_REPORT_FID, 0) failed");
+	}
+
+	/*
+	 * Watch dir modify events with name in filesystem/dir
+	 */
+	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
+			  AT_FDCWD, MOUNT_PATH) < 0) {
+		if (errno == EINVAL) {
+			tst_brk(TCONF,
+				"FAN_DIR_MODIFY not supported by kernel");
+			return;
+		}
+		tst_brk(TBROK | TERRNO,
+		    "fanotify_mark (%d, FAN_MARK_ADD | %s, "
+		    "FAN_DIR_MODIFY, AT_FDCWD, '"MOUNT_PATH"') "
+		    "failed", fd_notify, mark->name);
+	}
+
+	/* Save the mount root fid */
+	save_fid(MOUNT_PATH, &root_fid);
+
+	/*
+	 * Create subdir and watch open events "on children" with name.
+	 */
+	if (mkdir(dname1, 0755) < 0) {
+		tst_brk(TBROK | TERRNO,
+				"mkdir('"DIR_NAME1"', 0755) failed");
+	}
+
+	/* Save the subdir fid */
+	save_fid(dname1, &dir_fid);
+
+	if (tc->sub_mask &&
+	    fanotify_mark(fd_notify, FAN_MARK_ADD | sub_mark->flag, tc->sub_mask,
+			  AT_FDCWD, dname1) < 0) {
+		tst_brk(TBROK | TERRNO,
+		    "fanotify_mark (%d, FAN_MARK_ADD | %s, "
+		    "FAN_DIR_MODIFY | FAN_DELETE_SELF | FAN_ONDIR, "
+		    "AT_FDCWD, '%s') "
+		    "failed", fd_notify, sub_mark->name, dname1);
+	}
+
+	event_set[tst_count].mask = FAN_DIR_MODIFY;
+	event_set[tst_count].fid = &root_fid;
+	strcpy(event_set[tst_count].name, DIR_NAME1);
+	tst_count++;
+
+	/* Generate modify events "on child" */
+	if ((fd = creat(fname1, 0755)) == -1) {
+		tst_brk(TBROK | TERRNO,
+			"creat(\"%s\", 755) failed", FILE_NAME1);
+	}
+
+	/* Save the file fid */
+	save_fid(fname1, &file_fid);
+
+	SAFE_WRITE(1, fd, "1", 1);
+
+	if (rename(fname1, fname2) == -1) {
+		tst_brk(TBROK | TERRNO,
+				"rename(%s, %s) failed",
+				FILE_NAME1, FILE_NAME2);
+	}
+
+	if (close(fd) == -1) {
+		tst_brk(TBROK | TERRNO,
+				"close(%s) failed", FILE_NAME2);
+	}
+
+	/* Generate delete events with fname2 */
+	if (unlink(fname2) == -1) {
+		tst_brk(TBROK | TERRNO,
+				"unlink(%s) failed", FILE_NAME2);
+	}
+
+	/* Read events on files in subdir */
+	len += SAFE_READ(0, fd_notify, event_buf + len, EVENT_BUF_LEN - len);
+
+	/*
+	 * FAN_DIR_MODIFY events with the same name are merged.
+	 */
+	event_set[tst_count].mask = FAN_DIR_MODIFY;
+	event_set[tst_count].fid = &dir_fid;
+	strcpy(event_set[tst_count].name, FILE_NAME1);
+	tst_count++;
+	event_set[tst_count].mask = FAN_DIR_MODIFY;
+	event_set[tst_count].fid = &dir_fid;
+	strcpy(event_set[tst_count].name, FILE_NAME2);
+	tst_count++;
+	/*
+	 * Directory watch does not get self events on children.
+	 * Filesystem watch gets self event w/o name info.
+	 */
+	if (mark->flag == FAN_MARK_FILESYSTEM) {
+		event_set[tst_count].mask = FAN_DELETE_SELF;
+		event_set[tst_count].fid = &file_fid;
+		strcpy(event_set[tst_count].name, "");
+		tst_count++;
+	}
+
+	if (rename(dname1, dname2) == -1) {
+		tst_brk(TBROK | TERRNO,
+				"rename(%s, %s) failed",
+				DIR_NAME1, DIR_NAME2);
+	}
+
+	if (rmdir(dname2) == -1) {
+		tst_brk(TBROK | TERRNO,
+				"rmdir(%s) failed", DIR_NAME2);
+	}
+
+	/* Read more events on dirs */
+	len += SAFE_READ(0, fd_notify, event_buf + len, EVENT_BUF_LEN - len);
+
+	event_set[tst_count].mask = FAN_DIR_MODIFY;
+	event_set[tst_count].fid = &root_fid;
+	strcpy(event_set[tst_count].name, DIR_NAME1);
+	tst_count++;
+	event_set[tst_count].mask = FAN_DIR_MODIFY;
+	event_set[tst_count].fid = &root_fid;
+	strcpy(event_set[tst_count].name, DIR_NAME2);
+	tst_count++;
+	/*
+	 * Directory watch gets self event on itself w/o name info.
+	 */
+	event_set[tst_count].mask = FAN_DELETE_SELF | FAN_ONDIR;
+	strcpy(event_set[tst_count].name, "");
+	event_set[tst_count].fid = &dir_fid;
+	tst_count++;
+
+	/*
+	 * Cleanup the marks
+	 */
+	SAFE_CLOSE(fd_notify);
+	fd_notify = -1;
+
+	while (i < len) {
+		struct event_t *expected = &event_set[test_num];
+		struct fanotify_event_metadata *event;
+		struct fanotify_event_info_fid *event_fid;
+		struct file_handle *file_handle;
+		unsigned int fhlen;
+		const char *filename;
+		int namelen, info_type;
+
+		event = (struct fanotify_event_metadata *)&event_buf[i];
+		event_fid = (struct fanotify_event_info_fid *)(event + 1);
+		file_handle = (struct file_handle *)event_fid->handle;
+		fhlen = file_handle->handle_bytes;
+		filename = (char *)file_handle->f_handle + fhlen;
+		namelen = ((char *)event + event->event_len) - filename;
+		/* End of event could have name, zero padding, both or none */
+		if (namelen > 0) {
+			namelen = strlen(filename);
+		} else {
+			filename = "";
+			namelen = 0;
+		}
+		if (expected->name[0]) {
+			info_type = FAN_EVENT_INFO_TYPE_DFID_NAME;
+		} else {
+			info_type = FAN_EVENT_INFO_TYPE_FID;
+		}
+		if (test_num >= tst_count) {
+			tst_res(TFAIL,
+				"got unnecessary event: mask=%llx "
+				"pid=%u fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd, filename,
+				event->event_len, event_fid->hdr.info_type,
+				event_fid->hdr.len, fhlen);
+		} else if (!fhlen || namelen < 0) {
+			tst_res(TFAIL,
+				"got event without fid: mask=%llx pid=%u fd=%d, "
+				"len=%d info_type=%d info_len=%d fh_len=%d",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd,
+				event->event_len, event_fid->hdr.info_type,
+				event_fid->hdr.len, fhlen);
+		} else if (event->mask != expected->mask) {
+			tst_res(TFAIL,
+				"got event: mask=%llx (expected %llx) "
+				"pid=%u fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d",
+				(unsigned long long)event->mask, expected->mask,
+				(unsigned)event->pid, event->fd, filename,
+				event->event_len, event_fid->hdr.info_type,
+				event_fid->hdr.len, fhlen);
+		} else if (info_type != event_fid->hdr.info_type) {
+			tst_res(TFAIL,
+				"got event: mask=%llx pid=%u fd=%d, "
+				"len=%d info_type=%d expected(%d) info_len=%d fh_len=%d",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd,
+				event->event_len, event_fid->hdr.info_type,
+				info_type, event_fid->hdr.len, fhlen);
+		} else if (fhlen != expected->fid->handle.handle_bytes) {
+			tst_res(TFAIL,
+				"got event: mask=%llx pid=%u fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d expected(%d)"
+				"fh_type=%d",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd, filename,
+				event->event_len, info_type,
+				event_fid->hdr.len, fhlen,
+				expected->fid->handle.handle_bytes,
+				file_handle->handle_type);
+		} else if (file_handle->handle_type !=
+			   expected->fid->handle.handle_type) {
+			tst_res(TFAIL,
+				"got event: mask=%llx pid=%u fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d "
+				"fh_type=%d expected(%x)",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd, filename,
+				event->event_len, info_type,
+				event_fid->hdr.len, fhlen,
+				file_handle->handle_type,
+				expected->fid->handle.handle_type);
+		} else if (memcmp(file_handle->f_handle,
+				  expected->fid->handle.f_handle, fhlen)) {
+			tst_res(TFAIL,
+				"got event: mask=%llx pid=%u fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d "
+				"fh_type=%d unexpected file handle (%x...)",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd, filename,
+				event->event_len, info_type,
+				event_fid->hdr.len, fhlen,
+				file_handle->handle_type,
+				*(int *)(file_handle->f_handle));
+		} else if (memcmp(&event_fid->fsid, &expected->fid->fsid,
+				  sizeof(event_fid->fsid)) != 0) {
+			tst_res(TFAIL,
+				"got event: mask=%llx pid=%u fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d "
+				"fsid=%x.%x (expected %x.%x)",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd, filename,
+				event->event_len, info_type,
+				event_fid->hdr.len, fhlen,
+				event_fid->fsid.val[0], event_fid->fsid.val[1],
+				expected->fid->fsid.val[0],
+				expected->fid->fsid.val[1]);
+		} else if (strcmp(expected->name, filename)) {
+			tst_res(TFAIL,
+				"got event: mask=%llx "
+				"pid=%u fd=%d name='%s' expected('%s') "
+				"len=%d info_type=%d info_len=%d fh_len=%d",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd,
+				filename, expected->name,
+				event->event_len, event_fid->hdr.info_type,
+				event_fid->hdr.len, fhlen);
+		} else if (event->pid != getpid()) {
+			tst_res(TFAIL,
+				"got event: mask=%llx pid=%u "
+				"(expected %u) fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d",
+				(unsigned long long)event->mask,
+				(unsigned)event->pid,
+				(unsigned)getpid(),
+				event->fd, filename,
+				event->event_len, event_fid->hdr.info_type,
+				event_fid->hdr.len, fhlen);
+		} else {
+			tst_res(TPASS,
+				"got event #%d: mask=%llx pid=%u fd=%d name='%s' "
+				"len=%d info_type=%d info_len=%d fh_len=%d",
+				test_num, (unsigned long long)event->mask,
+				(unsigned)event->pid, event->fd, filename,
+				event->event_len, event_fid->hdr.info_type,
+				event_fid->hdr.len, fhlen);
+		}
+		i += event->event_len;
+		if (event->fd > 0)
+			SAFE_CLOSE(event->fd);
+		test_num++;
+	}
+	for (; test_num < tst_count; test_num++) {
+		tst_res(TFAIL, "didn't get event: mask=%llx, name='%s'",
+			 event_set[test_num].mask, event_set[test_num].name);
+
+	}
+}
+
+static void setup(void)
+{
+	sprintf(dname1, "%s/%s", MOUNT_PATH, DIR_NAME1);
+	sprintf(dname2, "%s/%s", MOUNT_PATH, DIR_NAME2);
+	sprintf(fname1, "%s/%s", dname1, FILE_NAME1);
+	sprintf(fname2, "%s/%s", dname1, FILE_NAME2);
+}
+
+static void cleanup(void)
+{
+	if (fd_notify > 0)
+		SAFE_CLOSE(fd_notify);
+}
+
+static struct tst_test test = {
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(test_cases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.mount_device = 1,
+	.mntpoint = MOUNT_PATH,
+	.all_filesystems = 1,
+	.needs_tmpdir = 1,
+	.needs_root = 1
+};
+
+#else
+	TST_TEST_TCONF("system doesn't have required fanotify support");
+#endif
-- 
2.17.1

[LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR

[Petr Vorel]

Hi Amir,

thank you for this patchset!

...
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c

...

> +		} else if (memcmp(&event_fid->fsid, &expected->fid->fsid,
> +				  sizeof(event_fid->fsid)) != 0) {
> +			tst_res(TFAIL,
> +				"got event: mask=%llx pid=%u fd=%d name='%s' "
> +				"len=%d info_type=%d info_len=%d fh_len=%d "
> +				"fsid=%x.%x (expected %x.%x)",
> +				(unsigned long long)event->mask,
> +				(unsigned)event->pid, event->fd, filename,
> +				event->event_len, info_type,
> +				event_fid->hdr.len, fhlen,
> +				event_fid->fsid.val[0], event_fid->fsid.val[1],

This needs to be:
+				FSID_VAL_MEMBER(event_fid->fsid, 0),
+				FSID_VAL_MEMBER(event_fid->fsid, 1),

FSID_VAL_MEMBER() is a wrapper struct fanotify_event_info_fid, needed to fix
build on musl (and it shouldn't be used for struct event_t).

https://travis-ci.org/github/pevik/ltp/jobs/680149701

Also I got problems on FUSE:
safe_macros.c:754: INFO: Trying FUSE...
tst_test.c:1244: INFO: Timeout per run is 0h 05m 00s
fanotify16.c:112: INFO: Test #0: FAN_REPORT_FID with mark type: FAN_MARK_FILESYSTEM
fanotify16.c:138: BROK: fanotify_mark (3, FAN_MARK_ADD | FAN_MARK_FILESYSTEM, FAN_DIR_MODIFY, AT_FDCWD, 'fs_mnt') failed: ENODEV (19)
tst_device.c:373: INFO: umount('fs_mnt') failed with EBUSY, try  1...
tst_device.c:377: INFO: Likely gvfsd-trash is probing newly mounted fs, kill it to speed up tests.

Skipping FUSE fixes it:
.dev_fs_flags = TST_FS_SKIP_FUSE,

Kind regards,
Petr

[LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR

[Petr Vorel]

Hi Amir,

...
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
...
> +	fd_notify = fanotify_init(FAN_REPORT_FID, 0);
> +	if (fd_notify == -1) {
> +		if (errno == EINVAL) {
> +			tst_brk(TCONF,
> +				"FAN_REPORT_FID not supported by kernel");
> +			return;
tst_brk() exits the test, so return is not needed.
> +		}
> +		tst_brk(TBROK | TERRNO,
> +			"fanotify_init(FAN_REPORT_FID, 0) failed");
> +	}
> +
> +	/*
> +	 * Watch dir modify events with name in filesystem/dir
> +	 */
> +	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
> +			  AT_FDCWD, MOUNT_PATH) < 0) {
> +		if (errno == EINVAL) {
> +			tst_brk(TCONF,
> +				"FAN_DIR_MODIFY not supported by kernel");
> +			return;
Also here.
> +		}
> +		tst_brk(TBROK | TERRNO,
> +		    "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> +		    "FAN_DIR_MODIFY, AT_FDCWD, '"MOUNT_PATH"') "
> +		    "failed", fd_notify, mark->name);
> +	}

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Suggesting these changes:

Kind regards,
Petr

diff --git testcases/kernel/syscalls/fanotify/fanotify16.c testcases/kernel/syscalls/fanotify/fanotify16.c
index 0ec151841..7c29d256a 100644
--- testcases/kernel/syscalls/fanotify/fanotify16.c
+++ testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -116,11 +116,10 @@ static void do_test(unsigned int number)
 
 	fd_notify = fanotify_init(FAN_REPORT_FID, 0);
 	if (fd_notify == -1) {
-		if (errno == EINVAL) {
+		if (errno == EINVAL)
 			tst_brk(TCONF,
 				"FAN_REPORT_FID not supported by kernel");
-			return;
-		}
+
 		tst_brk(TBROK | TERRNO,
 			"fanotify_init(FAN_REPORT_FID, 0) failed");
 	}
@@ -130,11 +129,10 @@ static void do_test(unsigned int number)
 	 */
 	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
 			  AT_FDCWD, MOUNT_PATH) < 0) {
-		if (errno == EINVAL) {
+		if (errno == EINVAL)
 			tst_brk(TCONF,
 				"FAN_DIR_MODIFY not supported by kernel");
-			return;
-		}
+
 		tst_brk(TBROK | TERRNO,
 		    "fanotify_mark (%d, FAN_MARK_ADD | %s, "
 		    "FAN_DIR_MODIFY, AT_FDCWD, '"MOUNT_PATH"') "
@@ -365,7 +363,8 @@ static void do_test(unsigned int number)
 				(unsigned)event->pid, event->fd, filename,
 				event->event_len, info_type,
 				event_fid->hdr.len, fhlen,
-				event_fid->fsid.val[0], event_fid->fsid.val[1],
+				FSID_VAL_MEMBER(event_fid->fsid, 0),
+				FSID_VAL_MEMBER(event_fid->fsid, 1),
 				expected->fid->fsid.val[0],
 				expected->fid->fsid.val[1]);
 		} else if (strcmp(expected->name, filename)) {
@@ -427,6 +426,7 @@ static void cleanup(void)
 static struct tst_test test = {
 	.test = do_test,
 	.tcnt = ARRAY_SIZE(test_cases),
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
 	.setup = setup,
 	.cleanup = cleanup,
 	.mount_device = 1,

[LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR

[Amir Goldstein]

On Tue, Apr 28, 2020 at 12:22 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> ...
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
> ...
> > +     fd_notify = fanotify_init(FAN_REPORT_FID, 0);
> > +     if (fd_notify == -1) {
> > +             if (errno == EINVAL) {
> > +                     tst_brk(TCONF,
> > +                             "FAN_REPORT_FID not supported by kernel");
> > +                     return;
> tst_brk() exits the test, so return is not needed.
> > +             }
> > +             tst_brk(TBROK | TERRNO,
> > +                     "fanotify_init(FAN_REPORT_FID, 0) failed");
> > +     }
> > +
> > +     /*
> > +      * Watch dir modify events with name in filesystem/dir
> > +      */
> > +     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
> > +                       AT_FDCWD, MOUNT_PATH) < 0) {
> > +             if (errno == EINVAL) {
> > +                     tst_brk(TCONF,
> > +                             "FAN_DIR_MODIFY not supported by kernel");
> > +                     return;
> Also here.
> > +             }
> > +             tst_brk(TBROK | TERRNO,
> > +                 "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> > +                 "FAN_DIR_MODIFY, AT_FDCWD, '"MOUNT_PATH"') "
> > +                 "failed", fd_notify, mark->name);
> > +     }
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Suggesting these changes:

Hi Petr,

Those changes are fine by me.

Thanks,
Amir.

[LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR

[Cyril Hrubis]

Hi!
> +	/*
> +	 * Create subdir and watch open events "on children" with name.
> +	 */
> +	if (mkdir(dname1, 0755) < 0) {
> +		tst_brk(TBROK | TERRNO,
> +				"mkdir('"DIR_NAME1"', 0755) failed");
> +	}

The rest of the tests are using SAFE_ macros to generate events, which
is basically the same these snippets do, but the code is a bit shorter.

Is there a reason not to use them in this test?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR

[Matthew Bobrowski]

On Tue, Apr 21, 2020 at 09:50:02AM +0300, Amir Goldstein wrote:
> +void save_fid(const char *path, struct fid_t *fid)
> +{
> +	int *fh = (int *)(fid->handle.f_handle);
> +	int *fsid = fid->fsid.val;
> +
> +	fh[0] = fh[1] = fh[2] = 0;
> +	fid->handle.handle_bytes = MAX_HANDLE_SZ;
> +	fanotify_get_fid(path, &fid->fsid, &fid->handle);
> +
> +	tst_res(TINFO,
> +		"fid(%s) = %x.%x.%x.%x.%x...",
> +		path, fsid[0], fsid[1], fh[0], fh[1], fh[2]);
> +}

What do you think about pulling this out and shoving it in fanotify.h
as another helper? Perhaps future tests would/could also make use of
this routine.

> +static void do_test(unsigned int number)
> +{
> +	int len = 0, i = 0, test_num = 0;
> +	int tst_count = 0;
> +	int fd;

Just shove all these on one line?

> +	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
> +			  AT_FDCWD, MOUNT_PATH) < 0) {
> +		if (errno == EINVAL) {
> +			tst_brk(TCONF,
> +				"FAN_DIR_MODIFY not supported by kernel");
> +			return;
> +		}
> +		tst_brk(TBROK | TERRNO,
> +		    "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> +		    "FAN_DIR_MODIFY, AT_FDCWD, '"MOUNT_PATH"') "
> +		    "failed", fd_notify, mark->name);

Should we be adding tc->mask here to the format string output?

> +	/*
> +	 * Create subdir and watch open events "on children" with name.
> +	 */
> +	if (mkdir(dname1, 0755) < 0) {
> +		tst_brk(TBROK | TERRNO,
> +				"mkdir('"DIR_NAME1"', 0755) failed");
> +	}

Perhaps we should be making use of the SAFE_MACROS() so that we're
adhering to the test writing guidelines?

> +	if (tc->sub_mask &&
> +	    fanotify_mark(fd_notify, FAN_MARK_ADD | sub_mark->flag, tc->sub_mask,
> +			  AT_FDCWD, dname1) < 0) {
> +		tst_brk(TBROK | TERRNO,
> +		    "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> +		    "FAN_DIR_MODIFY | FAN_DELETE_SELF | FAN_ONDIR, "
> +		    "AT_FDCWD, '%s') "
> +		    "failed", fd_notify, sub_mark->name, dname1);
> +	}

Maybe just replace the statically typed mask here with tc->sub_mask?
That way, if test cases are added or modified in the future, you don't
have to update it?

> +	if ((fd = creat(fname1, 0755)) == -1) {
> +		tst_brk(TBROK | TERRNO,
> +			"creat(\"%s\", 755) failed", FILE_NAME1);
> +	}
> +
> +	if (rename(fname1, fname2) == -1) {
> +		tst_brk(TBROK | TERRNO,
> +				"rename(%s, %s) failed",
> +				FILE_NAME1, FILE_NAME2);
> +	}
> +
> +	if (close(fd) == -1) {
> +		tst_brk(TBROK | TERRNO,
> +				"close(%s) failed", FILE_NAME2);
> +	}
> +
> +	/* Generate delete events with fname2 */
> +	if (unlink(fname2) == -1) {
> +		tst_brk(TBROK | TERRNO,
> +				"unlink(%s) failed", FILE_NAME2);
> +	}

The same applies with the above set of system calls? 

...

> +	if (rename(dname1, dname2) == -1) {
> +		tst_brk(TBROK | TERRNO,
> +				"rename(%s, %s) failed",
> +				DIR_NAME1, DIR_NAME2);
> +	}
> +
> +	if (rmdir(dname2) == -1) {
> +		tst_brk(TBROK | TERRNO,
> +				"rmdir(%s) failed", DIR_NAME2);
> +	}


And here...

> +	while (i < len) {
> +		struct event_t *expected = &event_set[test_num];
> +		struct fanotify_event_metadata *event;
> +		struct fanotify_event_info_fid *event_fid;
> +		struct file_handle *file_handle;
> +		unsigned int fhlen;
> +		const char *filename;
> +		int namelen, info_type;
> +
> +		event = (struct fanotify_event_metadata *)&event_buf[i];
> +		event_fid = (struct fanotify_event_info_fid *)(event + 1);
> +		file_handle = (struct file_handle *)event_fid->handle;
> +		fhlen = file_handle->handle_bytes;
> +		filename = (char *)file_handle->f_handle + fhlen;
> +		namelen = ((char *)event + event->event_len) - filename;
> +		/* End of event could have name, zero padding, both or none */
> +		if (namelen > 0) {
> +			namelen = strlen(filename);
> +		} else {
> +			filename = "";
> +			namelen = 0;
> +		}
> +		if (expected->name[0]) {
> +			info_type = FAN_EVENT_INFO_TYPE_DFID_NAME;
> +		} else {
> +			info_type = FAN_EVENT_INFO_TYPE_FID;
> +		}

Can we line break these conditional statements?

...

> +static void setup(void)
> +{

	int fd;

	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, 0_RDONLY);
	SAFE_CLOSE(fd);

Above snippet missing from test bootstrap? I remember we had to add
this in the past, but I can't remember the _why_?

Anyway, the functionality testing looks fine to me.

Reviewed-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>

/M

[LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR

[Amir Goldstein]

On Sat, May 2, 2020 at 12:39 PM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> On Tue, Apr 21, 2020 at 09:50:02AM +0300, Amir Goldstein wrote:
> > +void save_fid(const char *path, struct fid_t *fid)
> > +{
> > +     int *fh = (int *)(fid->handle.f_handle);
> > +     int *fsid = fid->fsid.val;
> > +
> > +     fh[0] = fh[1] = fh[2] = 0;
> > +     fid->handle.handle_bytes = MAX_HANDLE_SZ;
> > +     fanotify_get_fid(path, &fid->fsid, &fid->handle);
> > +
> > +     tst_res(TINFO,
> > +             "fid(%s) = %x.%x.%x.%x.%x...",
> > +             path, fsid[0], fsid[1], fh[0], fh[1], fh[2]);
> > +}
>
> What do you think about pulling this out and shoving it in fanotify.h
> as another helper? Perhaps future tests would/could also make use of
> this routine.
>

Ok. And I'll convert fanotify15/fanotify13 to use this helper in another patch.

> > +static void do_test(unsigned int number)
> > +{
> > +     int len = 0, i = 0, test_num = 0;
> > +     int tst_count = 0;
> > +     int fd;
>
> Just shove all these on one line?

ok.

>
> > +     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
> > +                       AT_FDCWD, MOUNT_PATH) < 0) {
> > +             if (errno == EINVAL) {
> > +                     tst_brk(TCONF,
> > +                             "FAN_DIR_MODIFY not supported by kernel");
> > +                     return;
> > +             }
> > +             tst_brk(TBROK | TERRNO,
> > +                 "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> > +                 "FAN_DIR_MODIFY, AT_FDCWD, '"MOUNT_PATH"') "
> > +                 "failed", fd_notify, mark->name);
>
> Should we be adding tc->mask here to the format string output?

Ok.

>
> > +     /*
> > +      * Create subdir and watch open events "on children" with name.
> > +      */
> > +     if (mkdir(dname1, 0755) < 0) {
> > +             tst_brk(TBROK | TERRNO,
> > +                             "mkdir('"DIR_NAME1"', 0755) failed");
> > +     }
>
> Perhaps we should be making use of the SAFE_MACROS() so that we're
> adhering to the test writing guidelines?
>

Of course.

> > +     if (tc->sub_mask &&
> > +         fanotify_mark(fd_notify, FAN_MARK_ADD | sub_mark->flag, tc->sub_mask,
> > +                       AT_FDCWD, dname1) < 0) {
> > +             tst_brk(TBROK | TERRNO,
> > +                 "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> > +                 "FAN_DIR_MODIFY | FAN_DELETE_SELF | FAN_ONDIR, "
> > +                 "AT_FDCWD, '%s') "
> > +                 "failed", fd_notify, sub_mark->name, dname1);
> > +     }
>
> Maybe just replace the statically typed mask here with tc->sub_mask?
> That way, if test cases are added or modified in the future, you don't
> have to update it?
>

Sure.

> > +     if ((fd = creat(fname1, 0755)) == -1) {
> > +             tst_brk(TBROK | TERRNO,
> > +                     "creat(\"%s\", 755) failed", FILE_NAME1);
> > +     }
> > +
> > +     if (rename(fname1, fname2) == -1) {
> > +             tst_brk(TBROK | TERRNO,
> > +                             "rename(%s, %s) failed",
> > +                             FILE_NAME1, FILE_NAME2);
> > +     }
> > +
> > +     if (close(fd) == -1) {
> > +             tst_brk(TBROK | TERRNO,
> > +                             "close(%s) failed", FILE_NAME2);
> > +     }
> > +
> > +     /* Generate delete events with fname2 */
> > +     if (unlink(fname2) == -1) {
> > +             tst_brk(TBROK | TERRNO,
> > +                             "unlink(%s) failed", FILE_NAME2);
> > +     }
>
> The same applies with the above set of system calls?
>
> ...
>
> > +     if (rename(dname1, dname2) == -1) {
> > +             tst_brk(TBROK | TERRNO,
> > +                             "rename(%s, %s) failed",
> > +                             DIR_NAME1, DIR_NAME2);
> > +     }
> > +
> > +     if (rmdir(dname2) == -1) {
> > +             tst_brk(TBROK | TERRNO,
> > +                             "rmdir(%s) failed", DIR_NAME2);
> > +     }
>
>
> And here...
>
> > +     while (i < len) {
> > +             struct event_t *expected = &event_set[test_num];
> > +             struct fanotify_event_metadata *event;
> > +             struct fanotify_event_info_fid *event_fid;
> > +             struct file_handle *file_handle;
> > +             unsigned int fhlen;
> > +             const char *filename;
> > +             int namelen, info_type;
> > +
> > +             event = (struct fanotify_event_metadata *)&event_buf[i];
> > +             event_fid = (struct fanotify_event_info_fid *)(event + 1);
> > +             file_handle = (struct file_handle *)event_fid->handle;
> > +             fhlen = file_handle->handle_bytes;
> > +             filename = (char *)file_handle->f_handle + fhlen;
> > +             namelen = ((char *)event + event->event_len) - filename;
> > +             /* End of event could have name, zero padding, both or none */
> > +             if (namelen > 0) {
> > +                     namelen = strlen(filename);
> > +             } else {
> > +                     filename = "";
> > +                     namelen = 0;
> > +             }
> > +             if (expected->name[0]) {
> > +                     info_type = FAN_EVENT_INFO_TYPE_DFID_NAME;
> > +             } else {
> > +                     info_type = FAN_EVENT_INFO_TYPE_FID;
> > +             }
>
> Can we line break these conditional statements?
>

ok.

> ...
>
> > +static void setup(void)
> > +{
>
>         int fd;
>
>         fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, 0_RDONLY);
>         SAFE_CLOSE(fd);
>
> Above snippet missing from test bootstrap? I remember we had to add
> this in the past, but I can't remember the _why_?
>

Because, if kernel does not support fanotify, we want the test
to fail on setup with "fanotify is not configured in this kernel."
instead of inaccurately reporting later:
"FAN_REPORT_FID not supported by kernel".

Thanks a lot for the review,
Amir.