[LTP] [PATCH] OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

[LTP] [PATCH] OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

[Murphy Zhou]

Some tests are mounting overlayfs internally and run tests on it.
This mount can fail if the filesystem we are running on does not
support overlay mount upon it. For example, we are already running
tests on overlayfs or NFS, or CIFS. Test will report broken and
failure.

Fixing this by put overlayfs dirs in a separaed mountpoint, like
readahead02 by Amir.

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---
 .../kernel/syscalls/execveat/execveat03.c     | 21 ++++++++++-----
 testcases/kernel/syscalls/inotify/inotify07.c | 24 +++++++++++------
 testcases/kernel/syscalls/inotify/inotify08.c | 26 ++++++++++++-------
 3 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
index def33923b..8ba6a656f 100644
--- a/testcases/kernel/syscalls/execveat/execveat03.c
+++ b/testcases/kernel/syscalls/execveat/execveat03.c
@@ -55,10 +55,17 @@
 #include "lapi/fcntl.h"
 #include "execveat.h"
 
-#define OVL_MNT "ovl"
+#define MNTPOINT	"mntpoint"
+#define OVL_LOWER	MNTPOINT"/lower"
+#define OVL_UPPER	MNTPOINT"/upper"
+#define OVL_WORK	MNTPOINT"/work"
+#define OVL_MNT		MNTPOINT"/ovl"
+
 #define TEST_APP "execveat_child"
 #define TEST_FILE_PATH OVL_MNT"/"TEST_APP
 
+static const char mntpoint[] = MNTPOINT;
+
 static int ovl_mounted;
 
 static void do_child(void)
