diff mbox series

[1/3] fanotify13: Test watching overlayfs upper fs

Message ID 20230903111558.2603332-2-amir73il@gmail.com
State Accepted
Headers show
Series Tests for fanotify and overlayfs | expand

Commit Message

Amir Goldstein Sept. 3, 2023, 11:15 a.m. UTC
Run a test variant with overlayfs (over all supported fs)
when watching the upper fs.

This is a regression test for kernel fix bc2473c90fca
("ovl: enable fsnotify events on underlying real files"),
from kernel 6.5, which is not likely to be backported to older kernels.

To avoid waiting for events that won't arrive when testing old kernels,
require that kernel supports encoding fid with new flag AT_HADNLE_FID,
also merged to 6.5 and not likely to be backported to older kernels.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 lib/tst_fs_setup.c                            |  2 +-
 testcases/kernel/syscalls/fanotify/fanotify.h | 21 +++++++
 .../kernel/syscalls/fanotify/fanotify13.c     | 62 +++++++++++++++++--
 3 files changed, 79 insertions(+), 6 deletions(-)

Comments

Richard Palethorpe Sept. 14, 2023, 10:32 a.m. UTC | #1
Hello Amir,

Amir Goldstein <amir73il@gmail.com> writes:

> Run a test variant with overlayfs (over all supported fs)
> when watching the upper fs.
>
> This is a regression test for kernel fix bc2473c90fca
> ("ovl: enable fsnotify events on underlying real files"),
> from kernel 6.5, which is not likely to be backported to older kernels.
>
> To avoid waiting for events that won't arrive when testing old kernels,
> require that kernel supports encoding fid with new flag AT_HADNLE_FID,
> also merged to 6.5 and not likely to be backported to older kernels.

Unfortunately Petr's not here at the moment.

I guess this first patch doesn't require 6.6? So it could be merged
independently without further considerations for what makes it into 6.6?
Amir Goldstein Sept. 14, 2023, 1:24 p.m. UTC | #2
On Thu, Sep 14, 2023 at 1:37 PM Richard Palethorpe <rpalethorpe@suse.de> wrote:
>
> Hello Amir,
>
> Amir Goldstein <amir73il@gmail.com> writes:
>
> > Run a test variant with overlayfs (over all supported fs)
> > when watching the upper fs.
> >
> > This is a regression test for kernel fix bc2473c90fca
> > ("ovl: enable fsnotify events on underlying real files"),
> > from kernel 6.5, which is not likely to be backported to older kernels.
> >
> > To avoid waiting for events that won't arrive when testing old kernels,
> > require that kernel supports encoding fid with new flag AT_HADNLE_FID,
> > also merged to 6.5 and not likely to be backported to older kernels.
>
> Unfortunately Petr's not here at the moment.
>
> I guess this first patch doesn't require 6.6? So it could be merged
> independently without further considerations for what makes it into 6.6?
>

Yes that is correct.

Thanks,
Amir.
Petr Vorel Sept. 15, 2023, 6:51 a.m. UTC | #3
Hi Amir, Richie,

> On Thu, Sep 14, 2023 at 1:37 PM Richard Palethorpe <rpalethorpe@suse.de> wrote:

> > Hello Amir,

> > Amir Goldstein <amir73il@gmail.com> writes:

> > > Run a test variant with overlayfs (over all supported fs)
> > > when watching the upper fs.

> > > This is a regression test for kernel fix bc2473c90fca
> > > ("ovl: enable fsnotify events on underlying real files"),
> > > from kernel 6.5, which is not likely to be backported to older kernels.

> > > To avoid waiting for events that won't arrive when testing old kernels,
> > > require that kernel supports encoding fid with new flag AT_HADNLE_FID,
> > > also merged to 6.5 and not likely to be backported to older kernels.

> > Unfortunately Petr's not here at the moment.

> > I guess this first patch doesn't require 6.6? So it could be merged
> > independently without further considerations for what makes it into 6.6?


> Yes that is correct.

I'm finally back :).

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

I'll ask Cyril this patch to be merged before LTP release (final day today
before git freeze).

Kind regards,
Petr

> Thanks,
> Amir.
Cyril Hrubis Sept. 15, 2023, 8:28 a.m. UTC | #4
Hi!
> ---
>  lib/tst_fs_setup.c                            |  2 +-
>  testcases/kernel/syscalls/fanotify/fanotify.h | 21 +++++++
>  .../kernel/syscalls/fanotify/fanotify13.c     | 62 +++++++++++++++++--
>  3 files changed, 79 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
> index 6b93483de..30673670f 100644
> --- a/lib/tst_fs_setup.c
> +++ b/lib/tst_fs_setup.c
> @@ -42,7 +42,7 @@ int mount_overlay(const char *file, const int lineno, int skip)
>  			tst_res_(file, lineno, TINFO,
>  				TST_FS_SETUP_OVERLAYFS_MSG);
>  		}
> -	} else {
> +	} else if (skip) {
>  		tst_brk_(file, lineno, TBROK | TERRNO,
>  			"overlayfs mount failed");
>  	}

