* [LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1
@ 2020-04-21 6:49 Amir Goldstein
2020-04-21 6:49 ` [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories Amir Goldstein
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-04-21 6:49 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories
2020-04-21 6:49 [LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1 Amir Goldstein
@ 2020-04-21 6:49 ` Amir Goldstein
2020-04-27 17:27 ` Petr Vorel
2020-05-01 7:17 ` Matthew Bobrowski
2020-04-21 6:50 ` [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections Amir Goldstein
` (2 subsequent siblings)
3 siblings, 2 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-04-21 6:49 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections
2020-04-21 6:49 [LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1 Amir Goldstein
2020-04-21 6:49 ` [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories Amir Goldstein
@ 2020-04-21 6:50 ` Amir Goldstein
2020-04-27 19:30 ` Petr Vorel
` (2 more replies)
2020-04-21 6:50 ` [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks Amir Goldstein
2020-04-21 6:50 ` [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR Amir Goldstein
3 siblings, 3 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-04-21 6:50 UTC (permalink / raw)
To: ltp
- 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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks
2020-04-21 6:49 [LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1 Amir Goldstein
2020-04-21 6:49 ` [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories Amir Goldstein
2020-04-21 6:50 ` [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections Amir Goldstein
@ 2020-04-21 6:50 ` Amir Goldstein
2020-04-27 19:43 ` Petr Vorel
` (2 more replies)
2020-04-21 6:50 ` [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR Amir Goldstein
3 siblings, 3 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-04-21 6:50 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR
2020-04-21 6:49 [LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1 Amir Goldstein
` (2 preceding siblings ...)
2020-04-21 6:50 ` [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks Amir Goldstein
@ 2020-04-21 6:50 ` Amir Goldstein
2020-04-27 16:49 ` Petr Vorel
` (3 more replies)
3 siblings, 4 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-04-21 6:50 UTC (permalink / raw)
To: ltp
- 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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR
2020-04-21 6:50 ` [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR Amir Goldstein
@ 2020-04-27 16:49 ` Petr Vorel
2020-04-28 9:22 ` Petr Vorel
` (2 subsequent siblings)
3 siblings, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-04-27 16:49 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories
2020-04-21 6:49 ` [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories Amir Goldstein
@ 2020-04-27 17:27 ` Petr Vorel
2020-05-01 7:17 ` Matthew Bobrowski
1 sibling, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-04-27 17:27 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections
2020-04-21 6:50 ` [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections Amir Goldstein
@ 2020-04-27 19:30 ` Petr Vorel
2020-04-29 15:08 ` Cyril Hrubis
2020-05-01 8:09 ` Matthew Bobrowski
2 siblings, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-04-27 19:30 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks
2020-04-21 6:50 ` [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks Amir Goldstein
@ 2020-04-27 19:43 ` Petr Vorel
2020-04-28 9:20 ` Petr Vorel
2020-04-29 15:28 ` Cyril Hrubis
2020-05-02 7:09 ` Matthew Bobrowski
2 siblings, 1 reply; 23+ messages in thread
From: Petr Vorel @ 2020-04-27 19:43 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks
2020-04-27 19:43 ` Petr Vorel
@ 2020-04-28 9:20 ` Petr Vorel
0 siblings, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-04-28 9:20 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR
2020-04-21 6:50 ` [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR Amir Goldstein
2020-04-27 16:49 ` Petr Vorel
@ 2020-04-28 9:22 ` Petr Vorel
2020-04-28 9:51 ` Amir Goldstein
2020-04-29 16:02 ` Cyril Hrubis
2020-05-02 9:39 ` Matthew Bobrowski
3 siblings, 1 reply; 23+ messages in thread
From: Petr Vorel @ 2020-04-28 9:22 UTC (permalink / raw)
To: ltp
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,
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR
2020-04-28 9:22 ` Petr Vorel
@ 2020-04-28 9:51 ` Amir Goldstein
0 siblings, 0 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-04-28 9:51 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections
2020-04-21 6:50 ` [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections Amir Goldstein
2020-04-27 19:30 ` Petr Vorel
@ 2020-04-29 15:08 ` Cyril Hrubis
2020-05-01 8:09 ` Matthew Bobrowski
2 siblings, 0 replies; 23+ messages in thread
From: Cyril Hrubis @ 2020-04-29 15:08 UTC (permalink / raw)
To: ltp
Hi!
Looks good to me as well.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks
2020-04-21 6:50 ` [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks Amir Goldstein
2020-04-27 19:43 ` Petr Vorel
@ 2020-04-29 15:28 ` Cyril Hrubis
2020-05-02 7:09 ` Matthew Bobrowski
2 siblings, 0 replies; 23+ messages in thread
From: Cyril Hrubis @ 2020-04-29 15:28 UTC (permalink / raw)
To: ltp
Hi!
Looks good to me, minus the things pointed out by Peter.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR
2020-04-21 6:50 ` [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR Amir Goldstein
2020-04-27 16:49 ` Petr Vorel
2020-04-28 9:22 ` Petr Vorel
@ 2020-04-29 16:02 ` Cyril Hrubis
2020-05-02 9:39 ` Matthew Bobrowski
3 siblings, 0 replies; 23+ messages in thread
From: Cyril Hrubis @ 2020-04-29 16:02 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories
2020-04-21 6:49 ` [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories Amir Goldstein
2020-04-27 17:27 ` Petr Vorel
@ 2020-05-01 7:17 ` Matthew Bobrowski
2020-05-01 9:05 ` Amir Goldstein
1 sibling, 1 reply; 23+ messages in thread
From: Matthew Bobrowski @ 2020-05-01 7:17 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections
2020-04-21 6:50 ` [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections Amir Goldstein
2020-04-27 19:30 ` Petr Vorel
2020-04-29 15:08 ` Cyril Hrubis
@ 2020-05-01 8:09 ` Matthew Bobrowski
2 siblings, 0 replies; 23+ messages in thread
From: Matthew Bobrowski @ 2020-05-01 8:09 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories
2020-05-01 7:17 ` Matthew Bobrowski
@ 2020-05-01 9:05 ` Amir Goldstein
2020-05-01 9:46 ` Matthew Bobrowski
0 siblings, 1 reply; 23+ messages in thread
From: Amir Goldstein @ 2020-05-01 9:05 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories
2020-05-01 9:05 ` Amir Goldstein
@ 2020-05-01 9:46 ` Matthew Bobrowski
0 siblings, 0 replies; 23+ messages in thread
From: Matthew Bobrowski @ 2020-05-01 9:46 UTC (permalink / raw)
To: ltp
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>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks
2020-04-21 6:50 ` [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks Amir Goldstein
2020-04-27 19:43 ` Petr Vorel
2020-04-29 15:28 ` Cyril Hrubis
@ 2020-05-02 7:09 ` Matthew Bobrowski
2020-05-02 13:17 ` Amir Goldstein
2 siblings, 1 reply; 23+ messages in thread
From: Matthew Bobrowski @ 2020-05-02 7:09 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR
2020-04-21 6:50 ` [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR Amir Goldstein
` (2 preceding siblings ...)
2020-04-29 16:02 ` Cyril Hrubis
@ 2020-05-02 9:39 ` Matthew Bobrowski
2020-05-02 14:58 ` Amir Goldstein
3 siblings, 1 reply; 23+ messages in thread
From: Matthew Bobrowski @ 2020-05-02 9:39 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks
2020-05-02 7:09 ` Matthew Bobrowski
@ 2020-05-02 13:17 ` Amir Goldstein
0 siblings, 0 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-05-02 13:17 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR
2020-05-02 9:39 ` Matthew Bobrowski
@ 2020-05-02 14:58 ` Amir Goldstein
0 siblings, 0 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-05-02 14:58 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-05-02 14:58 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 6:49 [LTP] [PATCH 0/4] fanotify ltp tests for v5.7-rc1 Amir Goldstein
2020-04-21 6:49 ` [LTP] [PATCH 1/4] syscalls/fanotify09: Check merging of events on directories Amir Goldstein
2020-04-27 17:27 ` Petr Vorel
2020-05-01 7:17 ` Matthew Bobrowski
2020-05-01 9:05 ` Amir Goldstein
2020-05-01 9:46 ` Matthew Bobrowski
2020-04-21 6:50 ` [LTP] [PATCH 2/4] syscalls/fanotify15: Minor corrections Amir Goldstein
2020-04-27 19:30 ` Petr Vorel
2020-04-29 15:08 ` Cyril Hrubis
2020-05-01 8:09 ` Matthew Bobrowski
2020-04-21 6:50 ` [LTP] [PATCH 3/4] syscalls/fanotify15: Add a test case for inode marks Amir Goldstein
2020-04-27 19:43 ` Petr Vorel
2020-04-28 9:20 ` Petr Vorel
2020-04-29 15:28 ` Cyril Hrubis
2020-05-02 7:09 ` Matthew Bobrowski
2020-05-02 13:17 ` Amir Goldstein
2020-04-21 6:50 ` [LTP] [PATCH 4/4] syscalls/fanotify: New test for FAN_MODIFY_DIR Amir Goldstein
2020-04-27 16:49 ` Petr Vorel
2020-04-28 9:22 ` Petr Vorel
2020-04-28 9:51 ` Amir Goldstein
2020-04-29 16:02 ` Cyril Hrubis
2020-05-02 9:39 ` Matthew Bobrowski
2020-05-02 14:58 ` Amir Goldstein
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.