Skip to contentSkip to conversation

OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

PATCH New Murphy Zhou 4 participants 39 messages Apr 30, 2019 – Jun 3, 2019 Library code

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

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.

---
 .../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(-)
testcases/kernel/syscalls/execveat/execveat03.c +15 -6
index def33923b..8ba6a656f 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify07.c +16 -8
index 96370b5cf..3e2d06485 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify08.c +17 -9
index acdb95345..a512dfe22 100644
@@ -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,
Signature · 1 line
2.21.0
#1 lore raw

PATCH 1/1 OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

Quoted 11 lines · 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.

LGTM.

Signature · 5 lines
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190430/d0fb3739/attachment.html>
#2 lore raw

PATCH 1/1 OVL_MNT: put overlayfs lower, upper, work, mnt dir in separated mountpoint

In reply to Murphy Zhou

Hi,

Quoted 18 lines
 testcases/kernel/syscalls/inotify/inotify07.c | 24 +++++++++++------
testcases/kernel/syscalls/execveat/execveat03.c +8 -1
-#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)

testcases/kernel/syscalls/execveat/execveat03.c static void setup(void) · line 104

Quoted 10 lines
testcases/kernel/syscalls/execveat/execveat03.c +3 -3
@@ -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

#3 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

To create overlayfs dirs, and mount overlayfs if needed.

---
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
include/tst_fs.h +12 -0
index 423ca82ec..750bb1a91 100644
@@ -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
lib/tst_fs_setup.c new file +42 -0

42 lines not on this page · raw message

Signature · 1 line
2.21.0
#4 lore raw

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

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.

---
 .../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(-)
testcases/kernel/syscalls/execveat/execveat03.c +6 -18
index def33923b..ff4de62ee 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify07.c +8 -9
index 96370b5cf..fd0caff37 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify08.c +9 -10
index acdb95345..a17a6a5c7 100644
@@ -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,
testcases/kernel/syscalls/readahead/readahead02.c +4 -29
index 39ddbd583..1b6b70466 100644
@@ -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)
Signature · 1 line
2.21.0
#5 lore raw

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

Hi Murphy,

Quoted 1 line

--- a/testcases/kernel/syscalls/execveat/execveat03.c

...