@@ -91,12 +98,12 @@ static void setup(void)
 	check_execveat();
 
 	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
 	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
 	if (ret < 0) {
 		if (errno == ENODEV) {
 			tst_brk(TCONF,
@@ -121,6 +128,8 @@ static const char *const resource_files[] = {
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.setup = setup,
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index 96370b5cf..3e2d06485 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -73,13 +73,19 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
+#define MNTPOINT	"mntpoint"
+#define OVL_LOWER	MNTPOINT"/lower"
+#define OVL_UPPER	MNTPOINT"/upper"
+#define OVL_WORK	MNTPOINT"/work"
+#define OVL_MNT		MNTPOINT"/ovl"
+
 #define DIR_NAME "test_dir"
 #define DIR_PATH OVL_MNT"/"DIR_NAME
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -164,14 +170,14 @@ static void setup(void)
 	int ret;
 
 	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("lower/"DIR_NAME, 0755);
-	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
+	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
 	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
 	if (ret < 0) {
 		if (errno == ENODEV) {
 			tst_brk(TCONF,
@@ -234,6 +240,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index acdb95345..a512dfe22 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -74,11 +74,17 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
+#define MNTPOINT	"mntpoint"
+#define OVL_LOWER	MNTPOINT"/lower"
+#define OVL_UPPER	MNTPOINT"/upper"
+#define OVL_WORK	MNTPOINT"/work"
+#define OVL_MNT		MNTPOINT"/ovl"
+
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -104,8 +110,8 @@ void verify_inotify(void)
 	test_cnt++;
 
 	/* Make sure events on upper/lower do not show in overlay watch */
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_TOUCH("upper/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
 
 	int len = read(fd_notify, event_buf, EVENT_BUF_LEN);
 	if (len == -1 && errno != EAGAIN) {
@@ -157,13 +163,13 @@ static void setup(void)
 	int ret;
 
 	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
 	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
 	if (ret < 0) {
 		if (errno == ENODEV) {
 			tst_brk(TCONF,
@@ -230,6 +236,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
-- 
2.21.0

[LTP] [PATCH] OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

[Li Wang]

On Tue, Apr 30, 2019 at 12:40 PM Murphy Zhou <xzhou@redhat.com> wrote:

> Some tests are mounting overlayfs internally and run tests on it.
> This mount can fail if the filesystem we are running on does not
> support overlay mount upon it. For example, we are already running
> tests on overlayfs or NFS, or CIFS. Test will report broken and
> failure.
>
> Fixing this by put overlayfs dirs in a separaed mountpoint, like
> readahead02 by Amir.
>
> Signed-off-by: Xiong Zhou <xzhou@redhat.com>
>
Reviewed-by: Li Wang <liwang@redhat.com>

LGTM.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190430/d0fb3739/attachment.html>

[LTP] [PATCH] OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

[Petr Vorel]

Hi,

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

>  testcases/kernel/syscalls/inotify/inotify07.c | 24 +++++++++++------

> -#define OVL_MNT "ovl"
> +#define MNTPOINT	"mntpoint"
> +#define OVL_LOWER	MNTPOINT"/lower"
> +#define OVL_UPPER	MNTPOINT"/upper"
> +#define OVL_WORK	MNTPOINT"/work"
> +#define OVL_MNT		MNTPOINT"/ovl"
> +
>  #define TEST_APP "execveat_child"
>  #define TEST_FILE_PATH OVL_MNT"/"TEST_APP

> +static const char mntpoint[] = MNTPOINT;
> +
>  static int ovl_mounted;

>  static void do_child(void)
> @@ -91,12 +98,12 @@ static void setup(void)
>  	check_execveat();

>  	/* Setup an overlay mount with lower file */
> -	SAFE_MKDIR("lower", 0755);
> -	SAFE_MKDIR("upper", 0755);
> -	SAFE_MKDIR("work", 0755);
> +	SAFE_MKDIR(OVL_LOWER, 0755);
> +	SAFE_MKDIR(OVL_UPPER, 0755);
> +	SAFE_MKDIR(OVL_WORK, 0755);
>  	SAFE_MKDIR(OVL_MNT, 0755);
NOTE: Maybe it'd be worth of adding some helper to define these constants and creating this setup (in separate commit).


Kind regards,
Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Murphy Zhou]

To create overlayfs dirs, and mount overlayfs if needed.

Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
v2:
  define constraints in header file
  add a setup helper to create dirs and mount

 include/tst_fs.h   | 12 ++++++++++++
 lib/tst_fs_setup.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 lib/tst_fs_setup.c

diff --git a/include/tst_fs.h b/include/tst_fs.h
index 423ca82ec..750bb1a91 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -50,6 +50,18 @@ enum {
 	TST_GB = 1073741824,
 };
 
+#define OVL_BASE_MNTPOINT        "mntpoint"
+#define OVL_LOWER	OVL_BASE_MNTPOINT"/lower"
+#define OVL_UPPER	OVL_BASE_MNTPOINT"/upper"
+#define OVL_WORK	OVL_BASE_MNTPOINT"/work"
+#define OVL_MNT		OVL_BASE_MNTPOINT"/ovl"
+
+/*
+ * Create above overlayfs constraints, if mount == 1,
+ * mount overlayfs
+ */
+int setup_overlay(int mount);
+
 /*
  * @path: path is the pathname of any file within the mounted file system
  * @mult: mult should be TST_KB, TST_MB or TST_GB
diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
new file mode 100644
index 000000000..b1880ee11
--- /dev/null
+++ b/lib/tst_fs_setup.c
@@ -0,0 +1,42 @@
+/*
+ * DESCRIPTION
+ *	A place for setup filesystem helpers.
+ */
+
+#include <stdint.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/vfs.h>
+#include <sys/mount.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_fs.h"
+
+int setup_overlay(int mountovl)
+{
+	int ret;
+
+	/* Setup an overlay mount with lower dir and file */
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
+	SAFE_MKDIR(OVL_MNT, 0755);
+
+	/* Only create dirs, do not mount */
+	if (mountovl == 0)
+		return 0;
+
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
+	if (ret < 0) {
+		if (errno == ENODEV) {
+			tst_res(TINFO,
+				"overlayfs is not configured in this kernel.");
+			return 1;
+		}
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+	}
+	return 0;
+}
-- 
2.21.0

[LTP] [PATCH v2 2/2] OVL_MNT: put overlayfs lower, upper, work, mnt dir in a separated mountpoint

[Murphy Zhou]

Some tests are mounting overlayfs internally and run tests on it.
This mount can fail if the filesystem we are running on does not
support overlay mount upon it. For example, we are already running
tests on overlayfs or NFS, or CIFS. Test will report broken and
failure.

Fixing this by put overlayfs dirs in a separaed mountpoint, like
readahead02 by Amir.

Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
 .../kernel/syscalls/execveat/execveat03.c     | 24 ++++----------
 testcases/kernel/syscalls/inotify/inotify07.c | 17 +++++-----
 testcases/kernel/syscalls/inotify/inotify08.c | 19 +++++------
 .../kernel/syscalls/readahead/readahead02.c   | 33 +++----------------
 4 files changed, 27 insertions(+), 66 deletions(-)

diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
index def33923b..ff4de62ee 100644
--- a/testcases/kernel/syscalls/execveat/execveat03.c
+++ b/testcases/kernel/syscalls/execveat/execveat03.c
@@ -55,10 +55,11 @@
 #include "lapi/fcntl.h"
 #include "execveat.h"
 
-#define OVL_MNT "ovl"
 #define TEST_APP "execveat_child"
 #define TEST_FILE_PATH OVL_MNT"/"TEST_APP
 
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
+
 static int ovl_mounted;
 
 static void do_child(void)
@@ -86,25 +87,10 @@ static void verify_execveat(void)
 
 static void setup(void)
 {
-	int ret;
-
 	check_execveat();
 
-	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
+	if (setup_overlay(1) == 0)
+		ovl_mounted = 1;
 }
 
 static void cleanup(void)
@@ -121,6 +107,8 @@ static const char *const resource_files[] = {
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.setup = setup,
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index 96370b5cf..fd0caff37 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -73,13 +73,13 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define DIR_NAME "test_dir"
 #define DIR_PATH OVL_MNT"/"DIR_NAME
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -164,14 +164,11 @@ static void setup(void)
 	int ret;
 
 	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("lower/"DIR_NAME, 0755);
-	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
+	setup_overlay(0);
+	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
+	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
 	if (ret < 0) {
 		if (errno == ENODEV) {
 			tst_brk(TCONF,
@@ -234,6 +231,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index acdb95345..a17a6a5c7 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -74,11 +74,11 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -104,8 +104,8 @@ void verify_inotify(void)
 	test_cnt++;
 
 	/* Make sure events on upper/lower do not show in overlay watch */
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_TOUCH("upper/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
 
 	int len = read(fd_notify, event_buf, EVENT_BUF_LEN);
 	if (len == -1 && errno != EAGAIN) {
@@ -157,13 +157,10 @@ static void setup(void)
 	int ret;
 
 	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
+	setup_overlay(0);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
 	if (ret < 0) {
 		if (errno == ENODEV) {
 			tst_brk(TCONF,
@@ -230,6 +227,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 39ddbd583..1b6b70466 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -44,16 +44,11 @@ static int pagesize;
 static unsigned long cached_max;
 static int ovl_mounted;
 
-#define MNTPOINT        "mntpoint"
-#define OVL_LOWER	MNTPOINT"/lower"
-#define OVL_UPPER	MNTPOINT"/upper"
-#define OVL_WORK	MNTPOINT"/work"
-#define OVL_MNT		MNTPOINT"/ovl"
 static int readahead_length  = 4096;
 static char sys_bdi_ra_path[PATH_MAX];
 static int orig_bdi_limit;
 
-static const char mntpoint[] = MNTPOINT;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct tst_option options[] = {
 	{"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
@@ -132,7 +127,7 @@ static void create_testfile(int use_overlay)
 	char *tmp;
 	size_t i;
 
-	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
+	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : OVL_BASE_MNTPOINT);
 	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
@@ -329,27 +324,6 @@ static void test_readahead(unsigned int n)
 	}
 }
 
-static void setup_overlay(void)
-{
-	int ret;
-
-	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR(OVL_LOWER, 0755);
-	SAFE_MKDIR(OVL_UPPER, 0755);
-	SAFE_MKDIR(OVL_WORK, 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
-		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_res(TINFO,
-				"overlayfs is not configured in this kernel.");
-			return;
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
-}
 
 /*
  * We try raising bdi readahead limit as much as we can. We write
@@ -413,7 +387,8 @@ static void setup(void)
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
 
-	setup_overlay();
+	if (setup_overlay(1) == 0)
+		ovl_mounted = 1;
 }
 
 static void cleanup(void)
-- 
2.21.0

[LTP] [PATCH v2 2/2] OVL_MNT: put overlayfs lower, upper, work, mnt dir in a separated mountpoint

[Petr Vorel]

Hi Murphy,

> --- a/testcases/kernel/syscalls/execveat/execveat03.c
...
> -	/* Setup an overlay mount with lower file */
> -	SAFE_MKDIR("lower", 0755);
> -	SAFE_MKDIR("upper", 0755);
> -	SAFE_MKDIR("work", 0755);
> -	SAFE_MKDIR(OVL_MNT, 0755);
> -	ret = mount("overlay", OVL_MNT, "overlay", 0,
> -		    "lowerdir=lower,upperdir=upper,workdir=work");
> -	if (ret < 0) {
> -		if (errno == ENODEV) {
> -			tst_brk(TCONF,
> -				"overlayfs is not configured in this kernel.");
> -		}
> -		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
> -	}
> -	ovl_mounted = 1;
> +	if (setup_overlay(1) == 0)
> +		ovl_mounted = 1;
Here you change test behavior on ENODEV.

+ more info at 1st patch.

Kind regards,
Petr

[LTP] [PATCH v2 2/2] OVL_MNT: put overlayfs lower, upper, work, mnt dir in a separated mountpoint

[Murphy Zhou]

On Wed, May 15, 2019 at 03:39:40PM +0200, Petr Vorel wrote:
> Hi Murphy,
> 
> > --- a/testcases/kernel/syscalls/execveat/execveat03.c
> ...
> > -	/* Setup an overlay mount with lower file */
> > -	SAFE_MKDIR("lower", 0755);
> > -	SAFE_MKDIR("upper", 0755);
> > -	SAFE_MKDIR("work", 0755);
> > -	SAFE_MKDIR(OVL_MNT, 0755);
> > -	ret = mount("overlay", OVL_MNT, "overlay", 0,
> > -		    "lowerdir=lower,upperdir=upper,workdir=work");
> > -	if (ret < 0) {
> > -		if (errno == ENODEV) {
> > -			tst_brk(TCONF,
> > -				"overlayfs is not configured in this kernel.");
> > -		}
> > -		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
> > -	}
> > -	ovl_mounted = 1;
> > +	if (setup_overlay(1) == 0)
> > +		ovl_mounted = 1;
> Here you change test behavior on ENODEV.

Great catch! I missed this when copying setup_overlay from readahead02.
inotify tests are different from this.

Thanks!
m

> 
> + more info at 1st patch.
> 
> Kind regards,
> Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Petr Vorel]

Hi Murphy,

> To create overlayfs dirs, and mount overlayfs if needed.

...
> +int setup_overlay(int mountovl)
> +{
> +	int ret;
> +
> +	/* Setup an overlay mount with lower dir and file */
> +	SAFE_MKDIR(OVL_LOWER, 0755);
> +	SAFE_MKDIR(OVL_UPPER, 0755);
> +	SAFE_MKDIR(OVL_WORK, 0755);
> +	SAFE_MKDIR(OVL_MNT, 0755);
> +
> +	/* Only create dirs, do not mount */
> +	if (mountovl == 0)
> +		return 0;
Instead of having int parameter, there could be create_overlay_dirs()
and mount_overlay(), which would call create_overlay_dirs().
(no need to lookup meaning of parameter).

> +
> +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> +	if (ret < 0) {
> +		if (errno == ENODEV) {
> +			tst_res(TINFO,
> +				"overlayfs is not configured in this kernel.");
> +			return 1;
First I thought we'd implement it as a test flag (member of struct tst_test).
When we have it as separate function I wonder whether we could TCONF on ENODEV
instead of TINFO and return. Maybe there could be here for int strict parameter,
where 1 would be force safe (i.e. TCONF), otherwise only TINFO.
This could also to have SAFE_MOUNT_OVERLAY() macro which would use mount_overlay().
Similar approach as SAFE_SEND() and safe_send().

> +		}
> +		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
> +	}
> +	return 0;
> +}

Kind regards,
Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Petr Vorel]

Hi Murphy,

> ...
> > +int setup_overlay(int mountovl)
> > +{
> > +	int ret;
> > +
> > +	/* Setup an overlay mount with lower dir and file */
> > +	SAFE_MKDIR(OVL_LOWER, 0755);
> > +	SAFE_MKDIR(OVL_UPPER, 0755);
> > +	SAFE_MKDIR(OVL_WORK, 0755);
> > +	SAFE_MKDIR(OVL_MNT, 0755);
> > +
> > +	/* Only create dirs, do not mount */
> > +	if (mountovl == 0)
> > +		return 0;
> Instead of having int parameter, there could be create_overlay_dirs()
> and mount_overlay(), which would call create_overlay_dirs().
> (no need to lookup meaning of parameter).

> > +
> > +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > +	if (ret < 0) {
> > +		if (errno == ENODEV) {
> > +			tst_res(TINFO,
> > +				"overlayfs is not configured in this kernel.");
> > +			return 1;
> First I thought we'd implement it as a test flag (member of struct tst_test).
> When we have it as separate function I wonder whether we could TCONF on ENODEV
> instead of TINFO and return. Maybe there could be here for int strict parameter,
> where 1 would be force safe (i.e. TCONF), otherwise only TINFO.
> This could also to have SAFE_MOUNT_OVERLAY() macro which would use mount_overlay().
> Similar approach as SAFE_SEND() and safe_send().

From next patch:
> +++ b/testcases/kernel/syscalls/inotify/inotify07.c
> -#define OVL_MNT "ovl"
>  #define DIR_NAME "test_dir"
>  #define DIR_PATH OVL_MNT"/"DIR_NAME
>  #define FILE_NAME "test_file"
>  #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME

>  static int ovl_mounted;
> +static const char mntpoint[] = OVL_BASE_MNTPOINT;

>  static struct event_t event_set[EVENT_MAX];

> @@ -164,14 +164,11 @@ static void setup(void)
>  	int ret;

>  	/* Setup an overlay mount with lower dir and file */
> -	SAFE_MKDIR("lower", 0755);
> -	SAFE_MKDIR("lower/"DIR_NAME, 0755);
> -	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> -	SAFE_MKDIR("upper", 0755);
> -	SAFE_MKDIR("work", 0755);
> -	SAFE_MKDIR(OVL_MNT, 0755);
> -	ret = mount("overlay", OVL_MNT, "overlay", 0,
> -		    "lowerdir=lower,upperdir=upper,workdir=work");
> +	setup_overlay(0);
> +	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
> +	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
>  	if (ret < 0) {
>  		if (errno == ENODEV) {
>  			tst_brk(TCONF,
In here in inotify07.c and in inotify08.c you create dirs (0 parameter) for because you it's
needed to create more dirs. Than the rest (mount, TCONF on ENODEV, TBROK
otherwise) is still copy pasted. I wonder how to move everything into
setup_overlay() helper. Maybe struct with files or directories and permissions
to for touch/mkdir? With this, int mountovl dir create_overlay_dirs() might not
be needed.


Kind regards,
Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Amir Goldstein]

On Wed, May 15, 2019 at 4:42 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Murphy,
>
> > ...
> > > +int setup_overlay(int mountovl)
> > > +{
> > > +   int ret;
> > > +
> > > +   /* Setup an overlay mount with lower dir and file */
> > > +   SAFE_MKDIR(OVL_LOWER, 0755);
> > > +   SAFE_MKDIR(OVL_UPPER, 0755);
> > > +   SAFE_MKDIR(OVL_WORK, 0755);
> > > +   SAFE_MKDIR(OVL_MNT, 0755);
> > > +
> > > +   /* Only create dirs, do not mount */
> > > +   if (mountovl == 0)
> > > +           return 0;
> > Instead of having int parameter, there could be create_overlay_dirs()
> > and mount_overlay(), which would call create_overlay_dirs().
> > (no need to lookup meaning of parameter).
>
> > > +
> > > +   ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > > +               ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > > +   if (ret < 0) {
> > > +           if (errno == ENODEV) {
> > > +                   tst_res(TINFO,
> > > +                           "overlayfs is not configured in this kernel.");
> > > +                   return 1;
> > First I thought we'd implement it as a test flag (member of struct tst_test).
> > When we have it as separate function I wonder whether we could TCONF on ENODEV
> > instead of TINFO and return. Maybe there could be here for int strict parameter,
> > where 1 would be force safe (i.e. TCONF), otherwise only TINFO.
> > This could also to have SAFE_MOUNT_OVERLAY() macro which would use mount_overlay().
> > Similar approach as SAFE_SEND() and safe_send().
>
> From next patch:
> > +++ b/testcases/kernel/syscalls/inotify/inotify07.c
> > -#define OVL_MNT "ovl"
> >  #define DIR_NAME "test_dir"
> >  #define DIR_PATH OVL_MNT"/"DIR_NAME
> >  #define FILE_NAME "test_file"
> >  #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
>
> >  static int ovl_mounted;
> > +static const char mntpoint[] = OVL_BASE_MNTPOINT;
>
> >  static struct event_t event_set[EVENT_MAX];
>
> > @@ -164,14 +164,11 @@ static void setup(void)
> >       int ret;
>
> >       /* Setup an overlay mount with lower dir and file */
> > -     SAFE_MKDIR("lower", 0755);
> > -     SAFE_MKDIR("lower/"DIR_NAME, 0755);
> > -     SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> > -     SAFE_MKDIR("upper", 0755);
> > -     SAFE_MKDIR("work", 0755);
> > -     SAFE_MKDIR(OVL_MNT, 0755);
> > -     ret = mount("overlay", OVL_MNT, "overlay", 0,
> > -                 "lowerdir=lower,upperdir=upper,workdir=work");
> > +     setup_overlay(0);
> > +     SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
> > +     SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> > +     ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > +                 ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> >       if (ret < 0) {
> >               if (errno == ENODEV) {
> >                       tst_brk(TCONF,
> In here in inotify07.c and in inotify08.c you create dirs (0 parameter) for because you it's
> needed to create more dirs. Than the rest (mount, TCONF on ENODEV, TBROK
> otherwise) is still copy pasted. I wonder how to move everything into
> setup_overlay() helper. Maybe struct with files or directories and permissions
> to for touch/mkdir? With this, int mountovl dir create_overlay_dirs() might not
> be needed.
>
>

I liked your idea of create_overlay_dirs/mount_overlay better.
It is more flexible for future tests that may want to umount and
mount overlay several times and the setup of lower/upper files
could be very different in future tests, not limited to just creating
files (maybe symlinks, xattr).
The experience from xfstests overlay tests suggests that the
create_overlay_dirs/mount_overlay helpers are a good model to follow.

Thanks,
Amir.

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Petr Vorel]

Hi Amir, Murphy,


> I liked your idea of create_overlay_dirs/mount_overlay better.
> It is more flexible for future tests that may want to umount and
> mount overlay several times and the setup of lower/upper files
> could be very different in future tests, not limited to just creating
> files (maybe symlinks, xattr).
> The experience from xfstests overlay tests suggests that the
> create_overlay_dirs/mount_overlay helpers are a good model to follow.

Thanks for ideas, agree with it.

Other thing is to have const char *file, const int lineno as first parameters
for mount_overlay_() and create mount_overlay() and SAFE_MOUNT_OVERLAY() as macros,
which would use it. See tst_mkfs{_,}() as examples.

> Thanks,
> Amir.

Kind regards,
Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Murphy Zhou]

Hi Amir and Petr,

On Wed, May 15, 2019 at 08:20:57PM +0200, Petr Vorel wrote:
> Hi Amir, Murphy,
> 
> 
> > I liked your idea of create_overlay_dirs/mount_overlay better.
> > It is more flexible for future tests that may want to umount and
> > mount overlay several times and the setup of lower/upper files
> > could be very different in future tests, not limited to just creating
> > files (maybe symlinks, xattr).
> > The experience from xfstests overlay tests suggests that the
> > create_overlay_dirs/mount_overlay helpers are a good model to follow.
> 
> Thanks for ideas, agree with it.
> 
> Other thing is to have const char *file, const int lineno as first parameters
> for mount_overlay_() and create mount_overlay() and SAFE_MOUNT_OVERLAY() as macros,
> which would use it. See tst_mkfs{_,}() as examples.

Very good suggests! I'll digging into it.

Thanks,
M
> 
> > Thanks,
> > Amir.
> 
> Kind regards,
> Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Murphy Zhou]

Hi Petr,

On Wed, May 15, 2019 at 03:42:45PM +0200, Petr Vorel wrote:
> Hi Murphy,
> 
> > ...
> > > +int setup_overlay(int mountovl)
> > > +{
> > > +	int ret;
> > > +
> > > +	/* Setup an overlay mount with lower dir and file */
> > > +	SAFE_MKDIR(OVL_LOWER, 0755);
> > > +	SAFE_MKDIR(OVL_UPPER, 0755);
> > > +	SAFE_MKDIR(OVL_WORK, 0755);
> > > +	SAFE_MKDIR(OVL_MNT, 0755);
> > > +
> > > +	/* Only create dirs, do not mount */
> > > +	if (mountovl == 0)
> > > +		return 0;
> > Instead of having int parameter, there could be create_overlay_dirs()
> > and mount_overlay(), which would call create_overlay_dirs().
> > (no need to lookup meaning of parameter).
> 
> > > +
> > > +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > > +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > > +	if (ret < 0) {
> > > +		if (errno == ENODEV) {
> > > +			tst_res(TINFO,
> > > +				"overlayfs is not configured in this kernel.");
> > > +			return 1;
> > First I thought we'd implement it as a test flag (member of struct tst_test).
> > When we have it as separate function I wonder whether we could TCONF on ENODEV
> > instead of TINFO and return. Maybe there could be here for int strict parameter,
> > where 1 would be force safe (i.e. TCONF), otherwise only TINFO.
> > This could also to have SAFE_MOUNT_OVERLAY() macro which would use mount_overlay().
> > Similar approach as SAFE_SEND() and safe_send().
> 
> From next patch:
> > +++ b/testcases/kernel/syscalls/inotify/inotify07.c
> > -#define OVL_MNT "ovl"
> >  #define DIR_NAME "test_dir"
> >  #define DIR_PATH OVL_MNT"/"DIR_NAME
> >  #define FILE_NAME "test_file"
> >  #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
> 
> >  static int ovl_mounted;
> > +static const char mntpoint[] = OVL_BASE_MNTPOINT;
> 
> >  static struct event_t event_set[EVENT_MAX];
> 
> > @@ -164,14 +164,11 @@ static void setup(void)
> >  	int ret;
> 
> >  	/* Setup an overlay mount with lower dir and file */
> > -	SAFE_MKDIR("lower", 0755);
> > -	SAFE_MKDIR("lower/"DIR_NAME, 0755);
> > -	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> > -	SAFE_MKDIR("upper", 0755);
> > -	SAFE_MKDIR("work", 0755);
> > -	SAFE_MKDIR(OVL_MNT, 0755);
> > -	ret = mount("overlay", OVL_MNT, "overlay", 0,
> > -		    "lowerdir=lower,upperdir=upper,workdir=work");
> > +	setup_overlay(0);
> > +	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
> > +	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> > +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> >  	if (ret < 0) {
> >  		if (errno == ENODEV) {
> >  			tst_brk(TCONF,
> In here in inotify07.c and in inotify08.c you create dirs (0 parameter) for because you it's
> needed to create more dirs. Than the rest (mount, TCONF on ENODEV, TBROK
> otherwise) is still copy pasted. I wonder how to move everything into
> setup_overlay() helper. Maybe struct with files or directories and permissions

If we define a struct to put names amd modes in it then pass to helper, we
still need to write all these OVL macros in the testcase to defile the struct.
So we need to write all the _same_ macros in every testcase where needed.

In this case, it's against my intention of this patch: dedupe duplicated lines.

Your struct idea is great when handling the different files that need to be
created in different testcases. However I'd like to do it in a simpler way.
Only make necessary dirs in the helper, let the testcases to create the other
dirs they want themselves.

Thanks,
Murphy

> to for touch/mkdir? With this, int mountovl dir create_overlay_dirs() might not
> be needed.
> 
> 
> Kind regards,
> Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Petr Vorel]

Hi Murphy,

> > > +	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
> > > +	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> > > +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > > +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > >  	if (ret < 0) {
> > >  		if (errno == ENODEV) {
> > >  			tst_brk(TCONF,
> > In here in inotify07.c and in inotify08.c you create dirs (0 parameter) for because you it's
> > needed to create more dirs. Than the rest (mount, TCONF on ENODEV, TBROK
> > otherwise) is still copy pasted. I wonder how to move everything into
> > setup_overlay() helper. Maybe struct with files or directories and permissions

> If we define a struct to put names amd modes in it then pass to helper, we
> still need to write all these OVL macros in the testcase to defile the struct.
> So we need to write all the _same_ macros in every testcase where needed.

> In this case, it's against my intention of this patch: dedupe duplicated lines.
Sure, DRY is intention :).

> Your struct idea is great when handling the different files that need to be
> created in different testcases. However I'd like to do it in a simpler way.
> Only make necessary dirs in the helper, let the testcases to create the other
> dirs they want themselves.
I agree with Amir [1] that my original approach [2]: to add
create_overlay_dirs() and mount_overlay() is better as it adds more flexibility
I'll recap my suggestions in v1.

> Thanks,
> Murphy

Kind regards,
Petr

[1] http://lists.linux.it/pipermail/ltp/2019-May/011983.html
[2] http://lists.linux.it/pipermail/ltp/2019-May/011979.html

[LTP] [PATCH v3 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Murphy Zhou]

Also defined constraints of needed overlayfs dirs.
setup_overlayfs_files() to create lower/upper/work dirs.
safe_mount_overlayfs() to mount overlayfs.

Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
v3:
  Split setup to 2 parts: creating files and mounting.
  Use macro SAFE_MOUNT_OVERLAYFS.
  Handle ENODEV differently for 4 testcases we have modified.
v2:
  define constraints in header file.
  add a setup helper to create dirs and mount.

 include/safe_file_ops_fn.h  |  5 ++++
 include/tst_fs.h            |  6 ++++
 include/tst_safe_file_ops.h |  3 ++
 lib/tst_fs_setup.c          | 56 +++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+)
 create mode 100644 lib/tst_fs_setup.c

diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
index 35ec4fb1f..2491fa40c 100644
--- a/include/safe_file_ops_fn.h
+++ b/include/safe_file_ops_fn.h
@@ -76,4 +76,9 @@ void safe_touch(const char *file, const int lineno,
 		const char *pathname,
 		mode_t mode, const struct timespec times[2]);
 
+/* helper functions to setup overlayfs mountpoint */
+void setup_overlayfs_files(void);
+
+int safe_mount_overlayfs(const char *file, const int lineno,
+		void (cleanup_fn)(void), int infoflag);
 #endif /* SAFE_FILE_OPS_FN */
diff --git a/include/tst_fs.h b/include/tst_fs.h
index 423ca82ec..ce110b723 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -50,6 +50,12 @@ enum {
 	TST_GB = 1073741824,
 };
 
+#define OVL_BASE_MNTPOINT        "mntpoint"
+#define OVL_LOWER	OVL_BASE_MNTPOINT"/lower"
+#define OVL_UPPER	OVL_BASE_MNTPOINT"/upper"
+#define OVL_WORK	OVL_BASE_MNTPOINT"/work"
+#define OVL_MNT		OVL_BASE_MNTPOINT"/ovl"
+
 /*
  * @path: path is the pathname of any file within the mounted file system
  * @mult: mult should be TST_KB, TST_MB or TST_GB
diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
index 5c3fea4e2..d211b40ff 100644
--- a/include/tst_safe_file_ops.h
+++ b/include/tst_safe_file_ops.h
@@ -59,4 +59,7 @@
 	safe_touch(__FILE__, __LINE__, NULL, \
 			(pathname), (mode), (times))
 
+#define SAFE_MOUNT_OVERLAYFS(flag) \
+	safe_mount_overlayfs(__FILE__, __LINE__, NULL, (flag))
+
 #endif /* TST_SAFE_FILE_OPS */
diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
new file mode 100644
index 000000000..0f28beb61
--- /dev/null
+++ b/lib/tst_fs_setup.c
@@ -0,0 +1,56 @@
+/*
+ * DESCRIPTION
+ *	A place for setup filesystem helpers.
+ */
+
+#include <stdint.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/vfs.h>
+#include <sys/mount.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_fs.h"
+
+void setup_overlayfs_files(void)
+{
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
+	SAFE_MKDIR(OVL_MNT, 0755);
+}
+
+int safe_mount_overlayfs(const char *file, const int lineno,
+		void (cleanup_fn)(void), int infoflag)
+{
+	int ret = 0;
+	char *cfgmsg = "overlayfs is not configured in this kernel.";
+
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
+	if (ret < 0) {
+		if (errno == ENODEV) {
+			/* To keep the original test logic of testcases, use
+			   infoflag to handle ENODEV differently. Like
+			   execveat03 inotify0{7,8} readahead02. */
+			switch (infoflag) {
+			case 1:
+				tst_res(TINFO, cfgmsg);
+				return 1;
+			case 2:
+				tst_brk(TCONF, cfgmsg);
+				break;
+			case 3:
+				tst_brk(TCONF, cfgmsg);
+			default:
+				tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+				break;
+			}
+		} else {
+			tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+		}
+	}
+	return 0;
+}
-- 
2.21.0

[LTP] [PATCH v3 2/2] OVL_MNT: setup overlayfs dirs and mount in a separated mountpoint

[Murphy Zhou]

Some tests are mounting overlayfs internally and run tests on it.
This mount can fail if the filesystem we are running on does not
support overlay mount upon it. For example, we are already running
tests on overlayfs or NFS, or CIFS. Test will report broken and
failure.

Fixing this by put overlayfs dirs in a separated mountpoint, like in
readahead02 by Amir.

Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
 .../kernel/syscalls/execveat/execveat03.c     | 24 +++++--------
 testcases/kernel/syscalls/inotify/inotify07.c | 29 +++++----------
 testcases/kernel/syscalls/inotify/inotify08.c | 31 ++++++----------
 .../kernel/syscalls/readahead/readahead02.c   | 36 ++++---------------
 4 files changed, 34 insertions(+), 86 deletions(-)

diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
index def33923b..7ec038134 100644
--- a/testcases/kernel/syscalls/execveat/execveat03.c
+++ b/testcases/kernel/syscalls/execveat/execveat03.c
@@ -55,10 +55,11 @@
 #include "lapi/fcntl.h"
 #include "execveat.h"
 
-#define OVL_MNT "ovl"
 #define TEST_APP "execveat_child"
 #define TEST_FILE_PATH OVL_MNT"/"TEST_APP
 
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
+
 static int ovl_mounted;
 
 static void do_child(void)
@@ -90,21 +91,10 @@ static void setup(void)
 
 	check_execveat();
 
-	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
+	setup_overlayfs_files();
+	ret = SAFE_MOUNT_OVERLAYFS(3);
+	if (ret == 0)
+		ovl_mounted = 1;
 }
 
 static void cleanup(void)
@@ -121,6 +111,8 @@ static const char *const resource_files[] = {
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.setup = setup,
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index 96370b5cf..c903032cb 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -73,13 +73,13 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define DIR_NAME "test_dir"
 #define DIR_PATH OVL_MNT"/"DIR_NAME
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -164,24 +164,12 @@ static void setup(void)
 	int ret;
 
 	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("lower/"DIR_NAME, 0755);
-	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
-	ovl_mounted = 1;
+	setup_overlayfs_files();
+	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
+	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
+	ret = SAFE_MOUNT_OVERLAYFS(2);
+	if (ret == 0)
+		ovl_mounted = 1;
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
 	if (fd_notify < 0) {
@@ -221,7 +209,6 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
@@ -234,6 +221,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index acdb95345..fb80a55a5 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -74,11 +74,11 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -104,8 +104,8 @@ void verify_inotify(void)
 	test_cnt++;
 
 	/* Make sure events on upper/lower do not show in overlay watch */
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_TOUCH("upper/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
 
 	int len = read(fd_notify, event_buf, EVENT_BUF_LEN);
 	if (len == -1 && errno != EAGAIN) {
@@ -157,23 +157,11 @@ static void setup(void)
 	int ret;
 
 	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
-	ovl_mounted = 1;
+	setup_overlayfs_files();
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	ret = SAFE_MOUNT_OVERLAYFS(2);
+	if (ret == 0)
+		ovl_mounted = 1;
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
 	if (fd_notify < 0) {
@@ -217,7 +205,6 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
@@ -230,6 +217,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 39ddbd583..57251c892 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -44,16 +44,11 @@ static int pagesize;
 static unsigned long cached_max;
 static int ovl_mounted;
 
-#define MNTPOINT        "mntpoint"
-#define OVL_LOWER	MNTPOINT"/lower"
-#define OVL_UPPER	MNTPOINT"/upper"
-#define OVL_WORK	MNTPOINT"/work"
-#define OVL_MNT		MNTPOINT"/ovl"
 static int readahead_length  = 4096;
 static char sys_bdi_ra_path[PATH_MAX];
 static int orig_bdi_limit;
 
-static const char mntpoint[] = MNTPOINT;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct tst_option options[] = {
 	{"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
@@ -132,7 +127,7 @@ static void create_testfile(int use_overlay)
 	char *tmp;
 	size_t i;
 
-	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
+	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : OVL_BASE_MNTPOINT);
 	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
@@ -329,27 +324,6 @@ static void test_readahead(unsigned int n)
 	}
 }
 
-static void setup_overlay(void)
-{
-	int ret;
-
-	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR(OVL_LOWER, 0755);
-	SAFE_MKDIR(OVL_UPPER, 0755);
-	SAFE_MKDIR(OVL_WORK, 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
-		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_res(TINFO,
-				"overlayfs is not configured in this kernel.");
-			return;
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
-}
 
 /*
  * We try raising bdi readahead limit as much as we can. We write
@@ -396,6 +370,7 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
+	int ret;
 	if (opt_fsizestr)
 		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
 
@@ -413,7 +388,10 @@ static void setup(void)
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
 
-	setup_overlay();
+	setup_overlayfs_files();
+	ret = SAFE_MOUNT_OVERLAYFS(1);
+	if (ret == 0)
+		ovl_mounted = 1;
 }
 
 static void cleanup(void)
-- 
2.21.0

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Murphy Zhou]

On Wed, May 15, 2019 at 03:31:02PM +0200, Petr Vorel wrote:
> Hi Murphy,
> 
> > To create overlayfs dirs, and mount overlayfs if needed.
> 
> ...
> > +int setup_overlay(int mountovl)
> > +{
> > +	int ret;
> > +
> > +	/* Setup an overlay mount with lower dir and file */
> > +	SAFE_MKDIR(OVL_LOWER, 0755);
> > +	SAFE_MKDIR(OVL_UPPER, 0755);
> > +	SAFE_MKDIR(OVL_WORK, 0755);
> > +	SAFE_MKDIR(OVL_MNT, 0755);
> > +
> > +	/* Only create dirs, do not mount */
> > +	if (mountovl == 0)
> > +		return 0;
> Instead of having int parameter, there could be create_overlay_dirs()
> and mount_overlay(), which would call create_overlay_dirs().
> (no need to lookup meaning of parameter).
> 
> > +
> > +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > +	if (ret < 0) {
> > +		if (errno == ENODEV) {
> > +			tst_res(TINFO,
> > +				"overlayfs is not configured in this kernel.");
> > +			return 1;
> First I thought we'd implement it as a test flag (member of struct tst_test).
> When we have it as separate function I wonder whether we could TCONF on ENODEV
> instead of TINFO and return. Maybe there could be here for int strict parameter,

The return value is referenced in the testcase to determine whether to run
tests in overlayfs. It's needed.

If this strict parameter is only for different wording on NODEV. Is it
necessary ?

Murphy

> where 1 would be force safe (i.e. TCONF), otherwise only TINFO.
> This could also to have SAFE_MOUNT_OVERLAY() macro which would use mount_overlay().
> Similar approach as SAFE_SEND() and safe_send().
> 
> > +		}
> > +		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
> > +	}
> > +	return 0;
> > +}
> 
> Kind regards,
> Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Petr Vorel]

Hi Murphy,

> > > +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > > +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > > +	if (ret < 0) {
> > > +		if (errno == ENODEV) {
> > > +			tst_res(TINFO,
> > > +				"overlayfs is not configured in this kernel.");
> > > +			return 1;
> > First I thought we'd implement it as a test flag (member of struct tst_test).
> > When we have it as separate function I wonder whether we could TCONF on ENODEV
> > instead of TINFO and return. Maybe there could be here for int strict parameter,

> The return value is referenced in the testcase to determine whether to run
> tests in overlayfs. It's needed.

> If this strict parameter is only for different wording on NODEV. Is it
> necessary ?

I'll recap my suggestions:
1) I like having macros to help reduce some parameters, this does not block
functions being flexible (which requires parameters).
2) Having helper function create_overlay_dirs() used separately, than parameter
in single function (Amir [1] suggestion makes sense).

Something like this, just a suggestion:

int create_overlay_dirs()
{
	SAFE_MKDIR(OVL_LOWER, 0755);
	...
	return ret;
}

int mount_overlay(const char *file, const int lineno, int safe)
{
	...
	if (create_overlay_dirs())
		tst_brk(TCONF, "...");

	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
	if (ret < 0) {
		if (errno == ENODEV) {
			/*
			 * TODO: maybe safe is confusing as we use tst_brk(TBROK anyway),
			 * + sometimes tst_res(TCONF, ..) would be preferred over
			 * tst_brk(TCONF, ...)
			 */
			if (safe)
				tst_brk(TCONF,
					"overlayfs is not configured in this kernel.");
			} else {
				tst_res(TINFO,
					"overlayfs is not configured in this kernel.");
				return 1;
			}
		}
		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
	}
}

#define SAFE_MOUNT_OVERLAY() \
	mount_overlay(__FILE__, __LINE__, 1);

#define TST_MOUNT_OVERLAY() \
	mount_overlay(__FILE__, __LINE__, 0);

Kind regards,
Petr

[1] http://lists.linux.it/pipermail/ltp/2019-May/011983.html

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Amir Goldstein]

On Fri, May 24, 2019 at 11:54 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Murphy,
>
> > > > + ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > > > +             ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > > > + if (ret < 0) {
> > > > +         if (errno == ENODEV) {
> > > > +                 tst_res(TINFO,
> > > > +                         "overlayfs is not configured in this kernel.");
> > > > +                 return 1;
> > > First I thought we'd implement it as a test flag (member of struct tst_test).
> > > When we have it as separate function I wonder whether we could TCONF on ENODEV
> > > instead of TINFO and return. Maybe there could be here for int strict parameter,
>
> > The return value is referenced in the testcase to determine whether to run
> > tests in overlayfs. It's needed.
>
> > If this strict parameter is only for different wording on NODEV. Is it
> > necessary ?
>
> I'll recap my suggestions:
> 1) I like having macros to help reduce some parameters, this does not block
> functions being flexible (which requires parameters).
> 2) Having helper function create_overlay_dirs() used separately, than parameter
> in single function (Amir [1] suggestion makes sense).
>
> Something like this, just a suggestion:
>
> int create_overlay_dirs()
> {
>         SAFE_MKDIR(OVL_LOWER, 0755);
>         ...
>         return ret;
> }
>
> int mount_overlay(const char *file, const int lineno, int safe)
> {
>         ...
>         if (create_overlay_dirs())
>                 tst_brk(TCONF, "...");
>
>         ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
>                     ",upperdir="OVL_UPPER",workdir="OVL_WORK);
>         if (ret < 0) {
>                 if (errno == ENODEV) {
>                         /*
>                          * TODO: maybe safe is confusing as we use tst_brk(TBROK anyway),
>                          * + sometimes tst_res(TCONF, ..) would be preferred over
>                          * tst_brk(TCONF, ...)
>                          */
>                         if (safe)
>                                 tst_brk(TCONF,
>                                         "overlayfs is not configured in this kernel.");
>                         } else {
>                                 tst_res(TINFO,
>                                         "overlayfs is not configured in this kernel.");
>                                 return 1;
>                         }
>                 }
>                 tst_brk(TBROK | TERRNO, "overlayfs mount failed");
>         }
> }
>
> #define SAFE_MOUNT_OVERLAY() \
>         mount_overlay(__FILE__, __LINE__, 1);
>
> #define TST_MOUNT_OVERLAY() \
>         mount_overlay(__FILE__, __LINE__, 0);
>

I like this version of the helpers/macros.

I would change TST_MOUNT_OVERLAY to
(mount_overlay(__FILE__, __LINE__, 0) == 0)
so that it could be used like this:

ovl_mounted = TST_MOUNT_OVERLAY(...)

Uses of SAFE_MOUNT_OVERLAY() should not check return value
could even place (void) in front of mount_overlay to enforce that.

Test that use SAFE_MOUNT_OVERLAY() should either have no
variable ovl_mounted or set it after SAFE_MOUNT_OVERLAY() without
checking return value.

Thanks,
Amir.

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Petr Vorel]

Hi Amir,

thanks for your suggestions, highly appreciated :).
...
> > #define SAFE_MOUNT_OVERLAY() \
> >         mount_overlay(__FILE__, __LINE__, 1);

> > #define TST_MOUNT_OVERLAY() \
> >         mount_overlay(__FILE__, __LINE__, 0);


> I like this version of the helpers/macros.

> I would change TST_MOUNT_OVERLAY to
> (mount_overlay(__FILE__, __LINE__, 0) == 0)
> so that it could be used like this:

> ovl_mounted = TST_MOUNT_OVERLAY(...)
+1

> Uses of SAFE_MOUNT_OVERLAY() should not check return value
> could even place (void) in front of mount_overlay to enforce that.
If we don't care, then it could be?
#define SAFE_MOUNT_OVERLAY() \
	(void) mount_overlay(__FILE__, __LINE__, 1);

I mean there is always mount_overlay() for special cases, macros should be
here to make easier common usage.

> Test that use SAFE_MOUNT_OVERLAY() should either have no
> variable ovl_mounted or set it after SAFE_MOUNT_OVERLAY() without
> checking return value.
+1

> Thanks,
> Amir.

Kind regards,
Petr

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Murphy Zhou]

Hi guys,

Very good ideas! Thank you both very much!

M
On Fri, May 24, 2019 at 02:23:57PM +0200, Petr Vorel wrote:
> Hi Amir,
> 
> thanks for your suggestions, highly appreciated :).
> ...
> > > #define SAFE_MOUNT_OVERLAY() \
> > >         mount_overlay(__FILE__, __LINE__, 1);
> 
> > > #define TST_MOUNT_OVERLAY() \
> > >         mount_overlay(__FILE__, __LINE__, 0);
> 
> 
> > I like this version of the helpers/macros.
> 
> > I would change TST_MOUNT_OVERLAY to
> > (mount_overlay(__FILE__, __LINE__, 0) == 0)
> > so that it could be used like this:
> 
> > ovl_mounted = TST_MOUNT_OVERLAY(...)
> +1
> 
> > Uses of SAFE_MOUNT_OVERLAY() should not check return value
> > could even place (void) in front of mount_overlay to enforce that.
> If we don't care, then it could be?
> #define SAFE_MOUNT_OVERLAY() \
> 	(void) mount_overlay(__FILE__, __LINE__, 1);
> 
> I mean there is always mount_overlay() for special cases, macros should be
> here to make easier common usage.
> 
> > Test that use SAFE_MOUNT_OVERLAY() should either have no
> > variable ovl_mounted or set it after SAFE_MOUNT_OVERLAY() without
> > checking return value.
> +1
> 
> > Thanks,
> > Amir.
> 
> Kind regards,
> Petr

[LTP] [PATCH v4 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Murphy Zhou]

Define constraints of needed overlayfs dirs;
create_overlay_dirs() to create lower/upper/work dirs;
mount_overlay() to mount overlayfs;
SAFE_MOUNT_OVERLAY macro to mount overlayfs safely, tst_brk TCONF
if mount fail with errno ENODEV;
TST_MOUNT_OVERLAY  macro to mount overlayfs and return 0 if succeeds;

Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
v4:
  Update ENODEV handle logic
  Define TST_MOUNT_OVERLAY and SAFE_MOUNT_OVERLAY macros
  Change helper names
v3:
  Split setup to 2 parts: creating files and mounting.
  Use macro SAFE_MOUNT_OVERLAYFS.
  Handle ENODEV differently for 4 testcases we have modified.
v2:
  define constraints in header file.
  add a setup helper to create dirs and mount.

 include/safe_file_ops_fn.h  |  4 ++++
 include/tst_fs.h            |  6 +++++
 include/tst_safe_file_ops.h |  6 +++++
 lib/tst_fs_setup.c          | 46 +++++++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+)
 create mode 100644 lib/tst_fs_setup.c

diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
index 35ec4fb1f..526454da7 100644
--- a/include/safe_file_ops_fn.h
+++ b/include/safe_file_ops_fn.h
@@ -76,4 +76,8 @@ void safe_touch(const char *file, const int lineno,
 		const char *pathname,
 		mode_t mode, const struct timespec times[2]);
 
+/* helper functions to setup overlayfs mountpoint */
+void create_overlay_dirs(void);
+int mount_overlay(const char *file, const int lineno, int safe);
+
 #endif /* SAFE_FILE_OPS_FN */
diff --git a/include/tst_fs.h b/include/tst_fs.h
index 423ca82ec..ce110b723 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -50,6 +50,12 @@ enum {
 	TST_GB = 1073741824,
 };
 
+#define OVL_BASE_MNTPOINT        "mntpoint"
+#define OVL_LOWER	OVL_BASE_MNTPOINT"/lower"
+#define OVL_UPPER	OVL_BASE_MNTPOINT"/upper"
+#define OVL_WORK	OVL_BASE_MNTPOINT"/work"
+#define OVL_MNT		OVL_BASE_MNTPOINT"/ovl"
+
 /*
  * @path: path is the pathname of any file within the mounted file system
  * @mult: mult should be TST_KB, TST_MB or TST_GB
diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
index 5c3fea4e2..b62a7447f 100644
--- a/include/tst_safe_file_ops.h
+++ b/include/tst_safe_file_ops.h
@@ -59,4 +59,10 @@
 	safe_touch(__FILE__, __LINE__, NULL, \
 			(pathname), (mode), (times))
 
+#define SAFE_MOUNT_OVERLAY() \
+	((void) mount_overlay(__FILE__, __LINE__, 1))
+
+#define TST_MOUNT_OVERLAY() \
+	(mount_overlay(__FILE__, __LINE__, 0) == 0)
+
 #endif /* TST_SAFE_FILE_OPS */
diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
new file mode 100644
index 000000000..de09c7135
--- /dev/null
+++ b/lib/tst_fs_setup.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * DESCRIPTION
+ *	A place for setup filesystem helpers.
+ */
+
+#include <stdint.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/vfs.h>
+#include <sys/mount.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_fs.h"
+
+void create_overlay_dirs(void)
+{
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
+	SAFE_MKDIR(OVL_MNT, 0755);
+}
+
+int mount_overlay(const char *file, const int lineno, int safe)
+{
+	int ret = 0;
+	char *cfgmsg = "overlayfs is not configured in this kernel.";
+
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
+	if (ret < 0) {
+		if (errno == ENODEV) {
+			if (safe) {
+				tst_brk(TCONF, cfgmsg);
+			} else {
+				tst_res(TINFO, cfgmsg);
+				return 1;
+			}
+		} else {
+			tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+		}
+	}
+	return 0;
+}
-- 
2.21.0

[LTP] [PATCH v4 2/2] OVL_MNT: setup overlayfs dirs and mount in a separated mountpoint

[Murphy Zhou]

Some tests are mounting overlayfs internally and run tests on it.
This mount can fail if the filesystem we are running on does not
support overlay mount upon it. For example, we are already running
tests on overlayfs or NFS, or CIFS. Test will report broken and
failure.

Fixing this by put overlayfs dirs in a separated mountpoint, like in
readahead02 by Amir.

Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
 .../kernel/syscalls/execveat/execveat03.c     | 23 ++++---------
 testcases/kernel/syscalls/inotify/inotify07.c | 27 ++++-----------
 testcases/kernel/syscalls/inotify/inotify08.c | 29 +++++-----------
 .../kernel/syscalls/readahead/readahead02.c   | 34 +++----------------
 4 files changed, 26 insertions(+), 87 deletions(-)

diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
index def33923b..446ff4f1d 100644
--- a/testcases/kernel/syscalls/execveat/execveat03.c
+++ b/testcases/kernel/syscalls/execveat/execveat03.c
@@ -55,10 +55,11 @@
 #include "lapi/fcntl.h"
 #include "execveat.h"
 
-#define OVL_MNT "ovl"
 #define TEST_APP "execveat_child"
 #define TEST_FILE_PATH OVL_MNT"/"TEST_APP
 
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
+
 static int ovl_mounted;
 
 static void do_child(void)
@@ -86,24 +87,10 @@ static void verify_execveat(void)
 
 static void setup(void)
 {
-	int ret;
-
 	check_execveat();
 
-	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
+	create_overlay_dirs();
+	SAFE_MOUNT_OVERLAY();
 	ovl_mounted = 1;
 }
 
@@ -121,6 +108,8 @@ static const char *const resource_files[] = {
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.setup = setup,
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index 96370b5cf..7d6aba51f 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -73,13 +73,13 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define DIR_NAME "test_dir"
 #define DIR_PATH OVL_MNT"/"DIR_NAME
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -161,26 +161,12 @@ void verify_inotify(void)
 static void setup(void)
 {
 	struct stat buf;
-	int ret;
 
 	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("lower/"DIR_NAME, 0755);
-	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
+	create_overlay_dirs();
+	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
+	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
+	SAFE_MOUNT_OVERLAY();
 	ovl_mounted = 1;
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
@@ -221,7 +207,6 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
@@ -234,6 +219,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index acdb95345..929ed08f1 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -74,11 +74,11 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"FILE_NAME
 
 static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -104,8 +104,8 @@ void verify_inotify(void)
 	test_cnt++;
 
 	/* Make sure events on upper/lower do not show in overlay watch */
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_TOUCH("upper/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
 
 	int len = read(fd_notify, event_buf, EVENT_BUF_LEN);
 	if (len == -1 && errno != EAGAIN) {
@@ -154,25 +154,11 @@ void verify_inotify(void)
 static void setup(void)
 {
 	struct stat buf;
-	int ret;
 
 	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
+	create_overlay_dirs();
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_MOUNT_OVERLAY();
 	ovl_mounted = 1;
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
@@ -217,7 +203,6 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
@@ -230,6 +215,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 39ddbd583..db95c2b01 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -44,16 +44,11 @@ static int pagesize;
 static unsigned long cached_max;
 static int ovl_mounted;
 
-#define MNTPOINT        "mntpoint"
-#define OVL_LOWER	MNTPOINT"/lower"
-#define OVL_UPPER	MNTPOINT"/upper"
-#define OVL_WORK	MNTPOINT"/work"
-#define OVL_MNT		MNTPOINT"/ovl"
 static int readahead_length  = 4096;
 static char sys_bdi_ra_path[PATH_MAX];
 static int orig_bdi_limit;
 
-static const char mntpoint[] = MNTPOINT;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct tst_option options[] = {
 	{"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
@@ -132,7 +127,8 @@ static void create_testfile(int use_overlay)
 	char *tmp;
 	size_t i;
 
-	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
+	sprintf(testfile, "%s/testfile",
+		use_overlay ? OVL_MNT : OVL_BASE_MNTPOINT);
 	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
@@ -329,27 +325,6 @@ static void test_readahead(unsigned int n)
 	}
 }
 
-static void setup_overlay(void)
-{
-	int ret;
-
-	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR(OVL_LOWER, 0755);
-	SAFE_MKDIR(OVL_UPPER, 0755);
-	SAFE_MKDIR(OVL_WORK, 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
-		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_res(TINFO,
-				"overlayfs is not configured in this kernel.");
-			return;
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
-}
 
 /*
  * We try raising bdi readahead limit as much as we can. We write
@@ -413,7 +388,8 @@ static void setup(void)
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
 
-	setup_overlay();
+	create_overlay_dirs();
+	ovl_mounted = TST_MOUNT_OVERLAY();
 }
 
 static void cleanup(void)
-- 
2.21.0

[LTP] [PATCH v4 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Petr Vorel]

Hi Murphy, Amir, Cyril,

just a suggestion to apply diff below:

1) mount_overlay(): you don't actually use lineno and safe in error messages
tst_fs_setup.c: In function ‘mount_overlay’:
tst_fs_setup.c:26:31: warning: unused parameter ‘file’ [-Wunused-parameter]
 int mount_overlay(const char *file, const int lineno, int safe)
                   ~~~~~~~~~~~~^~~~
tst_fs_setup.c:26:47: warning: unused parameter ‘lineno’ [-Wunused-parameter]
 int mount_overlay(const char *file, const int lineno, int safe)

2) mount_overlay(): return ret from mount (usually -1) instead of 1 (in case
anybody is interested).

3) mount_overlay(): earlier return 0 after checking ENODEV saves us one shift
right (but might look as error, as tst_res() is usually followed by return).

4) create_overlay_dirs(): checks for OVL_LOWER and thus can be called in
mount_overlay() without any check. This saves us calling it before
{SAFE,TST}_MOUNT_OVERLAY() in execveat03.c and readahead02.c

5) removed unused headers (<stdint.h>, <stdio.h>, <stdlib.h>, <sys/vfs.h>),
is any of them needed and the need masked by it's include in tst_test.h?

5) other cleanup

TODO:
I'm still not sure about ovl_mounted. There is static int mntpoint_mounted in
lib/tst_test.c, which does umount.  tst_test->mntpoint, I guess we should use
it. WDYT?

Kind regards,
Petr
---
* diff to first commit:
diff --git lib/tst_fs_setup.c lib/tst_fs_setup.c
index de09c7135..60dedc283 100644
--- lib/tst_fs_setup.c
+++ lib/tst_fs_setup.c
@@ -1,46 +1,46 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * DESCRIPTION
- *	A place for setup filesystem helpers.
- */
 
-#include <stdint.h>
+#include <dirent.h>
 #include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/vfs.h>
 #include <sys/mount.h>
 
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
 #include "tst_fs.h"
 
+#define TST_FS_SETUP_OVERLAYFS_MSG "overlayfs is not configured in this kernel"
+#define TST_FS_SETUP_OVERLAYFS_CONFIG "lowerdir="OVL_LOWER",upperdir="OVL_UPPER",workdir="OVL_WORK
+
 void create_overlay_dirs(void)
 {
-	SAFE_MKDIR(OVL_LOWER, 0755);
-	SAFE_MKDIR(OVL_UPPER, 0755);
-	SAFE_MKDIR(OVL_WORK, 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
+	DIR* dir = opendir(OVL_BASE_MNTPOINT);
+	if (!dir) {
+		SAFE_MKDIR(OVL_LOWER, 0755);
+		SAFE_MKDIR(OVL_UPPER, 0755);
+		SAFE_MKDIR(OVL_WORK, 0755);
+		SAFE_MKDIR(OVL_MNT, 0755);
+	}
 }
 
 int mount_overlay(const char *file, const int lineno, int safe)
 {
-	int ret = 0;
-	char *cfgmsg = "overlayfs is not configured in this kernel.";
-
-	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
-		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			if (safe) {
-				tst_brk(TCONF, cfgmsg);
-			} else {
-				tst_res(TINFO, cfgmsg);
-				return 1;
-			}
+	int ret;
+
+	create_overlay_dirs();
+
+	ret = mount("overlay", OVL_MNT, "overlay", 0, TST_FS_SETUP_OVERLAYFS_CONFIG);
+
+	if (!ret)
+		return 0;
+
+	if (errno == ENODEV) {
+		if (safe) {
+			tst_brk(TCONF, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG, file, lineno);
 		} else {
-			tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+			tst_res(TINFO, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG, file, lineno);
 		}
+	} else {
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
 	}
-	return 0;
+	return ret;
 }

* diff to second commit:
diff --git testcases/kernel/syscalls/execveat/execveat03.c testcases/kernel/syscalls/execveat/execveat03.c
index 446ff4f1d..eb088bc4c 100644
--- testcases/kernel/syscalls/execveat/execveat03.c
+++ testcases/kernel/syscalls/execveat/execveat03.c
@@ -88,8 +88,6 @@ static void verify_execveat(void)
 static void setup(void)
 {
 	check_execveat();
-
-	create_overlay_dirs();
 	SAFE_MOUNT_OVERLAY();
 	ovl_mounted = 1;
 }
diff --git testcases/kernel/syscalls/inotify/inotify08.c testcases/kernel/syscalls/inotify/inotify08.c
index 929ed08f1..9433315e2 100644
--- testcases/kernel/syscalls/inotify/inotify08.c
+++ testcases/kernel/syscalls/inotify/inotify08.c
@@ -165,7 +165,7 @@ static void setup(void)
 	if (fd_notify < 0) {
 		if (errno == ENOSYS) {
 			tst_brk(TCONF,
-				"inotify is not configured in this kernel.");
+				"inotify is not configured in this kernel");
 		} else {
 			tst_brk(TBROK | TERRNO,
 				"inotify_init () failed");
diff --git testcases/kernel/syscalls/readahead/readahead02.c testcases/kernel/syscalls/readahead/readahead02.c
index db95c2b01..f6e07173d 100644
--- testcases/kernel/syscalls/readahead/readahead02.c
+++ testcases/kernel/syscalls/readahead/readahead02.c
@@ -388,7 +388,6 @@ static void setup(void)
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
 
-	create_overlay_dirs();
 	ovl_mounted = TST_MOUNT_OVERLAY();
 }

[LTP] [PATCH v4 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Amir Goldstein]

On Mon, May 27, 2019 at 3:09 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Murphy, Amir, Cyril,
>
> just a suggestion to apply diff below:
>
> 1) mount_overlay(): you don't actually use lineno and safe in error messages
> tst_fs_setup.c: In function ‘mount_overlay’:
> tst_fs_setup.c:26:31: warning: unused parameter ‘file’ [-Wunused-parameter]
>  int mount_overlay(const char *file, const int lineno, int safe)
>                    ~~~~~~~~~~~~^~~~
> tst_fs_setup.c:26:47: warning: unused parameter ‘lineno’ [-Wunused-parameter]
>  int mount_overlay(const char *file, const int lineno, int safe)
>
> 2) mount_overlay(): return ret from mount (usually -1) instead of 1 (in case
> anybody is interested).
>
> 3) mount_overlay(): earlier return 0 after checking ENODEV saves us one shift
> right (but might look as error, as tst_res() is usually followed by return).
>
> 4) create_overlay_dirs(): checks for OVL_LOWER and thus can be called in
> mount_overlay() without any check. This saves us calling it before
> {SAFE,TST}_MOUNT_OVERLAY() in execveat03.c and readahead02.c
>
> 5) removed unused headers (<stdint.h>, <stdio.h>, <stdlib.h>, <sys/vfs.h>),
> is any of them needed and the need masked by it's include in tst_test.h?
>

The changes above look fine to me

> 5) other cleanup
>
> TODO:
> I'm still not sure about ovl_mounted. There is static int mntpoint_mounted in
> lib/tst_test.c, which does umount.  tst_test->mntpoint, I guess we should use
> it. WDYT?

It's not exactly the same as mntpoint_mounted.
In readahead02 ovl_mounted is used to decide whether to
run test only on base fs or on base fs and also on overlayfs.
Or maybe I did not understand what you mean.
For other tests ovl_moutned is only used for cleanup and could
probably be replaced with mntpoint_mounted.

Thanks,
Amir.

[LTP] [PATCH v4 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Petr Vorel]

Hi Murphy, Amir,

thanks a lot for your answers.

> > 5) other cleanup

> > TODO:
> > I'm still not sure about ovl_mounted. There is static int mntpoint_mounted in
> > lib/tst_test.c, which does umount.  tst_test->mntpoint, I guess we should use
> > it. WDYT?

> It's not exactly the same as mntpoint_mounted.
> In readahead02 ovl_mounted is used to decide whether to
> run test only on base fs or on base fs and also on overlayfs.
> Or maybe I did not understand what you mean.
> For other tests ovl_moutned is only used for cleanup and could
> probably be replaced with mntpoint_mounted.
Indeed, sorry for confusion. readahead02.c uses .mount_device = 1 flag, which is
then umounted in do_setup() in tst_test.c.
I was still playing with idea having some some other flag for overlay which
would be meant for simple use cases (execveat03.c, inotify0[78].c,
execveat03.c). With this flag library would call SAFE_MOUNT_OVERLAY() in
do_setup() and manage ovl_mounted, doing SAFE_UMOUNT in do_cleanup() (same
approach as .mount_device and mntpoint_mounted). The idea was already mentioned
by Amir. There still would be {SAFE,TST}_MOUNT_OVERLAY() giving a freedom to do
more complicated things (readahead02.c).

Just a suggestion, sorry for complicating things.

> Thanks,
> Amir.

Kind regards,
Petr

[LTP] [PATCH v4 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Murphy Zhou]

Hi Petr,

On Mon, May 27, 2019 at 02:09:45PM +0200, Petr Vorel wrote:
> Hi Murphy, Amir, Cyril,
> 
> just a suggestion to apply diff below:
> 
> 1) mount_overlay(): you don't actually use lineno and safe in error messages
> tst_fs_setup.c: In function ‘mount_overlay’:
> tst_fs_setup.c:26:31: warning: unused parameter ‘file’ [-Wunused-parameter]
>  int mount_overlay(const char *file, const int lineno, int safe)
>                    ~~~~~~~~~~~~^~~~
> tst_fs_setup.c:26:47: warning: unused parameter ‘lineno’ [-Wunused-parameter]
>  int mount_overlay(const char *file, const int lineno, int safe)

Yes, it's good to use them in the messages.

> 
> 2) mount_overlay(): return ret from mount (usually -1) instead of 1 (in case
> anybody is interested).

Agree.
> 
> 3) mount_overlay(): earlier return 0 after checking ENODEV saves us one shift
> right (but might look as error, as tst_res() is usually followed by return).

Brilliant!

> 
> 4) create_overlay_dirs(): checks for OVL_LOWER and thus can be called in
> mount_overlay() without any check. This saves us calling it before
> {SAFE,TST}_MOUNT_OVERLAY() in execveat03.c and readahead02.c
> 
> 5) removed unused headers (<stdint.h>, <stdio.h>, <stdlib.h>, <sys/vfs.h>),
> is any of them needed and the need masked by it's include in tst_test.h?
> 
> 5) other cleanup

All agree.
> 
> TODO:
> I'm still not sure about ovl_mounted. There is static int mntpoint_mounted in
> lib/tst_test.c, which does umount.  tst_test->mntpoint, I guess we should use
> it. WDYT?

mntpoint_mounted is tracking mount status of a separated mountpoint which
is the base for creating overlay dirs and overlay mountpoint. This separated
mountpoint is setup from scratch, grab dev, mfks etc. It's separated from
runltp -d DIR. This is why this patch is needed.

Overlayfs is mounted on this separated mountpoint. Testcases need to umount
overlayfs in cleanup if setup overlay successfully. That's why ovl_mounted
is needed.

How about SAFE_UMOUNT_OVERLAY ignoring EINVAL ?

Thanks!
M


> 
> Kind regards,
> Petr
> ---
> * diff to first commit:
> diff --git lib/tst_fs_setup.c lib/tst_fs_setup.c
> index de09c7135..60dedc283 100644
> --- lib/tst_fs_setup.c
> +++ lib/tst_fs_setup.c
> @@ -1,46 +1,46 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * DESCRIPTION
> - *	A place for setup filesystem helpers.
> - */
>  
> -#include <stdint.h>
> +#include <dirent.h>
>  #include <errno.h>
> -#include <stdio.h>
> -#include <stdlib.h>
> -#include <sys/vfs.h>
>  #include <sys/mount.h>
>  
>  #define TST_NO_DEFAULT_MAIN
>  #include "tst_test.h"
>  #include "tst_fs.h"
>  
> +#define TST_FS_SETUP_OVERLAYFS_MSG "overlayfs is not configured in this kernel"
> +#define TST_FS_SETUP_OVERLAYFS_CONFIG "lowerdir="OVL_LOWER",upperdir="OVL_UPPER",workdir="OVL_WORK
> +
>  void create_overlay_dirs(void)
>  {
> -	SAFE_MKDIR(OVL_LOWER, 0755);
> -	SAFE_MKDIR(OVL_UPPER, 0755);
> -	SAFE_MKDIR(OVL_WORK, 0755);
> -	SAFE_MKDIR(OVL_MNT, 0755);
> +	DIR* dir = opendir(OVL_BASE_MNTPOINT);
> +	if (!dir) {
> +		SAFE_MKDIR(OVL_LOWER, 0755);
> +		SAFE_MKDIR(OVL_UPPER, 0755);
> +		SAFE_MKDIR(OVL_WORK, 0755);
> +		SAFE_MKDIR(OVL_MNT, 0755);
> +	}
>  }
>  
>  int mount_overlay(const char *file, const int lineno, int safe)
>  {
> -	int ret = 0;
> -	char *cfgmsg = "overlayfs is not configured in this kernel.";
> -
> -	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> -		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> -	if (ret < 0) {
> -		if (errno == ENODEV) {
> -			if (safe) {
> -				tst_brk(TCONF, cfgmsg);
> -			} else {
> -				tst_res(TINFO, cfgmsg);
> -				return 1;
> -			}
> +	int ret;
> +
> +	create_overlay_dirs();
> +
> +	ret = mount("overlay", OVL_MNT, "overlay", 0, TST_FS_SETUP_OVERLAYFS_CONFIG);
> +
> +	if (!ret)
> +		return 0;
> +
> +	if (errno == ENODEV) {
> +		if (safe) {
> +			tst_brk(TCONF, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG, file, lineno);
>  		} else {
> -			tst_brk(TBROK | TERRNO, "overlayfs mount failed");
> +			tst_res(TINFO, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG, file, lineno);
>  		}
> +	} else {
> +		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
>  	}
> -	return 0;
> +	return ret;
>  }
> 
> * diff to second commit:
> diff --git testcases/kernel/syscalls/execveat/execveat03.c testcases/kernel/syscalls/execveat/execveat03.c
> index 446ff4f1d..eb088bc4c 100644
> --- testcases/kernel/syscalls/execveat/execveat03.c
> +++ testcases/kernel/syscalls/execveat/execveat03.c
> @@ -88,8 +88,6 @@ static void verify_execveat(void)
>  static void setup(void)
>  {
>  	check_execveat();
> -
> -	create_overlay_dirs();
>  	SAFE_MOUNT_OVERLAY();
>  	ovl_mounted = 1;
>  }
> diff --git testcases/kernel/syscalls/inotify/inotify08.c testcases/kernel/syscalls/inotify/inotify08.c
> index 929ed08f1..9433315e2 100644
> --- testcases/kernel/syscalls/inotify/inotify08.c
> +++ testcases/kernel/syscalls/inotify/inotify08.c
> @@ -165,7 +165,7 @@ static void setup(void)
>  	if (fd_notify < 0) {
>  		if (errno == ENOSYS) {
>  			tst_brk(TCONF,
> -				"inotify is not configured in this kernel.");
> +				"inotify is not configured in this kernel");
>  		} else {
>  			tst_brk(TBROK | TERRNO,
>  				"inotify_init () failed");
> diff --git testcases/kernel/syscalls/readahead/readahead02.c testcases/kernel/syscalls/readahead/readahead02.c
> index db95c2b01..f6e07173d 100644
> --- testcases/kernel/syscalls/readahead/readahead02.c
> +++ testcases/kernel/syscalls/readahead/readahead02.c
> @@ -388,7 +388,6 @@ static void setup(void)
>  	setup_readahead_length();
>  	tst_res(TINFO, "readahead length: %d", readahead_length);
>  
> -	create_overlay_dirs();
>  	ovl_mounted = TST_MOUNT_OVERLAY();
>  }

[LTP] [PATCH v4 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Petr Vorel]

Hi Amir, Murphy,

> > TODO:
> > I'm still not sure about ovl_mounted. There is static int mntpoint_mounted in
> > lib/tst_test.c, which does umount.  tst_test->mntpoint, I guess we should use
> > it. WDYT?

> mntpoint_mounted is tracking mount status of a separated mountpoint which
> is the base for creating overlay dirs and overlay mountpoint. This separated
> mountpoint is setup from scratch, grab dev, mfks etc. It's separated from
> runltp -d DIR. This is why this patch is needed.

> Overlayfs is mounted on this separated mountpoint. Testcases need to umount
> overlayfs in cleanup if setup overlay successfully. That's why ovl_mounted
> is needed.

> How about SAFE_UMOUNT_OVERLAY ignoring EINVAL ?
I don't see much benefits, when we have SAFE_UMOUNT().  More useful looks to me
for simple cases move ovl_mounted and SAFE_UMOUNT(OVL_MNT)  to library (the only
thing needed would be some flag for struct tst_test e.g. .mount_overlay = 1).

> Thanks!
> M


Kind regards,
Petr

[LTP] [PATCH v4 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Murphy Zhou]

On Mon, May 27, 2019 at 05:36:32PM +0200, Petr Vorel wrote:
> Hi Amir, Murphy,
> 
> > > TODO:
> > > I'm still not sure about ovl_mounted. There is static int mntpoint_mounted in
> > > lib/tst_test.c, which does umount.  tst_test->mntpoint, I guess we should use
> > > it. WDYT?
> 
> > mntpoint_mounted is tracking mount status of a separated mountpoint which
> > is the base for creating overlay dirs and overlay mountpoint. This separated
> > mountpoint is setup from scratch, grab dev, mfks etc. It's separated from
> > runltp -d DIR. This is why this patch is needed.
> 
> > Overlayfs is mounted on this separated mountpoint. Testcases need to umount
> > overlayfs in cleanup if setup overlay successfully. That's why ovl_mounted
> > is needed.
> 
> > How about SAFE_UMOUNT_OVERLAY ignoring EINVAL ?
> I don't see much benefits, when we have SAFE_UMOUNT().  More useful looks to me
> for simple cases move ovl_mounted and SAFE_UMOUNT(OVL_MNT)  to library (the only
> thing needed would be some flag for struct tst_test e.g. .mount_overlay = 1).

After some digging, I think putting ovl_mounted and UMOUNT to library is
good but creating dirs and MOUNT in library benefits less. Because splitting
creating dirs and MOUNT was intended to be more flexible on this. Also,
inotify07/8 needs to create extra dirs before mounting. So execveat03 is
the only one case to benefit from creating dirs and MOUNT in the library.

Thanks!

> 
> > Thanks!
> > M
> 
> 
> Kind regards,
> Petr

[LTP] [PATCH v5 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Murphy Zhou]

Introduce tst_test->needs_overlay;
Define constraints of needed overlayfs dirs;
create_overlay_dirs() to create lower/upper/work dirs;
mount_overlay() to mount overlayfs;
SAFE_MOUNT_OVERLAY macro to mount overlayfs safely, tst_brk TCONF
if mount fail with errno ENODEV;
TST_MOUNT_OVERLAY  macro to mount overlayfs and return 0 if succeeds;

Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
v5:
  Introduce tst_test->needs_overlay to mount and umount overlay
  Several other fixes suggested by Petr
  Update execveat03, inotify07/8 to use needs_overlay
v4:
  Update ENODEV handle logic
  Define TST_MOUNT_OVERLAY and SAFE_MOUNT_OVERLAY macros
  Change helper names
v3:
  Split setup to 2 parts: creating files and mounting.
  Use macro SAFE_MOUNT_OVERLAYFS.
  Handle ENODEV differently for 4 testcases we have modified.
v2:
  define constraints in header file.
  add a setup helper to create dirs and mount.
 include/safe_file_ops_fn.h  |  4 ++++
 include/tst_fs.h            |  6 +++++
 include/tst_safe_file_ops.h |  6 +++++
 include/tst_test.h          |  1 +
 lib/tst_fs_setup.c          | 48 +++++++++++++++++++++++++++++++++++++
 lib/tst_test.c              | 15 ++++++++++++
 6 files changed, 80 insertions(+)
 create mode 100644 lib/tst_fs_setup.c

diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
index 35ec4fb1f..052fb1b9a 100644
--- a/include/safe_file_ops_fn.h
+++ b/include/safe_file_ops_fn.h
@@ -76,4 +76,8 @@ void safe_touch(const char *file, const int lineno,
 		const char *pathname,
 		mode_t mode, const struct timespec times[2]);
 
+/* helper functions to setup overlayfs mountpoint */
+void create_overlay_dirs(void);
+int mount_overlay(const char *file, const int lineno, int skip);
+
 #endif /* SAFE_FILE_OPS_FN */
diff --git a/include/tst_fs.h b/include/tst_fs.h
index b2b19ada6..ebca065c6 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -50,6 +50,12 @@ enum {
 	TST_GB = 1073741824,
 };
 
+#define OVL_BASE_MNTPOINT        "mntpoint"
+#define OVL_LOWER	OVL_BASE_MNTPOINT"/lower"
+#define OVL_UPPER	OVL_BASE_MNTPOINT"/upper"
+#define OVL_WORK	OVL_BASE_MNTPOINT"/work"
+#define OVL_MNT		OVL_BASE_MNTPOINT"/ovl"
+
 /*
  * @path: path is the pathname of any file within the mounted file system
  * @mult: mult should be TST_KB, TST_MB or TST_GB
diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
index 5c3fea4e2..b62a7447f 100644
--- a/include/tst_safe_file_ops.h
+++ b/include/tst_safe_file_ops.h
@@ -59,4 +59,10 @@
 	safe_touch(__FILE__, __LINE__, NULL, \
 			(pathname), (mode), (times))
 
+#define SAFE_MOUNT_OVERLAY() \
+	((void) mount_overlay(__FILE__, __LINE__, 1))
+
+#define TST_MOUNT_OVERLAY() \
+	(mount_overlay(__FILE__, __LINE__, 0) == 0)
+
 #endif /* TST_SAFE_FILE_OPS */
diff --git a/include/tst_test.h b/include/tst_test.h
index e4b935c45..f3b8901a2 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -121,6 +121,7 @@ struct tst_test {
 	int needs_root:1;
 	int forks_child:1;
 	int needs_device:1;
+	int needs_overlay:1;
 	int needs_checkpoints:1;
 	int format_device:1;
 	int mount_device:1;
diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
new file mode 100644
index 000000000..e8ae929d7
--- /dev/null
+++ b/lib/tst_fs_setup.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <dirent.h>
+#include <errno.h>
+#include <sys/mount.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_fs.h"
+
+#define TST_FS_SETUP_OVERLAYFS_MSG "overlayfs is not configured in this kernel"
+#define TST_FS_SETUP_OVERLAYFS_CONFIG "lowerdir="OVL_LOWER",upperdir="OVL_UPPER",workdir="OVL_WORK
+
+void create_overlay_dirs(void)
+{
+	DIR *dir = opendir(OVL_LOWER);
+	if (dir == NULL) {
+		SAFE_MKDIR(OVL_LOWER, 0755);
+		SAFE_MKDIR(OVL_UPPER, 0755);
+		SAFE_MKDIR(OVL_WORK, 0755);
+		SAFE_MKDIR(OVL_MNT, 0755);
+	}
+	closedir(dir);
+}
+
+int mount_overlay(const char *file, const int lineno, int skip)
+{
+	int ret = 0;
+
+	create_overlay_dirs();
+	ret = mount("overlay", OVL_MNT, "overlay", 0,
+				TST_FS_SETUP_OVERLAYFS_CONFIG);
+	if (ret == 0)
+		return 0;
+
+	if (errno == ENODEV) {
+		if (skip) {
+			tst_brk(TCONF, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG,
+				file, lineno);
+		} else {
+			tst_res(TINFO, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG,
+				file, lineno);
+		}
+	} else {
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+	}
+	return ret;
+}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2d88adbd7..95f389d2e 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -50,6 +50,7 @@ static int iterations = 1;
 static float duration = -1;
 static pid_t main_pid, lib_pid;
 static int mntpoint_mounted;
+static int ovl_mounted;
 static struct timespec tst_start_time; /* valid only for test pid */
 
 struct results {
@@ -871,6 +872,17 @@ static void do_setup(int argc, char *argv[])
 			prepare_device();
 	}
 
+	if (tst_test->needs_overlay && !tst_test->mount_device) {
+		tst_brk(TBROK, "tst_test->mount_device must be set");
+	}
+	if (tst_test->needs_overlay && !mntpoint_mounted) {
+		tst_brk(TBROK, "tst_test->mntpoint must be mounted");
+	}
+	if (tst_test->needs_overlay && !ovl_mounted) {
+		SAFE_MOUNT_OVERLAY();
+		ovl_mounted = 1;
+	}
+
 	if (tst_test->resource_files)
 		copy_resources();
 
@@ -891,6 +903,9 @@ static void do_test_setup(void)
 
 static void do_cleanup(void)
 {
+	if (ovl_mounted)
+		SAFE_UMOUNT(OVL_MNT);
+
 	if (mntpoint_mounted)
 		tst_umount(tst_test->mntpoint);
 
-- 
2.21.0

[LTP] [PATCH v5 2/2] OVL_MNT: setup overlayfs dirs and mount in a separated mountpoint

[Murphy Zhou]

Some tests are mounting overlayfs internally and run tests on it.
This mount can fail if the filesystem we are running on does not
support overlay mount upon it. For example, we are already running
tests on overlayfs or NFS, or CIFS. Test will report broken and
failure.

Fixing this by put overlayfs dirs in a separated mountpoint, like in
readahead02 by Amir.

Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
 .../kernel/syscalls/execveat/execveat03.c     | 31 ++--------------
 testcases/kernel/syscalls/inotify/inotify07.c | 33 ++++-------------
 testcases/kernel/syscalls/inotify/inotify08.c | 37 +++++--------------
 .../kernel/syscalls/readahead/readahead02.c   | 33 ++---------------
 4 files changed, 26 insertions(+), 108 deletions(-)

diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
index def33923b..7df1b0faa 100644
--- a/testcases/kernel/syscalls/execveat/execveat03.c
+++ b/testcases/kernel/syscalls/execveat/execveat03.c
@@ -55,11 +55,10 @@
 #include "lapi/fcntl.h"
 #include "execveat.h"
 
-#define OVL_MNT "ovl"
 #define TEST_APP "execveat_child"
 #define TEST_FILE_PATH OVL_MNT"/"TEST_APP
 
-static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static void do_child(void)
 {
@@ -86,31 +85,7 @@ static void verify_execveat(void)
 
 static void setup(void)
 {
-	int ret;
-
 	check_execveat();
-
-	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
-}
-
-static void cleanup(void)
-{
-	if (ovl_mounted)
-		SAFE_UMOUNT(OVL_MNT);
 }
 
 static const char *const resource_files[] = {
@@ -121,10 +96,12 @@ static const char *const resource_files[] = {
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.needs_overlay = 1,
+	.mntpoint = mntpoint,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.setup = setup,
-	.cleanup = cleanup,
 	.test_all = verify_execveat,
 	.resource_files = resource_files,
 };
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index 96370b5cf..47817cd42 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -73,13 +73,12 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define DIR_NAME "test_dir"
 #define DIR_PATH OVL_MNT"/"DIR_NAME
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
 
-static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -161,27 +160,12 @@ void verify_inotify(void)
 static void setup(void)
 {
 	struct stat buf;
-	int ret;
 
 	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("lower/"DIR_NAME, 0755);
-	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
-	ovl_mounted = 1;
+	SAFE_UMOUNT(OVL_MNT);
+	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
+	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
+	SAFE_MOUNT_OVERLAY();
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
 	if (fd_notify < 0) {
@@ -221,19 +205,18 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
 		SAFE_CLOSE(fd_notify);
-
-	if (ovl_mounted)
-		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.needs_overlay = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index acdb95345..067c01dbb 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -74,11 +74,10 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"FILE_NAME
 
-static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -104,8 +103,8 @@ void verify_inotify(void)
 	test_cnt++;
 
 	/* Make sure events on upper/lower do not show in overlay watch */
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_TOUCH("upper/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_UPPER"/"FILE_NAME, 0644, NULL);
 
 	int len = read(fd_notify, event_buf, EVENT_BUF_LEN);
 	if (len == -1 && errno != EAGAIN) {
@@ -154,32 +153,17 @@ void verify_inotify(void)
 static void setup(void)
 {
 	struct stat buf;
-	int ret;
 
 	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
-	ovl_mounted = 1;
+	SAFE_UMOUNT(OVL_MNT);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_MOUNT_OVERLAY();
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
 	if (fd_notify < 0) {
 		if (errno == ENOSYS) {
 			tst_brk(TCONF,
-				"inotify is not configured in this kernel.");
+				"inotify is not configured in this kernel");
 		} else {
 			tst_brk(TBROK | TERRNO,
 				"inotify_init () failed");
@@ -217,19 +201,18 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
 		SAFE_CLOSE(fd_notify);
-
-	if (ovl_mounted)
-		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.needs_overlay = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 39ddbd583..f6e07173d 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -44,16 +44,11 @@ static int pagesize;
 static unsigned long cached_max;
 static int ovl_mounted;
 
-#define MNTPOINT        "mntpoint"
-#define OVL_LOWER	MNTPOINT"/lower"
-#define OVL_UPPER	MNTPOINT"/upper"
-#define OVL_WORK	MNTPOINT"/work"
-#define OVL_MNT		MNTPOINT"/ovl"
 static int readahead_length  = 4096;
 static char sys_bdi_ra_path[PATH_MAX];
 static int orig_bdi_limit;
 
-static const char mntpoint[] = MNTPOINT;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct tst_option options[] = {
 	{"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
@@ -132,7 +127,8 @@ static void create_testfile(int use_overlay)
 	char *tmp;
 	size_t i;
 
-	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
+	sprintf(testfile, "%s/testfile",
+		use_overlay ? OVL_MNT : OVL_BASE_MNTPOINT);
 	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
@@ -329,27 +325,6 @@ static void test_readahead(unsigned int n)
 	}
 }
 
-static void setup_overlay(void)
-{
-	int ret;
-
-	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR(OVL_LOWER, 0755);
-	SAFE_MKDIR(OVL_UPPER, 0755);
-	SAFE_MKDIR(OVL_WORK, 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
-		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_res(TINFO,
-				"overlayfs is not configured in this kernel.");
-			return;
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
-}
 
 /*
  * We try raising bdi readahead limit as much as we can. We write
@@ -413,7 +388,7 @@ static void setup(void)
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
 
-	setup_overlay();
+	ovl_mounted = TST_MOUNT_OVERLAY();
 }
 
 static void cleanup(void)
-- 
2.21.0

[LTP] [PATCH v5 2/2] OVL_MNT: setup overlayfs dirs and mount in a separated mountpoint

[Amir Goldstein]

On Wed, May 29, 2019 at 1:19 PM Murphy Zhou <xzhou@redhat.com> wrote:
>
> Some tests are mounting overlayfs internally and run tests on it.
> This mount can fail if the filesystem we are running on does not
> support overlay mount upon it. For example, we are already running
> tests on overlayfs or NFS, or CIFS. Test will report broken and
> failure.
>
> Fixing this by put overlayfs dirs in a separated mountpoint, like in
> readahead02 by Amir.
>
> Suggested-by: Petr Vorel <pvorel@suse.cz>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
>  .../kernel/syscalls/execveat/execveat03.c     | 31 ++--------------
>  testcases/kernel/syscalls/inotify/inotify07.c | 33 ++++-------------
>  testcases/kernel/syscalls/inotify/inotify08.c | 37 +++++--------------
>  .../kernel/syscalls/readahead/readahead02.c   | 33 ++---------------
>  4 files changed, 26 insertions(+), 108 deletions(-)

That's a very nice diffstat! :-)

Reviewed-by: Amir Goldstein <amir73il@gmail.com>



>
> diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
> index def33923b..7df1b0faa 100644
> --- a/testcases/kernel/syscalls/execveat/execveat03.c
> +++ b/testcases/kernel/syscalls/execveat/execveat03.c
> @@ -55,11 +55,10 @@
>  #include "lapi/fcntl.h"
>  #include "execveat.h"
>
> -#define OVL_MNT "ovl"
>  #define TEST_APP "execveat_child"
>  #define TEST_FILE_PATH OVL_MNT"/"TEST_APP
>
> -static int ovl_mounted;
> +static const char mntpoint[] = OVL_BASE_MNTPOINT;
>
>  static void do_child(void)
>  {
> @@ -86,31 +85,7 @@ static void verify_execveat(void)
>
>  static void setup(void)
>  {
> -       int ret;
> -
>         check_execveat();
> -
> -       /* Setup an overlay mount with lower file */
> -       SAFE_MKDIR("lower", 0755);
> -       SAFE_MKDIR("upper", 0755);
> -       SAFE_MKDIR("work", 0755);
> -       SAFE_MKDIR(OVL_MNT, 0755);
> -       ret = mount("overlay", OVL_MNT, "overlay", 0,
> -                   "lowerdir=lower,upperdir=upper,workdir=work");
> -       if (ret < 0) {
> -               if (errno == ENODEV) {
> -                       tst_brk(TCONF,
> -                               "overlayfs is not configured in this kernel.");
> -               }
> -               tst_brk(TBROK | TERRNO, "overlayfs mount failed");
> -       }
> -       ovl_mounted = 1;
> -}
> -
> -static void cleanup(void)
> -{
> -       if (ovl_mounted)
> -               SAFE_UMOUNT(OVL_MNT);
>  }
>
>  static const char *const resource_files[] = {
> @@ -121,10 +96,12 @@ static const char *const resource_files[] = {
>  static struct tst_test test = {
>         .needs_root = 1,
>         .needs_tmpdir = 1,
> +       .mount_device = 1,
> +       .needs_overlay = 1,
> +       .mntpoint = mntpoint,
>         .forks_child = 1,
>         .child_needs_reinit = 1,
>         .setup = setup,
> -       .cleanup = cleanup,
>         .test_all = verify_execveat,
>         .resource_files = resource_files,
>  };
> diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
> index 96370b5cf..47817cd42 100644
> --- a/testcases/kernel/syscalls/inotify/inotify07.c
> +++ b/testcases/kernel/syscalls/inotify/inotify07.c
> @@ -73,13 +73,12 @@ struct event_t {
>         unsigned int mask;
>  };
>
> -#define OVL_MNT "ovl"
>  #define DIR_NAME "test_dir"
>  #define DIR_PATH OVL_MNT"/"DIR_NAME
>  #define FILE_NAME "test_file"
>  #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
>
> -static int ovl_mounted;
> +static const char mntpoint[] = OVL_BASE_MNTPOINT;
>
>  static struct event_t event_set[EVENT_MAX];
>
> @@ -161,27 +160,12 @@ void verify_inotify(void)
>  static void setup(void)
>  {
>         struct stat buf;
> -       int ret;
>
>         /* Setup an overlay mount with lower dir and file */
> -       SAFE_MKDIR("lower", 0755);
> -       SAFE_MKDIR("lower/"DIR_NAME, 0755);
> -       SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> -       SAFE_MKDIR("upper", 0755);
> -       SAFE_MKDIR("work", 0755);
> -       SAFE_MKDIR(OVL_MNT, 0755);
> -       ret = mount("overlay", OVL_MNT, "overlay", 0,
> -                   "lowerdir=lower,upperdir=upper,workdir=work");
> -       if (ret < 0) {
> -               if (errno == ENODEV) {
> -                       tst_brk(TCONF,
> -                               "overlayfs is not configured in this kernel.");
> -               } else {
> -                       tst_brk(TBROK | TERRNO,
> -                               "overlayfs mount failed");
> -               }
> -       }
> -       ovl_mounted = 1;
> +       SAFE_UMOUNT(OVL_MNT);
> +       SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
> +       SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
> +       SAFE_MOUNT_OVERLAY();
>
>         fd_notify = myinotify_init1(O_NONBLOCK);
>         if (fd_notify < 0) {
> @@ -221,19 +205,18 @@ static void cleanup(void)
>         if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
>                 tst_res(TWARN,
>                         "inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
> -
>         }
>
>         if (fd_notify > 0)
>                 SAFE_CLOSE(fd_notify);
> -
> -       if (ovl_mounted)
> -               SAFE_UMOUNT(OVL_MNT);
>  }
>
>  static struct tst_test test = {
>         .needs_root = 1,
>         .needs_tmpdir = 1,
> +       .mount_device = 1,
> +       .needs_overlay = 1,
> +       .mntpoint = mntpoint,
>         .setup = setup,
>         .cleanup = cleanup,
>         .test_all = verify_inotify,
> diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
> index acdb95345..067c01dbb 100644
> --- a/testcases/kernel/syscalls/inotify/inotify08.c
> +++ b/testcases/kernel/syscalls/inotify/inotify08.c
> @@ -74,11 +74,10 @@ struct event_t {
>         unsigned int mask;
>  };
>
> -#define OVL_MNT "ovl"
>  #define FILE_NAME "test_file"
>  #define FILE_PATH OVL_MNT"/"FILE_NAME
>
> -static int ovl_mounted;
> +static const char mntpoint[] = OVL_BASE_MNTPOINT;
>
>  static struct event_t event_set[EVENT_MAX];
>
> @@ -104,8 +103,8 @@ void verify_inotify(void)
>         test_cnt++;
>
>         /* Make sure events on upper/lower do not show in overlay watch */
> -       SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
> -       SAFE_TOUCH("upper/"FILE_NAME, 0644, NULL);
> +       SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
> +       SAFE_TOUCH(OVL_UPPER"/"FILE_NAME, 0644, NULL);
>
>         int len = read(fd_notify, event_buf, EVENT_BUF_LEN);
>         if (len == -1 && errno != EAGAIN) {
> @@ -154,32 +153,17 @@ void verify_inotify(void)
>  static void setup(void)
>  {
>         struct stat buf;
> -       int ret;
>
>         /* Setup an overlay mount with lower file */
> -       SAFE_MKDIR("lower", 0755);
> -       SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
> -       SAFE_MKDIR("upper", 0755);
> -       SAFE_MKDIR("work", 0755);
> -       SAFE_MKDIR(OVL_MNT, 0755);
> -       ret = mount("overlay", OVL_MNT, "overlay", 0,
> -                   "lowerdir=lower,upperdir=upper,workdir=work");
> -       if (ret < 0) {
> -               if (errno == ENODEV) {
> -                       tst_brk(TCONF,
> -                               "overlayfs is not configured in this kernel.");
> -               } else {
> -                       tst_brk(TBROK | TERRNO,
> -                               "overlayfs mount failed");
> -               }
> -       }
> -       ovl_mounted = 1;
> +       SAFE_UMOUNT(OVL_MNT);
> +       SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
> +       SAFE_MOUNT_OVERLAY();
>
>         fd_notify = myinotify_init1(O_NONBLOCK);
>         if (fd_notify < 0) {
>                 if (errno == ENOSYS) {
>                         tst_brk(TCONF,
> -                               "inotify is not configured in this kernel.");
> +                               "inotify is not configured in this kernel");
>                 } else {
>                         tst_brk(TBROK | TERRNO,
>                                 "inotify_init () failed");
> @@ -217,19 +201,18 @@ static void cleanup(void)
>         if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
>                 tst_res(TWARN,
>                         "inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
> -
>         }
>
>         if (fd_notify > 0)
>                 SAFE_CLOSE(fd_notify);
> -
> -       if (ovl_mounted)
> -               SAFE_UMOUNT(OVL_MNT);
>  }
>
>  static struct tst_test test = {
>         .needs_root = 1,
>         .needs_tmpdir = 1,
> +       .mount_device = 1,
> +       .needs_overlay = 1,
> +       .mntpoint = mntpoint,
>         .setup = setup,
>         .cleanup = cleanup,
>         .test_all = verify_inotify,
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 39ddbd583..f6e07173d 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -44,16 +44,11 @@ static int pagesize;
>  static unsigned long cached_max;
>  static int ovl_mounted;
>
> -#define MNTPOINT        "mntpoint"
> -#define OVL_LOWER      MNTPOINT"/lower"
> -#define OVL_UPPER      MNTPOINT"/upper"
> -#define OVL_WORK       MNTPOINT"/work"
> -#define OVL_MNT                MNTPOINT"/ovl"
>  static int readahead_length  = 4096;
>  static char sys_bdi_ra_path[PATH_MAX];
>  static int orig_bdi_limit;
>
> -static const char mntpoint[] = MNTPOINT;
> +static const char mntpoint[] = OVL_BASE_MNTPOINT;
>
>  static struct tst_option options[] = {
>         {"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
> @@ -132,7 +127,8 @@ static void create_testfile(int use_overlay)
>         char *tmp;
>         size_t i;
>
> -       sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
> +       sprintf(testfile, "%s/testfile",
> +               use_overlay ? OVL_MNT : OVL_BASE_MNTPOINT);
>         tst_res(TINFO, "creating test file of size: %zu", testfile_size);
>         tmp = SAFE_MALLOC(pagesize);
>
> @@ -329,27 +325,6 @@ static void test_readahead(unsigned int n)
>         }
>  }
>
> -static void setup_overlay(void)
> -{
> -       int ret;
> -
> -       /* Setup an overlay mount with lower dir and file */
> -       SAFE_MKDIR(OVL_LOWER, 0755);
> -       SAFE_MKDIR(OVL_UPPER, 0755);
> -       SAFE_MKDIR(OVL_WORK, 0755);
> -       SAFE_MKDIR(OVL_MNT, 0755);
> -       ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> -                   ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> -       if (ret < 0) {
> -               if (errno == ENODEV) {
> -                       tst_res(TINFO,
> -                               "overlayfs is not configured in this kernel.");
> -                       return;
> -               }
> -               tst_brk(TBROK | TERRNO, "overlayfs mount failed");
> -       }
> -       ovl_mounted = 1;
> -}
>
>  /*
>   * We try raising bdi readahead limit as much as we can. We write
> @@ -413,7 +388,7 @@ static void setup(void)
>         setup_readahead_length();
>         tst_res(TINFO, "readahead length: %d", readahead_length);
>
> -       setup_overlay();
> +       ovl_mounted = TST_MOUNT_OVERLAY();
>  }
>
>  static void cleanup(void)
> --
> 2.21.0
>

[LTP] [PATCH v5 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Amir Goldstein]

On Wed, May 29, 2019 at 1:19 PM Murphy Zhou <xzhou@redhat.com> wrote:
>
> Introduce tst_test->needs_overlay;
> Define constraints of needed overlayfs dirs;
> create_overlay_dirs() to create lower/upper/work dirs;
> mount_overlay() to mount overlayfs;
> SAFE_MOUNT_OVERLAY macro to mount overlayfs safely, tst_brk TCONF
> if mount fail with errno ENODEV;
> TST_MOUNT_OVERLAY  macro to mount overlayfs and return 0 if succeeds;
>
> Suggested-by: Petr Vorel <pvorel@suse.cz>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---

Very nice! just one minor comment, otherwise

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> v5:
>   Introduce tst_test->needs_overlay to mount and umount overlay
>   Several other fixes suggested by Petr
>   Update execveat03, inotify07/8 to use needs_overlay
> v4:
>   Update ENODEV handle logic
>   Define TST_MOUNT_OVERLAY and SAFE_MOUNT_OVERLAY macros
>   Change helper names
> v3:
>   Split setup to 2 parts: creating files and mounting.
>   Use macro SAFE_MOUNT_OVERLAYFS.
>   Handle ENODEV differently for 4 testcases we have modified.
> v2:
>   define constraints in header file.
>   add a setup helper to create dirs and mount.
>  include/safe_file_ops_fn.h  |  4 ++++
>  include/tst_fs.h            |  6 +++++
>  include/tst_safe_file_ops.h |  6 +++++
>  include/tst_test.h          |  1 +
>  lib/tst_fs_setup.c          | 48 +++++++++++++++++++++++++++++++++++++
>  lib/tst_test.c              | 15 ++++++++++++
>  6 files changed, 80 insertions(+)
>  create mode 100644 lib/tst_fs_setup.c
>
> diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
> index 35ec4fb1f..052fb1b9a 100644
> --- a/include/safe_file_ops_fn.h
> +++ b/include/safe_file_ops_fn.h
> @@ -76,4 +76,8 @@ void safe_touch(const char *file, const int lineno,
>                 const char *pathname,
>                 mode_t mode, const struct timespec times[2]);
>
> +/* helper functions to setup overlayfs mountpoint */
> +void create_overlay_dirs(void);
> +int mount_overlay(const char *file, const int lineno, int skip);
> +
>  #endif /* SAFE_FILE_OPS_FN */
> diff --git a/include/tst_fs.h b/include/tst_fs.h
> index b2b19ada6..ebca065c6 100644
> --- a/include/tst_fs.h
> +++ b/include/tst_fs.h
> @@ -50,6 +50,12 @@ enum {
>         TST_GB = 1073741824,
>  };
>
> +#define OVL_BASE_MNTPOINT        "mntpoint"
> +#define OVL_LOWER      OVL_BASE_MNTPOINT"/lower"
> +#define OVL_UPPER      OVL_BASE_MNTPOINT"/upper"
> +#define OVL_WORK       OVL_BASE_MNTPOINT"/work"
> +#define OVL_MNT                OVL_BASE_MNTPOINT"/ovl"
> +
>  /*
>   * @path: path is the pathname of any file within the mounted file system
>   * @mult: mult should be TST_KB, TST_MB or TST_GB
> diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
> index 5c3fea4e2..b62a7447f 100644
> --- a/include/tst_safe_file_ops.h
> +++ b/include/tst_safe_file_ops.h
> @@ -59,4 +59,10 @@
>         safe_touch(__FILE__, __LINE__, NULL, \
>                         (pathname), (mode), (times))
>
> +#define SAFE_MOUNT_OVERLAY() \
> +       ((void) mount_overlay(__FILE__, __LINE__, 1))
> +
> +#define TST_MOUNT_OVERLAY() \
> +       (mount_overlay(__FILE__, __LINE__, 0) == 0)
> +
>  #endif /* TST_SAFE_FILE_OPS */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index e4b935c45..f3b8901a2 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -121,6 +121,7 @@ struct tst_test {
>         int needs_root:1;
>         int forks_child:1;
>         int needs_device:1;
> +       int needs_overlay:1;
>         int needs_checkpoints:1;
>         int format_device:1;
>         int mount_device:1;
> diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
> new file mode 100644
> index 000000000..e8ae929d7
> --- /dev/null
> +++ b/lib/tst_fs_setup.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <dirent.h>
> +#include <errno.h>
> +#include <sys/mount.h>
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_fs.h"
> +
> +#define TST_FS_SETUP_OVERLAYFS_MSG "overlayfs is not configured in this kernel"
> +#define TST_FS_SETUP_OVERLAYFS_CONFIG "lowerdir="OVL_LOWER",upperdir="OVL_UPPER",workdir="OVL_WORK
> +
> +void create_overlay_dirs(void)
> +{
> +       DIR *dir = opendir(OVL_LOWER);
> +       if (dir == NULL) {
> +               SAFE_MKDIR(OVL_LOWER, 0755);
> +               SAFE_MKDIR(OVL_UPPER, 0755);
> +               SAFE_MKDIR(OVL_WORK, 0755);
> +               SAFE_MKDIR(OVL_MNT, 0755);

return here so we don't closedir(NULL)?

> +       }
> +       closedir(dir);
> +}
> +

Thanks for doing this work!
Amir.

[LTP] [PATCH v5 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Murphy Zhou]

On Wed, May 29, 2019 at 01:59:15PM +0300, Amir Goldstein wrote:
> On Wed, May 29, 2019 at 1:19 PM Murphy Zhou <xzhou@redhat.com> wrote:
> >
> > Introduce tst_test->needs_overlay;
> > Define constraints of needed overlayfs dirs;
> > create_overlay_dirs() to create lower/upper/work dirs;
> > mount_overlay() to mount overlayfs;
> > SAFE_MOUNT_OVERLAY macro to mount overlayfs safely, tst_brk TCONF
> > if mount fail with errno ENODEV;
> > TST_MOUNT_OVERLAY  macro to mount overlayfs and return 0 if succeeds;
> >
> > Suggested-by: Petr Vorel <pvorel@suse.cz>
> > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> > ---
> 
> Very nice! just one minor comment, otherwise

Thank you very much!

> 
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> > v5:
> >   Introduce tst_test->needs_overlay to mount and umount overlay
> >   Several other fixes suggested by Petr
> >   Update execveat03, inotify07/8 to use needs_overlay
> > v4:
> >   Update ENODEV handle logic
> >   Define TST_MOUNT_OVERLAY and SAFE_MOUNT_OVERLAY macros
> >   Change helper names
> > v3:
> >   Split setup to 2 parts: creating files and mounting.
> >   Use macro SAFE_MOUNT_OVERLAYFS.
> >   Handle ENODEV differently for 4 testcases we have modified.
> > v2:
> >   define constraints in header file.
> >   add a setup helper to create dirs and mount.
> >  include/safe_file_ops_fn.h  |  4 ++++
> >  include/tst_fs.h            |  6 +++++
> >  include/tst_safe_file_ops.h |  6 +++++
> >  include/tst_test.h          |  1 +
> >  lib/tst_fs_setup.c          | 48 +++++++++++++++++++++++++++++++++++++
> >  lib/tst_test.c              | 15 ++++++++++++
> >  6 files changed, 80 insertions(+)
> >  create mode 100644 lib/tst_fs_setup.c
> >
> > diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
> > index 35ec4fb1f..052fb1b9a 100644
> > --- a/include/safe_file_ops_fn.h
> > +++ b/include/safe_file_ops_fn.h
> > @@ -76,4 +76,8 @@ void safe_touch(const char *file, const int lineno,
> >                 const char *pathname,
> >                 mode_t mode, const struct timespec times[2]);
> >
> > +/* helper functions to setup overlayfs mountpoint */
> > +void create_overlay_dirs(void);
> > +int mount_overlay(const char *file, const int lineno, int skip);
> > +
> >  #endif /* SAFE_FILE_OPS_FN */
> > diff --git a/include/tst_fs.h b/include/tst_fs.h
> > index b2b19ada6..ebca065c6 100644
> > --- a/include/tst_fs.h
> > +++ b/include/tst_fs.h
> > @@ -50,6 +50,12 @@ enum {
> >         TST_GB = 1073741824,
> >  };
> >
> > +#define OVL_BASE_MNTPOINT        "mntpoint"
> > +#define OVL_LOWER      OVL_BASE_MNTPOINT"/lower"
> > +#define OVL_UPPER      OVL_BASE_MNTPOINT"/upper"
> > +#define OVL_WORK       OVL_BASE_MNTPOINT"/work"
> > +#define OVL_MNT                OVL_BASE_MNTPOINT"/ovl"
> > +
> >  /*
> >   * @path: path is the pathname of any file within the mounted file system
> >   * @mult: mult should be TST_KB, TST_MB or TST_GB
> > diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
> > index 5c3fea4e2..b62a7447f 100644
> > --- a/include/tst_safe_file_ops.h
> > +++ b/include/tst_safe_file_ops.h
> > @@ -59,4 +59,10 @@
> >         safe_touch(__FILE__, __LINE__, NULL, \
> >                         (pathname), (mode), (times))
> >
> > +#define SAFE_MOUNT_OVERLAY() \
> > +       ((void) mount_overlay(__FILE__, __LINE__, 1))
> > +
> > +#define TST_MOUNT_OVERLAY() \
> > +       (mount_overlay(__FILE__, __LINE__, 0) == 0)
> > +
> >  #endif /* TST_SAFE_FILE_OPS */
> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index e4b935c45..f3b8901a2 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -121,6 +121,7 @@ struct tst_test {
> >         int needs_root:1;
> >         int forks_child:1;
> >         int needs_device:1;
> > +       int needs_overlay:1;
> >         int needs_checkpoints:1;
> >         int format_device:1;
> >         int mount_device:1;
> > diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
> > new file mode 100644
> > index 000000000..e8ae929d7
> > --- /dev/null
> > +++ b/lib/tst_fs_setup.c
> > @@ -0,0 +1,48 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include <dirent.h>
> > +#include <errno.h>
> > +#include <sys/mount.h>
> > +
> > +#define TST_NO_DEFAULT_MAIN
> > +#include "tst_test.h"
> > +#include "tst_fs.h"
> > +
> > +#define TST_FS_SETUP_OVERLAYFS_MSG "overlayfs is not configured in this kernel"
> > +#define TST_FS_SETUP_OVERLAYFS_CONFIG "lowerdir="OVL_LOWER",upperdir="OVL_UPPER",workdir="OVL_WORK
> > +
> > +void create_overlay_dirs(void)
> > +{
> > +       DIR *dir = opendir(OVL_LOWER);
> > +       if (dir == NULL) {
> > +               SAFE_MKDIR(OVL_LOWER, 0755);
> > +               SAFE_MKDIR(OVL_UPPER, 0755);
> > +               SAFE_MKDIR(OVL_WORK, 0755);
> > +               SAFE_MKDIR(OVL_MNT, 0755);
> 
> return here so we don't closedir(NULL)?

Sure! This needs to be fixed. Good catch!

I'll send v6 with this fix and adding your Reivewed-by.

Thank Amir and Petr very much!

--
Murphy

> 
> > +       }
> > +       closedir(dir);
> > +}
> > +
> 
> Thanks for doing this work!
> Amir.

[LTP] [PATCH v6 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Murphy Zhou]

Define constraints of needed overlayfs dirs;
create_overlay_dirs() to create lower/upper/work dirs;
mount_overlay() to mount overlayfs;
SAFE_MOUNT_OVERLAY macro to mount overlayfs safely, tst_brk TCONF
if mount fail with errno ENODEV;
TST_MOUNT_OVERLAY  macro to mount overlayfs and return 0 if succeeds;

Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
v6:
  Return before close NULL dir in create_overlay_dirs
v5:
  Introduce tst_test->needs_overlay to mount and umount overlay
  Several other fixes suggested by Petr
  Update execveat03, inotify07/8 to use needs_overlay
v4:
  Update ENODEV handle logic
  Define TST_MOUNT_OVERLAY and SAFE_MOUNT_OVERLAY macros
  Change helper names
v3:
  Split setup to 2 parts: creating files and mounting.
  Use macro SAFE_MOUNT_OVERLAYFS.
  Handle ENODEV differently for 4 testcases we have modified.
v2:
  define constraints in header file.
  add a setup helper to create dirs and mount.

 include/safe_file_ops_fn.h  |  4 +++
 include/tst_fs.h            |  6 +++++
 include/tst_safe_file_ops.h |  6 +++++
 include/tst_test.h          |  1 +
 lib/tst_fs_setup.c          | 49 +++++++++++++++++++++++++++++++++++++
 lib/tst_test.c              | 15 ++++++++++++
 6 files changed, 81 insertions(+)
 create mode 100644 lib/tst_fs_setup.c

diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
index 35ec4fb1f..052fb1b9a 100644
--- a/include/safe_file_ops_fn.h
+++ b/include/safe_file_ops_fn.h
@@ -76,4 +76,8 @@ void safe_touch(const char *file, const int lineno,
 		const char *pathname,
 		mode_t mode, const struct timespec times[2]);
 
+/* helper functions to setup overlayfs mountpoint */
+void create_overlay_dirs(void);
+int mount_overlay(const char *file, const int lineno, int skip);
+
 #endif /* SAFE_FILE_OPS_FN */
diff --git a/include/tst_fs.h b/include/tst_fs.h
index b2b19ada6..ebca065c6 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -50,6 +50,12 @@ enum {
 	TST_GB = 1073741824,
 };
 
+#define OVL_BASE_MNTPOINT        "mntpoint"
+#define OVL_LOWER	OVL_BASE_MNTPOINT"/lower"
+#define OVL_UPPER	OVL_BASE_MNTPOINT"/upper"
+#define OVL_WORK	OVL_BASE_MNTPOINT"/work"
+#define OVL_MNT		OVL_BASE_MNTPOINT"/ovl"
+
 /*
  * @path: path is the pathname of any file within the mounted file system
  * @mult: mult should be TST_KB, TST_MB or TST_GB
diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
index 5c3fea4e2..b62a7447f 100644
--- a/include/tst_safe_file_ops.h
+++ b/include/tst_safe_file_ops.h
@@ -59,4 +59,10 @@
 	safe_touch(__FILE__, __LINE__, NULL, \
 			(pathname), (mode), (times))
 
+#define SAFE_MOUNT_OVERLAY() \
+	((void) mount_overlay(__FILE__, __LINE__, 1))
+
+#define TST_MOUNT_OVERLAY() \
+	(mount_overlay(__FILE__, __LINE__, 0) == 0)
+
 #endif /* TST_SAFE_FILE_OPS */
diff --git a/include/tst_test.h b/include/tst_test.h
index e4b935c45..f3b8901a2 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -121,6 +121,7 @@ struct tst_test {
 	int needs_root:1;
 	int forks_child:1;
 	int needs_device:1;
+	int needs_overlay:1;
 	int needs_checkpoints:1;
 	int format_device:1;
 	int mount_device:1;
diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
new file mode 100644
index 000000000..32a6218e3
--- /dev/null
+++ b/lib/tst_fs_setup.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <dirent.h>
+#include <errno.h>
+#include <sys/mount.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_fs.h"
+
+#define TST_FS_SETUP_OVERLAYFS_MSG "overlayfs is not configured in this kernel"
+#define TST_FS_SETUP_OVERLAYFS_CONFIG "lowerdir="OVL_LOWER",upperdir="OVL_UPPER",workdir="OVL_WORK
+
+void create_overlay_dirs(void)
+{
+	DIR *dir = opendir(OVL_LOWER);
+	if (dir == NULL) {
+		SAFE_MKDIR(OVL_LOWER, 0755);
+		SAFE_MKDIR(OVL_UPPER, 0755);
+		SAFE_MKDIR(OVL_WORK, 0755);
+		SAFE_MKDIR(OVL_MNT, 0755);
+		return;
+	}
+	closedir(dir);
+}
+
+int mount_overlay(const char *file, const int lineno, int skip)
+{
+	int ret = 0;
+
+	create_overlay_dirs();
+	ret = mount("overlay", OVL_MNT, "overlay", 0,
+				TST_FS_SETUP_OVERLAYFS_CONFIG);
+	if (ret == 0)
+		return 0;
+
+	if (errno == ENODEV) {
+		if (skip) {
+			tst_brk(TCONF, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG,
+				file, lineno);
+		} else {
+			tst_res(TINFO, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG,
+				file, lineno);
+		}
+	} else {
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+	}
+	return ret;
+}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2d88adbd7..95f389d2e 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -50,6 +50,7 @@ static int iterations = 1;
 static float duration = -1;
 static pid_t main_pid, lib_pid;
 static int mntpoint_mounted;
+static int ovl_mounted;
 static struct timespec tst_start_time; /* valid only for test pid */
 
 struct results {
@@ -871,6 +872,17 @@ static void do_setup(int argc, char *argv[])
 			prepare_device();
 	}
 
+	if (tst_test->needs_overlay && !tst_test->mount_device) {
+		tst_brk(TBROK, "tst_test->mount_device must be set");
+	}
+	if (tst_test->needs_overlay && !mntpoint_mounted) {
+		tst_brk(TBROK, "tst_test->mntpoint must be mounted");
+	}
+	if (tst_test->needs_overlay && !ovl_mounted) {
+		SAFE_MOUNT_OVERLAY();
+		ovl_mounted = 1;
+	}
+
 	if (tst_test->resource_files)
 		copy_resources();
 
@@ -891,6 +903,9 @@ static void do_test_setup(void)
 
 static void do_cleanup(void)
 {
+	if (ovl_mounted)
+		SAFE_UMOUNT(OVL_MNT);
+
 	if (mntpoint_mounted)
 		tst_umount(tst_test->mntpoint);
 
-- 
2.21.0

[LTP] [PATCH v6 2/2] OVL_MNT: setup overlayfs dirs and mount in a separated mountpoint

[Murphy Zhou]

Some tests are mounting overlayfs internally and run tests on it.
This mount can fail if the filesystem we are running on does not
support overlay mount upon it. For example, we are already running
tests on overlayfs or NFS, or CIFS. Test will report broken and
failure.

Fixing this by put overlayfs dirs in a separated mountpoint, like in
readahead02 by Amir.

Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
 .../kernel/syscalls/execveat/execveat03.c     | 31 ++--------------
 testcases/kernel/syscalls/inotify/inotify07.c | 33 ++++-------------
 testcases/kernel/syscalls/inotify/inotify08.c | 37 +++++--------------
 .../kernel/syscalls/readahead/readahead02.c   | 33 ++---------------
 4 files changed, 26 insertions(+), 108 deletions(-)

diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
index def33923b..7df1b0faa 100644
--- a/testcases/kernel/syscalls/execveat/execveat03.c
+++ b/testcases/kernel/syscalls/execveat/execveat03.c
@@ -55,11 +55,10 @@
 #include "lapi/fcntl.h"
 #include "execveat.h"
 
-#define OVL_MNT "ovl"
 #define TEST_APP "execveat_child"
 #define TEST_FILE_PATH OVL_MNT"/"TEST_APP
 
-static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static void do_child(void)
 {
@@ -86,31 +85,7 @@ static void verify_execveat(void)
 
 static void setup(void)
 {
-	int ret;
-
 	check_execveat();
-
-	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
-}
-
-static void cleanup(void)
-{
-	if (ovl_mounted)
-		SAFE_UMOUNT(OVL_MNT);
 }
 
 static const char *const resource_files[] = {
@@ -121,10 +96,12 @@ static const char *const resource_files[] = {
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.needs_overlay = 1,
+	.mntpoint = mntpoint,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.setup = setup,
-	.cleanup = cleanup,
 	.test_all = verify_execveat,
 	.resource_files = resource_files,
 };
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index 96370b5cf..47817cd42 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -73,13 +73,12 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define DIR_NAME "test_dir"
 #define DIR_PATH OVL_MNT"/"DIR_NAME
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"DIR_NAME"/"FILE_NAME
 
-static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -161,27 +160,12 @@ void verify_inotify(void)
 static void setup(void)
 {
 	struct stat buf;
-	int ret;
 
 	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_MKDIR("lower/"DIR_NAME, 0755);
-	SAFE_TOUCH("lower/"DIR_NAME"/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
-	ovl_mounted = 1;
+	SAFE_UMOUNT(OVL_MNT);
+	SAFE_MKDIR(OVL_LOWER"/"DIR_NAME, 0755);
+	SAFE_TOUCH(OVL_LOWER"/"DIR_NAME"/"FILE_NAME, 0644, NULL);
+	SAFE_MOUNT_OVERLAY();
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
 	if (fd_notify < 0) {
@@ -221,19 +205,18 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
 		SAFE_CLOSE(fd_notify);
-
-	if (ovl_mounted)
-		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.needs_overlay = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index acdb95345..067c01dbb 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -74,11 +74,10 @@ struct event_t {
 	unsigned int mask;
 };
 
-#define OVL_MNT "ovl"
 #define FILE_NAME "test_file"
 #define FILE_PATH OVL_MNT"/"FILE_NAME
 
-static int ovl_mounted;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct event_t event_set[EVENT_MAX];
 
@@ -104,8 +103,8 @@ void verify_inotify(void)
 	test_cnt++;
 
 	/* Make sure events on upper/lower do not show in overlay watch */
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_TOUCH("upper/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_TOUCH(OVL_UPPER"/"FILE_NAME, 0644, NULL);
 
 	int len = read(fd_notify, event_buf, EVENT_BUF_LEN);
 	if (len == -1 && errno != EAGAIN) {
@@ -154,32 +153,17 @@ void verify_inotify(void)
 static void setup(void)
 {
 	struct stat buf;
-	int ret;
 
 	/* Setup an overlay mount with lower file */
-	SAFE_MKDIR("lower", 0755);
-	SAFE_TOUCH("lower/"FILE_NAME, 0644, NULL);
-	SAFE_MKDIR("upper", 0755);
-	SAFE_MKDIR("work", 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0,
-		    "lowerdir=lower,upperdir=upper,workdir=work");
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_brk(TCONF,
-				"overlayfs is not configured in this kernel.");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"overlayfs mount failed");
-		}
-	}
-	ovl_mounted = 1;
+	SAFE_UMOUNT(OVL_MNT);
+	SAFE_TOUCH(OVL_LOWER"/"FILE_NAME, 0644, NULL);
+	SAFE_MOUNT_OVERLAY();
 
 	fd_notify = myinotify_init1(O_NONBLOCK);
 	if (fd_notify < 0) {
 		if (errno == ENOSYS) {
 			tst_brk(TCONF,
-				"inotify is not configured in this kernel.");
+				"inotify is not configured in this kernel");
 		} else {
 			tst_brk(TBROK | TERRNO,
 				"inotify_init () failed");
@@ -217,19 +201,18 @@ static void cleanup(void)
 	if (reap_wd && myinotify_rm_watch(fd_notify, wd) < 0) {
 		tst_res(TWARN,
 			"inotify_rm_watch (%d, %d) failed,", fd_notify, wd);
-
 	}
 
 	if (fd_notify > 0)
 		SAFE_CLOSE(fd_notify);
-
-	if (ovl_mounted)
-		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.needs_overlay = 1,
+	.mntpoint = mntpoint,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 39ddbd583..f6e07173d 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -44,16 +44,11 @@ static int pagesize;
 static unsigned long cached_max;
 static int ovl_mounted;
 
-#define MNTPOINT        "mntpoint"
-#define OVL_LOWER	MNTPOINT"/lower"
-#define OVL_UPPER	MNTPOINT"/upper"
-#define OVL_WORK	MNTPOINT"/work"
-#define OVL_MNT		MNTPOINT"/ovl"
 static int readahead_length  = 4096;
 static char sys_bdi_ra_path[PATH_MAX];
 static int orig_bdi_limit;
 
-static const char mntpoint[] = MNTPOINT;
+static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
 static struct tst_option options[] = {
 	{"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
@@ -132,7 +127,8 @@ static void create_testfile(int use_overlay)
 	char *tmp;
 	size_t i;
 
-	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
+	sprintf(testfile, "%s/testfile",
+		use_overlay ? OVL_MNT : OVL_BASE_MNTPOINT);
 	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
@@ -329,27 +325,6 @@ static void test_readahead(unsigned int n)
 	}
 }
 
-static void setup_overlay(void)
-{
-	int ret;
-
-	/* Setup an overlay mount with lower dir and file */
-	SAFE_MKDIR(OVL_LOWER, 0755);
-	SAFE_MKDIR(OVL_UPPER, 0755);
-	SAFE_MKDIR(OVL_WORK, 0755);
-	SAFE_MKDIR(OVL_MNT, 0755);
-	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
-		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
-	if (ret < 0) {
-		if (errno == ENODEV) {
-			tst_res(TINFO,
-				"overlayfs is not configured in this kernel.");
-			return;
-		}
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
-	}
-	ovl_mounted = 1;
-}
 
 /*
  * We try raising bdi readahead limit as much as we can. We write
@@ -413,7 +388,7 @@ static void setup(void)
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
 
-	setup_overlay();
+	ovl_mounted = TST_MOUNT_OVERLAY();
 }
 
 static void cleanup(void)
-- 
2.21.0

[LTP] [PATCH v6 2/2] OVL_MNT: setup overlayfs dirs and mount in a separated mountpoint

[Petr Vorel]

Hi Murphy, Amir,

merged, with tiny changes in the code and updated commit message.
Thank you both for a very nice collaboration.

Kind regards,
Petr

[LTP] [PATCH v6 1/2] OVL_MNT: add helpers to setup overlayfs mountpoint

[Petr Vorel]

Hi Murphy,

> Define constraints of needed overlayfs dirs;
> create_overlay_dirs() to create lower/upper/work dirs;
> mount_overlay() to mount overlayfs;
> SAFE_MOUNT_OVERLAY macro to mount overlayfs safely, tst_brk TCONF
> if mount fail with errno ENODEV;
> TST_MOUNT_OVERLAY  macro to mount overlayfs and return 0 if succeeds;

> Suggested-by: Petr Vorel <pvorel@suse.cz>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
Acked-by: Petr Vorel <pvorel@suse.cz>

with tiny change not initialize ret (no need as it's initialized by mount()).

Kind regards,
Petr

diff --git lib/tst_fs_setup.c lib/tst_fs_setup.c
index 32a6218e3..54ea37077 100644
--- lib/tst_fs_setup.c
+++ lib/tst_fs_setup.c
@@ -26,7 +26,7 @@ void create_overlay_dirs(void)
 
 int mount_overlay(const char *file, const int lineno, int skip)
 {
-	int ret = 0;
+	int ret;
 
 	create_overlay_dirs();
 	ret = mount("overlay", OVL_MNT, "overlay", 0,

[LTP] [PATCH v2 1/2] OVL_MNT: add setup_overlay helper

[Murphy Zhou]

Hi Petr,

On Fri, May 24, 2019 at 10:54:52AM +0200, Petr Vorel wrote:
> Hi Murphy,
> 
> > > > +	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> > > > +		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> > > > +	if (ret < 0) {
> > > > +		if (errno == ENODEV) {
> > > > +			tst_res(TINFO,
> > > > +				"overlayfs is not configured in this kernel.");
> > > > +			return 1;
> > > First I thought we'd implement it as a test flag (member of struct tst_test).
> > > When we have it as separate function I wonder whether we could TCONF on ENODEV
> > > instead of TINFO and return. Maybe there could be here for int strict parameter,
> 
> > The return value is referenced in the testcase to determine whether to run
> > tests in overlayfs. It's needed.
> 
> > If this strict parameter is only for different wording on NODEV. Is it
> > necessary ?
> 
> I'll recap my suggestions:
> 1) I like having macros to help reduce some parameters, this does not block
> functions being flexible (which requires parameters).
> 2) Having helper function create_overlay_dirs() used separately, than parameter
> in single function (Amir [1] suggestion makes sense).
> 
> Something like this, just a suggestion:
> 
> int create_overlay_dirs()
> {
> 	SAFE_MKDIR(OVL_LOWER, 0755);
> 	...
> 	return ret;
> }
> 
> int mount_overlay(const char *file, const int lineno, int safe)
> {
> 	...
> 	if (create_overlay_dirs())
> 		tst_brk(TCONF, "...");
> 
> 	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
> 		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
> 	if (ret < 0) {
> 		if (errno == ENODEV) {
> 			/*
> 			 * TODO: maybe safe is confusing as we use tst_brk(TBROK anyway),
> 			 * + sometimes tst_res(TCONF, ..) would be preferred over
> 			 * tst_brk(TCONF, ...)
> 			 */
> 			if (safe)
> 				tst_brk(TCONF,
> 					"overlayfs is not configured in this kernel.");
> 			} else {
> 				tst_res(TINFO,
> 					"overlayfs is not configured in this kernel.");
> 				return 1;
> 			}
> 		}
> 		tst_brk(TBROK | TERRNO, "overlayfs mount failed");

Hmm.. This would changed "test logic" in the existing 4 testcases.

Murphy
> 	}
> }
> 
> #define SAFE_MOUNT_OVERLAY() \
> 	mount_overlay(__FILE__, __LINE__, 1);
> 
> #define TST_MOUNT_OVERLAY() \
> 	mount_overlay(__FILE__, __LINE__, 0);
> 
> Kind regards,
> Petr
> 
> [1] http://lists.linux.it/pipermail/ltp/2019-May/011983.html