The skip flag should be called strict, at least that is what we usually
name it, but that is very minor.

...

>  static struct tst_test test = {
>  	.test = do_test,
>  	.tcnt = ARRAY_SIZE(test_cases),
> +	.test_variants = 2,
>  	.setup = do_setup,
>  	.cleanup = do_cleanup,
>  	.needs_root = 1,
>  	.mount_device = 1,
> -	.mntpoint = MOUNT_PATH,
> +	.mntpoint = OVL_BASE_MNTPOINT,
>  	.all_filesystems = 1,
>  	.tags = (const struct tst_tag[]) {
>  		{"linux-git", "c285a2f01d69"},

The git hash for the regression test with variant=1 should have been
added here.


The rest looks good to me. With the two minor issues fixed:

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

@Peter Vorel Feel free to push the patch with the two fixes applied.
Petr Vorel Sept. 15, 2023, 9:02 a.m. UTC | #5
Hi all,

...
> > -	} else {
> > +	} else if (skip) {
> >  		tst_brk_(file, lineno, TBROK | TERRNO,
> >  			"overlayfs mount failed");
> >  	}

> The skip flag should be called strict, at least that is what we usually
> name it, but that is very minor.

> ...

> >  static struct tst_test test = {
> >  	.test = do_test,
> >  	.tcnt = ARRAY_SIZE(test_cases),
> > +	.test_variants = 2,
> >  	.setup = do_setup,
> >  	.cleanup = do_cleanup,
> >  	.needs_root = 1,
> >  	.mount_device = 1,
> > -	.mntpoint = MOUNT_PATH,
> > +	.mntpoint = OVL_BASE_MNTPOINT,
> >  	.all_filesystems = 1,
> >  	.tags = (const struct tst_tag[]) {
> >  		{"linux-git", "c285a2f01d69"},

> The git hash for the regression test with variant=1 should have been
> added here.

> The rest looks good to me. With the two minor issues fixed:

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

> @Peter Vorel Feel free to push the patch with the two fixes applied.

Thanks for spotting both issues, fixed and merged.

Kind regards,
Petr
diff mbox series

Patch

diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
index 6b93483de..30673670f 100644
--- a/lib/tst_fs_setup.c
+++ b/lib/tst_fs_setup.c
@@ -42,7 +42,7 @@  int mount_overlay(const char *file, const int lineno, int skip)
 			tst_res_(file, lineno, TINFO,
 				TST_FS_SETUP_OVERLAYFS_MSG);
 		}
-	} else {
+	} else if (skip) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"overlayfs mount failed");
 	}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 51078103e..75a081dc9 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -72,6 +72,10 @@  static inline int safe_fanotify_mark(const char *file, const int lineno,
 #define MAX_HANDLE_SZ		128
 #endif
 
+#ifndef AT_HANDLE_FID
+#define AT_HANDLE_FID		0x200
+#endif
+
 /*
  * Helper function used to obtain fsid and file_handle for a given path.
  * Used by test files correlated to FAN_REPORT_FID functionality.
@@ -260,10 +264,27 @@  static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
 	return rval;
 }
 
+static inline int fanotify_handle_supported_by_kernel(int flag)
+{
+	/*
+	 * On Kernel that does not support AT_HANDLE_FID this will result
+	 * with EINVAL. On older kernels, this will result in EBADF.
+	 */
+	if (name_to_handle_at(-1, "", NULL, NULL, AT_EMPTY_PATH | flag)) {
+		if (errno == EINVAL)
+			return -1;
+	}
+	return 0;
+}
+
 #define REQUIRE_MARK_TYPE_SUPPORTED_BY_KERNEL(mark_type) \
 	fanotify_init_flags_err_msg(#mark_type, __FILE__, __LINE__, tst_brk_, \
 				    fanotify_mark_supported_by_kernel(mark_type))
 
+#define REQUIRE_HANDLE_TYPE_SUPPORTED_BY_KERNEL(handle_type) \
+	fanotify_init_flags_err_msg(#handle_type, __FILE__, __LINE__, tst_brk_, \
+				    fanotify_handle_supported_by_kernel(handle_type))
+
 #define REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(init_flags, mark_type, mask, fname) do { \
 	if (mark_type)							\
 		REQUIRE_MARK_TYPE_SUPPORTED_BY_KERNEL(mark_type);	\
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index c3daaf3a2..adba41453 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -25,6 +25,7 @@ 
 #include <sys/statfs.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/mount.h>
 #include <errno.h>
 #include <unistd.h>
 #include "tst_test.h"