Quoted 7 lines
testcases/kernel/syscalls/execveat/execveat03.c +0 -7
-	/* 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");

testcases/kernel/syscalls/execveat/execveat03.c static void verify_execveat(void) · line 93

Quoted 10 lines
testcases/kernel/syscalls/execveat/execveat03.c +2 -8
-	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

#6 lore raw

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

Quoted 14 lines · Petr Vorel

Hi Murphy,

Quoted 1 line

--- a/testcases/kernel/syscalls/execveat/execveat03.c

...

Quoted 10 lines
testcases/kernel/syscalls/execveat/execveat03.c +0 -10
-	/* 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,
Quoted 8 lines

testcases/kernel/syscalls/execveat/execveat03.c static void verify_execveat(void) · line 93

Quoted 7 lines
testcases/kernel/syscalls/execveat/execveat03.c +2 -5
-				"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

Quoted 5 lines

+ more info at 1st patch.

Kind regards,
Petr

#7 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

In reply to Murphy Zhou

Hi Murphy,

Quoted 1 line

To create overlayfs dirs, and mount overlayfs if needed.

...

lib/tst_fs_setup.c line 29

Quoted 13 lines
lib/tst_fs_setup.c new file +13 -0
+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).

lib/tst_fs_setup.c line 37

Quoted 8 lines
lib/tst_fs_setup.c new file +8 -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;

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().

lib/tst_fs_setup.c line 42

Quoted 5 lines
lib/tst_fs_setup.c new file +5 -0
+		}
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+	}
+	return 0;
+}

Kind regards,
Petr

#8 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Hi Murphy,

Quoted 18 lines

...

lib/tst_fs_setup.c line 29

Quoted 13 lines
lib/tst_fs_setup.c new file +13 -0
+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).

Quoted 14 lines

lib/tst_fs_setup.c line 37

Quoted 8 lines
lib/tst_fs_setup.c new file +8 -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;

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:

Quoted 22 lines

+++ b/testcases/kernel/syscalls/inotify/inotify07.c

(quoted diff) +1 -7
-#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);

Review static void setup(void) · line 174

Quoted 10 lines
(quoted diff) +5 -2
-	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

#9 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Quoted 69 lines · Petr Vorel

Hi Murphy,

Quoted 26 lines

...

lib/tst_fs_setup.c line 29

Quoted 13 lines
lib/tst_fs_setup.c new file +13 -0
+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).

lib/tst_fs_setup.c line 37

Quoted 8 lines
lib/tst_fs_setup.c new file +8 -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;
Quoted 6 lines

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:

Quoted 22 lines

+++ b/testcases/kernel/syscalls/inotify/inotify07.c

(quoted diff) +1 -7
-#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);

Review static void setup(void) · line 174

Quoted 10 lines
(quoted diff) +5 -2
-     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,
Quoted 8 lines

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.

#10 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Hi Amir, Murphy,

Quoted 7 lines

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.

Quoted 2 lines

Thanks,
Amir.

Kind regards,
Petr

#11 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Hi Amir and Petr,

Quoted 11 lines · Petr Vorel

Hi Amir, Murphy,

Quoted 7 lines

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.

Quoted 5 lines

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

Quoted 6 lines
Quoted 3 lines

Thanks,
Amir.

Kind regards,
Petr

#12 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

In reply to Petr Vorel

Hi Petr,

Quoted 64 lines · Petr Vorel

Hi Murphy,

Quoted 18 lines

...

lib/tst_fs_setup.c line 29

Quoted 13 lines
lib/tst_fs_setup.c new file +13 -0
+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).

Quoted 14 lines

lib/tst_fs_setup.c line 37

Quoted 8 lines
lib/tst_fs_setup.c new file +8 -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;

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:

Quoted 28 lines

+++ b/testcases/kernel/syscalls/inotify/inotify07.c

(quoted diff) +5 -9
-#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
Quoted 8 lines

Review static void setup(void) · line 174

Quoted 4 lines
(quoted diff) +1 -0
+		    ",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

Quoted 6 lines

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

Kind regards,
Petr

#13 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Hi Murphy,

Quoted 12 lines
Quoted 11 lines

Review

Quoted 7 lines
(quoted diff) +4 -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

Quoted 5 lines

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 :).

Quoted 4 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.

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.

Quoted 2 lines

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

#14 lore raw

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

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

---
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
include/safe_file_ops_fn.h +5 -0
index 35ec4fb1f..2491fa40c 100644
@@ -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 */
include/tst_fs.h +6 -0
index 423ca82ec..ce110b723 100644
@@ -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
include/tst_safe_file_ops.h +3 -0
index 5c3fea4e2..d211b40ff 100644
@@ -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 */
lib/tst_fs_setup.c new file +56 -0

56 lines not on this page · raw message

Signature · 1 line
2.21.0
#15 lore raw

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

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.

---
 .../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(-)
testcases/kernel/syscalls/execveat/execveat03.c +8 -16
index def33923b..7ec038134 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify07.c +9 -20
index 96370b5cf..c903032cb 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify08.c +10 -21
index acdb95345..fb80a55a5 100644
@@ -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,
testcases/kernel/syscalls/readahead/readahead02.c +7 -29
index 39ddbd583..57251c892 100644
@@ -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)
Signature · 1 line
2.21.0
#16 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

In reply to Petr Vorel
Quoted 22 lines · Petr Vorel

Hi Murphy,

Quoted 1 line

To create overlayfs dirs, and mount overlayfs if needed.

...

lib/tst_fs_setup.c line 29

Quoted 13 lines
lib/tst_fs_setup.c new file +13 -0
+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).

Quoted 11 lines

lib/tst_fs_setup.c line 37

Quoted 8 lines
lib/tst_fs_setup.c new file +8 -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;

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

Quoted 12 lines

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().

lib/tst_fs_setup.c line 42

Quoted 5 lines
lib/tst_fs_setup.c new file +5 -0
+		}
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+	}
+	return 0;
+}

Kind regards,
Petr

#17 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Hi Murphy,

Quoted 11 lines
Quoted 10 lines

lib/tst_fs_setup.c line 37

Quoted 7 lines
lib/tst_fs_setup.c new file +7 -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;

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,

Quoted 5 lines

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

#18 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Quoted 63 lines · Petr Vorel

Hi Murphy,

Quoted 11 lines
Quoted 10 lines

lib/tst_fs_setup.c line 37

Quoted 7 lines

+ ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER

lib/tst_fs_setup.c new file +6 -0
+             ",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,

Quoted 5 lines

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");
        }
}
Quoted 6 lines
#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.

#19 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Hi Amir,

thanks for your suggestions, highly appreciated :).
...

Quoted 7 lines
Quoted 5 lines
#define SAFE_MOUNT_OVERLAY() \
        mount_overlay(__FILE__, __LINE__, 1);
#define TST_MOUNT_OVERLAY() \
        mount_overlay(__FILE__, __LINE__, 0);
Quoted 7 lines

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

Quoted 2 lines

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.

Quoted 3 lines

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

Quoted 2 lines

Thanks,
Amir.

Kind regards,
Petr

#20 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

Hi guys,

Very good ideas! Thank you both very much!

M

Quoted 39 lines · Petr Vorel

Hi Amir,

thanks for your suggestions, highly appreciated :).
...

Quoted 7 lines
Quoted 5 lines
#define SAFE_MOUNT_OVERLAY() \
        mount_overlay(__FILE__, __LINE__, 1);
#define TST_MOUNT_OVERLAY() \
        mount_overlay(__FILE__, __LINE__, 0);
Quoted 7 lines

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

Quoted 2 lines

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.

Quoted 3 lines

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

Quoted 2 lines

Thanks,
Amir.

Kind regards,
Petr

#21 lore raw

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

In reply to Petr Vorel

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;

---
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
include/safe_file_ops_fn.h +4 -0
index 35ec4fb1f..526454da7 100644
@@ -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 */
include/tst_fs.h +6 -0
index 423ca82ec..ce110b723 100644
@@ -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
include/tst_safe_file_ops.h +6 -0
index 5c3fea4e2..b62a7447f 100644
@@ -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 */
lib/tst_fs_setup.c new file +46 -0

46 lines not on this page · raw message

Signature · 1 line
2.21.0
#22 lore raw

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

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.

---
 .../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(-)
testcases/kernel/syscalls/execveat/execveat03.c +6 -17
index def33923b..446ff4f1d 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify07.c +7 -20
index 96370b5cf..7d6aba51f 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify08.c +8 -21
index acdb95345..929ed08f1 100644
@@ -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,
testcases/kernel/syscalls/readahead/readahead02.c +5 -29
index 39ddbd583..db95c2b01 100644
@@ -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)
Signature · 1 line
2.21.0
#23 lore raw

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

In reply to Murphy Zhou

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:

tst_fs_setup.c +27 -27
index de09c7135..60dedc283 100644
@@ -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:

kernel/syscalls/execveat/execveat03.c +0 -2
index 446ff4f1d..eb088bc4c 100644
@@ -88,8 +88,6 @@ static void verify_execveat(void)
 static void setup(void)
 {
 	check_execveat();
-
-	create_overlay_dirs();
 	SAFE_MOUNT_OVERLAY();
 	ovl_mounted = 1;
 }
kernel/syscalls/inotify/inotify08.c +1 -1
index 929ed08f1..9433315e2 100644
@@ -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");
kernel/syscalls/readahead/readahead02.c +0 -1
index db95c2b01..f6e07173d 100644
@@ -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();
 }
#24 lore raw

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

Quoted 19 lines · 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).

Quoted 7 lines

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

Quoted 6 lines

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.

#25 lore raw

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

Hi Murphy, Amir,

thanks a lot for your answers.

Quoted 7 lines
Quoted 6 lines

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?

Quoted 6 lines

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.

Quoted 2 lines

Thanks,
Amir.