@@ -37,7 +38,7 @@ 
 #define DIR_ONE "dir_one"
 #define FILE_ONE "file_one"
 #define FILE_TWO "file_two"
-#define MOUNT_PATH "mntpoint"
+#define MOUNT_PATH "tstmnt"
 #define EVENT_MAX ARRAY_SIZE(objects)
 #define DIR_PATH_ONE MOUNT_PATH"/"DIR_ONE
 #define FILE_PATH_ONE MOUNT_PATH"/"FILE_ONE
@@ -88,6 +89,8 @@  static struct test_case_t {
 	}
 };
 
+static int ovl_mounted;
+static int bind_mounted;
 static int nofid_fd;
 static int fanotify_fd;
 static int filesystem_mark_unsupported;
@@ -143,8 +146,13 @@  static void do_test(unsigned int number)
 	struct fanotify_mark_type *mark = &tc->mark;
 
 	tst_res(TINFO,
-		"Test #%d: FAN_REPORT_FID with mark flag: %s",
-		number, mark->name);
+		"Test #%d.%d: FAN_REPORT_FID with mark flag: %s",
+		number, tst_variant, mark->name);
+
+	if (tst_variant && !ovl_mounted) {
+		tst_res(TCONF, "overlayfs not supported on %s", tst_device->fs_type);
+		return;
+	}
 
 	if (filesystem_mark_unsupported && mark->flag & FAN_MARK_FILESYSTEM) {
 		tst_res(TCONF, "FAN_MARK_FILESYSTEM not supported in kernel?");
@@ -160,6 +168,15 @@  static void do_test(unsigned int number)
 	if (setup_marks(fanotify_fd, tc) != 0)
 		goto out;
 
+	/* Variant #1: watching upper fs - open files on overlayfs */
+	if (tst_variant == 1) {
+		if (mark->flag & FAN_MARK_MOUNT) {
+			tst_res(TCONF, "overlayfs upper fs cannot be watched with mount mark");
+			goto out;
+		}
+		SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
+	}
+
 	/* Generate sequence of FAN_OPEN events on objects */
 	for (i = 0; i < ARRAY_SIZE(objects); i++)
 		fds[i] = SAFE_OPEN(objects[i].path, O_RDONLY);
@@ -174,6 +191,9 @@  static void do_test(unsigned int number)
 			SAFE_CLOSE(fds[i]);
 	}
 
+	if (tst_variant == 1)
+		SAFE_UMOUNT(MOUNT_PATH);
+
 	/* Read events from event queue */
 	len = SAFE_READ(0, fanotify_fd, events_buf, BUF_SIZE);
 
@@ -261,7 +281,32 @@  out:
 
 static void do_setup(void)
 {
-	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MOUNT_PATH);
+	const char *mnt;
+
+	/*
+	 * Bind mount to either base fs or to overlayfs over base fs:
+	 * Variant #0: watch base fs - open files on base fs
+	 * Variant #1: watch upper fs - open files on overlayfs
+	 *
+	 * Variant #1 tests a bug whose fix bc2473c90fca ("ovl: enable fsnotify
+	 * events on underlying real files") in kernel 6.5 is not likely to be
+	 * backported to older kernels.
+	 * To avoid waiting for events that won't arrive when testing old kernels,
+	 * require that kernel supports encoding fid with new flag AT_HADNLE_FID,
+	 * also merged to 6.5 and not likely to be backported to older kernels.
+	 */
+	if (tst_variant) {
+		REQUIRE_HANDLE_TYPE_SUPPORTED_BY_KERNEL(AT_HANDLE_FID);
+		ovl_mounted = TST_MOUNT_OVERLAY();
+		mnt = OVL_UPPER;
+	} else {
+		mnt = OVL_BASE_MNTPOINT;
+
+	}
+	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
+	SAFE_MKDIR(MOUNT_PATH, 0755);
+	SAFE_MOUNT(mnt, MOUNT_PATH, "none", MS_BIND, NULL);
+	bind_mounted = 1;
 
 	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
 
@@ -287,16 +332,23 @@  static void do_cleanup(void)
 	SAFE_CLOSE(nofid_fd);
 	if (fanotify_fd > 0)
 		SAFE_CLOSE(fanotify_fd);
+	if (bind_mounted) {
+		SAFE_UMOUNT(MOUNT_PATH);
+		SAFE_RMDIR(MOUNT_PATH);
+	}
+	if (ovl_mounted)
+		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
 	.test = do_test,
 	.tcnt = ARRAY_SIZE(test_cases),
+	.test_variants = 2,
 	.setup = do_setup,
 	.cleanup = do_cleanup,
 	.needs_root = 1,
 	.mount_device = 1,
-	.mntpoint = MOUNT_PATH,
+	.mntpoint = OVL_BASE_MNTPOINT,
 	.all_filesystems = 1,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "c285a2f01d69"},