Kind regards,
Petr

#26 lore raw

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

In reply to Petr Vorel

Hi Petr,

Quoted 11 lines · 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)

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

Quoted 3 lines

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

Agree.

Quoted 3 lines

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!

Quoted 9 lines

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.

Quoted 5 lines

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

Quoted 122 lines

Kind regards,
Petr
---
* diff to first commit:

tst_fs_setup.c +27 -27
index de09c7135..60dedc283 100644
@@ -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:

kernel/syscalls/execveat/execveat03.c +0 -2
index 446ff4f1d..eb088bc4c 100644
@@ -88,8 +88,6 @@ static void verify_execveat(void)
 static void setup(void)
 {
 	check_execveat();
-
-	create_overlay_dirs();
 	SAFE_MOUNT_OVERLAY();
 	ovl_mounted = 1;
 }
kernel/syscalls/inotify/inotify08.c +1 -1
index 929ed08f1..9433315e2 100644
@@ -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");
kernel/syscalls/readahead/readahead02.c +0 -1
index db95c2b01..f6e07173d 100644
@@ -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();
 }
#27 lore raw

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

Hi Amir, Murphy,

Quoted 10 lines
Quoted 4 lines

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.

Quoted 5 lines

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).

Quoted 2 lines

Thanks!
M

Kind regards,
Petr

#28 lore raw

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

Quoted 12 lines · Petr Vorel

Hi Amir, Murphy,

Quoted 9 lines
Quoted 4 lines

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.

Quoted 8 lines
Quoted 5 lines

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!

Quoted 7 lines
Quoted 3 lines

Thanks!
M

Kind regards,
Petr

#29 lore raw

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

In reply to Petr Vorel

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;

---
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
include/safe_file_ops_fn.h +4 -0
index 35ec4fb1f..052fb1b9a 100644
@@ -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 */
include/tst_fs.h +6 -0
index b2b19ada6..ebca065c6 100644
@@ -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
include/tst_safe_file_ops.h +6 -0
index 5c3fea4e2..b62a7447f 100644
@@ -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 */
include/tst_test.h +1 -0
index e4b935c45..f3b8901a2 100644
@@ -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;
lib/tst_fs_setup.c new file +48 -0

48 lines not on this page · raw message

lib/tst_test.c +15 -0
index 2d88adbd7..95f389d2e 100644
@@ -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);
 
Signature · 1 line
2.21.0
#30 lore raw

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

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.

---
 .../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(-)
testcases/kernel/syscalls/execveat/execveat03.c +4 -27
index def33923b..7df1b0faa 100644
@@ -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,
 };
testcases/kernel/syscalls/inotify/inotify07.c +8 -25
index 96370b5cf..47817cd42 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify08.c +10 -27
index acdb95345..067c01dbb 100644
@@ -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,
testcases/kernel/syscalls/readahead/readahead02.c +4 -29
index 39ddbd583..f6e07173d 100644
@@ -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)
Signature · 1 line
2.21.0
#31 lore raw

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

Quoted 13 lines · 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.

Quoted 6 lines
---
 .../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! :-)

Quoted 298 lines
testcases/kernel/syscalls/execveat/execveat03.c +4 -27
index def33923b..7df1b0faa 100644
@@ -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,
 };
testcases/kernel/syscalls/inotify/inotify07.c +8 -25
index 96370b5cf..47817cd42 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify08.c +10 -27
index acdb95345..067c01dbb 100644
@@ -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,
testcases/kernel/syscalls/readahead/readahead02.c +4 -30
index 39ddbd583..f6e07173d 100644
@@ -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

#32 lore raw

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

In reply to Murphy Zhou
Quoted 9 lines · 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;

Quoted 4 lines

---

Very nice! just one minor comment, otherwise

Quoted 98 lines

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
include/safe_file_ops_fn.h +4 -0
index 35ec4fb1f..052fb1b9a 100644
@@ -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 */
include/tst_fs.h +6 -0
index b2b19ada6..ebca065c6 100644
@@ -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
include/tst_safe_file_ops.h +6 -0
index 5c3fea4e2..b62a7447f 100644
@@ -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 */
include/tst_test.h +1 -0
index e4b935c45..f3b8901a2 100644
@@ -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;
lib/tst_fs_setup.c new file +11 -0
new file mode 100644
index 000000000..e8ae929d7
@@ -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"

lib/tst_fs_setup.c line 21

Quoted 10 lines
lib/tst_fs_setup.c new file +10 -0
+#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)?

lib/tst_fs_setup.c line 25

Quoted 4 lines
lib/tst_fs_setup.c new file +4 -0
+       }
+       closedir(dir);
+}
+

Thanks for doing this work!
Amir.

#33 lore raw

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

Quoted 10 lines · Amir Goldstein
Quoted 9 lines · 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;

Quoted 6 lines
Quoted 4 lines

---

Very nice! just one minor comment, otherwise

Thank you very much!

Quoted 105 lines
Quoted 102 lines

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
include/safe_file_ops_fn.h +4 -0
index 35ec4fb1f..052fb1b9a 100644
@@ -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 */
include/tst_fs.h +6 -0
index b2b19ada6..ebca065c6 100644
@@ -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
include/tst_safe_file_ops.h +6 -0
index 5c3fea4e2..b62a7447f 100644
@@ -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 */
include/tst_test.h +1 -0
index e4b935c45..f3b8901a2 100644
@@ -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;
lib/tst_fs_setup.c new file +15 -0
new file mode 100644
index 000000000..e8ae929d7
@@ -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)
+{
Quoted 8 lines

lib/tst_fs_setup.c line 21

Quoted 6 lines
lib/tst_fs_setup.c new file +6 -0
+       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

Quoted 8 lines

lib/tst_fs_setup.c line 25

Quoted 5 lines
lib/tst_fs_setup.c new file +4 -0
+       }
+       closedir(dir);
+}
+

Thanks for doing this work!
Amir.

#34 lore raw

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

In reply to Amir Goldstein

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;

---
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
include/safe_file_ops_fn.h +4 -0
index 35ec4fb1f..052fb1b9a 100644
@@ -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 */
include/tst_fs.h +6 -0
index b2b19ada6..ebca065c6 100644
@@ -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
include/tst_safe_file_ops.h +6 -0
index 5c3fea4e2..b62a7447f 100644
@@ -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 */
include/tst_test.h +1 -0
index e4b935c45..f3b8901a2 100644
@@ -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;
lib/tst_fs_setup.c new file +49 -0

49 lines not on this page · raw message

lib/tst_test.c +15 -0
index 2d88adbd7..95f389d2e 100644
@@ -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);
 
Signature · 1 line
2.21.0
#35 lore raw

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

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.

---
 .../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(-)
testcases/kernel/syscalls/execveat/execveat03.c +4 -27
index def33923b..7df1b0faa 100644
@@ -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,
 };
testcases/kernel/syscalls/inotify/inotify07.c +8 -25
index 96370b5cf..47817cd42 100644
@@ -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,
testcases/kernel/syscalls/inotify/inotify08.c +10 -27
index acdb95345..067c01dbb 100644
@@ -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,
testcases/kernel/syscalls/readahead/readahead02.c +4 -29
index 39ddbd583..f6e07173d 100644
@@ -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)
Signature · 1 line
2.21.0
#36 lore raw

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

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

#37 lore raw

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

In reply to Murphy Zhou

Hi Murphy,

Quoted 11 lines

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;

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

Kind regards,
Petr

tst_fs_setup.c +1 -1
index 32a6218e3..54ea37077 100644
@@ -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,
#38 lore raw

PATCH v2 1/2 OVL_MNT: add setup_overlay helper

In reply to Petr Vorel

Hi Petr,

Quoted 51 lines · Petr Vorel

Hi Murphy,

Quoted 11 lines
Quoted 10 lines

lib/tst_fs_setup.c line 37

Quoted 7 lines
lib/tst_fs_setup.c new file +7 -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;

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,

Quoted 5 lines

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,
Quoted 8 lines
					"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

Quoted 13 lines
	}
}
#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