syscalls/open11.c: Fix the failure of opening device special file

1) If temporary directory(TMPDIR/TEMPDIR) is on the mountpoint which
is mounted with nodev option(or temporary directory is mounted with
with nodev option directly), open(2) has no permission to open device
special file in the temporary directory and failed as expected. as below:
--------------------------------------------------------------------------
open11.c:351: Open device special file O_RDONLY - expected errno 0 - Got: TEST_ERRNO=EACCES(13): Permission denied open11.c:351: Open device special file O_WRONLY - expected errno 0 - Got: TEST_ERRNO=EACCES(13): Permission denied open11.c:351: Open device special file O_RDWR - expected errno 0 - Got: TEST_ERRNO=EACCES(13): Permission denied
--------------------------------------------------------------------------
We try to fix this issue by creating device special file in /dev/ instead.

2) Cleanup && Convert to new API.

Signed-off-by Xiao Yang <[email protected]>

---
 testcases/kernel/syscalls/open/open11.c | 165 +++++++++-----------------------
 1 file changed, 45 insertions(+), 120 deletions(-)

testcases/kernel/syscalls/open/open11.c +45 -120

index 78002f3..c540eb8 100644
@@ -1,28 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (C) 2013 Red Hat, Inc.
  *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- */
-
-/*
  * Basic tests for open(2) and make sure open(2) works and handles error
  * conditions correctly.
  *

@@ -60,25 +39,15 @@
  */
 
 #define _GNU_SOURCE
-#include "config.h"
+#include <errno.h>
+#include <sys/sysmacros.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <sys/sysmacros.h>
-#include <sys/wait.h>
-#include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include "test.h"
-#include "safe_macros.h"
 
-char *TCID = "open11";
+#include "tst_test.h"
 
-/* Define test files */
 #define T_REG "t_reg"			/* regular file with content */
 #define T_REG_EMPTY "t_reg_empty"	/* empty regular file */
 #define T_LINK_REG "t_link_reg"		/* hard link to T_REG */

@@ -86,24 +55,18 @@ char *TCID = "open11";
 #define T_SYMLINK_REG "t_symlink_reg"	/* symlink to T_REG */
 #define T_DIR "t_dir"			/* test dir */
 #define T_SYMLINK_DIR "t_symlink_dir"	/* symlink to T_DIR */
-#define T_DEV "t_dev"			/* test device special file */
+#define T_DEV "/dev/t_dev"		/* test device special file */
 
 #define T_MSG "this is a test string"
 
-static void setup(void);
-static void cleanup(void);
-
-struct test_case {
+static struct test_case {
 	char *desc;
 	char *path;
 	int flags;
 	mode_t mode;
 	int err;
-};
-struct test_case tc[] = {
-	/*
-	 * Test open(2) regular file
-	 */
+} tc[] = {
+	/* Test open(2) regular file */
 	{	/* open regular file O_RDONLY */
 		.desc = "Open regular file O_RDONLY",
 		.path = T_REG_EMPTY,

@@ -139,9 +102,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * Test open(2) directory
-	 */
+	/* Test open(2) directory */
 	{	/* open dir O_RDONLY */
 		.desc = "Open dir O_RDONLY",
 		.path = T_DIR,

@@ -163,9 +124,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = ENOTDIR,
 	},
-	/*
-	 * Test open(2) hard link
-	 */
+	/* Test open(2) hard link */
 	{	/* open hard link file O_RDONLY */
 		.desc = "Open hard link file O_RDONLY",
 		.path = T_LINK_REG,

@@ -187,9 +146,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * Test open(2) sym link
-	 */
+	/* Test open(2) sym link */
 	{	/* open sym link file O_RDONLY */
 		.desc = "Open sym link file O_RDONLY",
 		.path = T_SYMLINK_REG,

@@ -232,9 +189,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = EISDIR,
 	},
-	/*
-	 * Test open(2) device special
-	 */
+	/* * Test open(2) device special */
 	{	/* open device special file O_RDONLY */
 		.desc = "Open device special file O_RDONLY",
 		.path = T_DEV,

@@ -256,9 +211,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * Test open(2) non-existing file
-	 */
+	/* * Test open(2) non-existing file */
 	{	/* open non-existing regular file in existing dir */
 		.desc = "Open non-existing regular file in existing dir",
 		.path = T_DIR"/"T_NEW_REG,

@@ -266,9 +219,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * test open(2) with O_CREAT
-	 */
+	/* test open(2) with O_CREAT */
 	{	/* open hard link file O_RDONLY | O_CREAT */
 		.desc = "Open link file O_RDONLY | O_CREAT",
 		.path = T_LINK_REG,

@@ -304,9 +255,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = EISDIR,
 	},
-	/*
-	 * Other random open(2) tests
-	 */
+	/* Other random open(2) tests */
 	{	/* open regular file O_RDONLY | O_TRUNC */
 		.desc = "Open regular file O_RDONLY | O_TRUNC, "
 			"behaviour is undefined but should not oops or hang",

@@ -325,42 +274,22 @@ struct test_case tc[] = {
 	},
 };
 
-int TST_TOTAL = sizeof(tc) / sizeof(tc[0]);
-
-int main(int argc, char *argv[])
+static void verify_open(unsigned int n)
 {
-	int lc;
-	int i;
 	int fd;
-	int ret;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
+	TEST(open(tc[n].path, tc[n].flags, tc[n].mode));
+	fd = TEST_RETURN;
 
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		for (i = 0; i < TST_TOTAL; i++) {
-			TEST(open(tc[i].path, tc[i].flags, tc[i].mode));
-			fd = TEST_RETURN;
-
-			if (tc[i].err == -1 || TEST_ERRNO == tc[i].err) {
-				tst_resm(TPASS, "%s", tc[i].desc);
-			} else {
-				tst_resm(TFAIL | TTERRNO,
-					 "%s - expected errno %d - Got",
-					 tc[i].desc, tc[i].err);
-			}
-			if (fd > 0) {
-				ret = close(fd);
-				if (ret < 0)
-					tst_resm(TWARN, "%s - close failed: %s",
-						 tc[i].desc, strerror(errno));
-			}
-		}
+	if (tc[n].err == -1 || TEST_ERRNO == tc[n].err) {
+		tst_res(TPASS, "%s", tc[n].desc);
+	} else {
+		tst_res(TFAIL | TTERRNO, "%s - expected errno %d - Got",
+			tc[n].desc, tc[n].err);
 	}
 
-	cleanup();
-	tst_exit();
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static void setup(void)

@@ -368,36 +297,32 @@ static void setup(void)
 	int fd;
 	int ret;
 
-	tst_require_root();
-
-	tst_tmpdir();
-
-	/* Create test files */
-	fd = SAFE_OPEN(cleanup, T_REG, O_WRONLY | O_CREAT, 0644);
+	fd = SAFE_OPEN(T_REG, O_WRONLY | O_CREAT, 0644);
 	ret = write(fd, T_MSG, sizeof(T_MSG));
 	if (ret == -1) {
-		close(fd);
-		tst_brkm(TBROK | TERRNO, cleanup, "Write %s failed", T_REG);
+		SAFE_CLOSE(fd);
+		tst_brk(TBROK | TERRNO, "Write %s failed", T_REG);
 	}
-	close(fd);
-
-	fd = SAFE_CREAT(cleanup, T_REG_EMPTY, 0644);
-	close(fd);
-
-	SAFE_LINK(cleanup, T_REG, T_LINK_REG);
-	SAFE_SYMLINK(cleanup, T_REG, T_SYMLINK_REG);
-	SAFE_MKDIR(cleanup, T_DIR, 0755);
-	SAFE_SYMLINK(cleanup, T_DIR, T_SYMLINK_DIR);
+	SAFE_CLOSE(fd);
 
-	ret = mknod(T_DEV, S_IFCHR, makedev(1, 5));
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "Create char dev %s failed",
-			 T_DEV);
-
-	TEST_PAUSE;
+	SAFE_TOUCH(T_REG_EMPTY, 0644, NULL);
+	SAFE_LINK(T_REG, T_LINK_REG);
+	SAFE_SYMLINK(T_REG, T_SYMLINK_REG);
+	SAFE_MKDIR(T_DIR, 0755);
+	SAFE_SYMLINK(T_DIR, T_SYMLINK_DIR);
+	SAFE_MKNOD(T_DEV, S_IFCHR, makedev(1, 5));
 }
 
 static void cleanup(void)
 {
-	tst_rmdir();
+	if (!access(T_DEV, F_OK))
+		SAFE_UNLINK(T_DEV);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tc),
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = verify_open,
+	.needs_tmpdir = 1,
+};

1) If temporary directory(TMPDIR/TEMPDIR) is on the mountpoint which
is mounted with nodev option(or temporary directory is mounted with
with nodev option directly), open(2) has no permission to open device
special file in the temporary directory and failed as expected. as below:
--------------------------------------------------------------------------
open11.c:351: Open device special file O_RDONLY - expected errno 0 - Got: TEST_ERRNO=EACCES(13): Permission denied open11.c:351: Open device special file O_WRONLY - expected errno 0 - Got: TEST_ERRNO=EACCES(13): Permission denied open11.c:351: Open device special file O_RDWR - expected errno 0 - Got: TEST_ERRNO=EACCES(13): Permission denied
--------------------------------------------------------------------------
We try to fix this issue by creating device special file in /dev/ instead.

2) Cleanup && Convert to new API.

Signed-off-by Xiao Yang <[email protected]>

---
 testcases/kernel/syscalls/open/open11.c | 165 +++++++++-----------------------
 1 file changed, 45 insertions(+), 120 deletions(-)

testcases/kernel/syscalls/open/open11.c +45 -120

index 78002f3..c540eb8 100644
@@ -1,28 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (C) 2013 Red Hat, Inc.
  *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- */
-
-/*
  * Basic tests for open(2) and make sure open(2) works and handles error
  * conditions correctly.
  *

@@ -60,25 +39,15 @@
  */
 
 #define _GNU_SOURCE
-#include "config.h"
+#include <errno.h>
+#include <sys/sysmacros.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <sys/sysmacros.h>
-#include <sys/wait.h>
-#include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include "test.h"
-#include "safe_macros.h"
 
-char *TCID = "open11";
+#include "tst_test.h"
 
-/* Define test files */
 #define T_REG "t_reg"			/* regular file with content */
 #define T_REG_EMPTY "t_reg_empty"	/* empty regular file */
 #define T_LINK_REG "t_link_reg"		/* hard link to T_REG */

@@ -86,24 +55,18 @@ char *TCID = "open11";
 #define T_SYMLINK_REG "t_symlink_reg"	/* symlink to T_REG */
 #define T_DIR "t_dir"			/* test dir */
 #define T_SYMLINK_DIR "t_symlink_dir"	/* symlink to T_DIR */
-#define T_DEV "t_dev"			/* test device special file */
+#define T_DEV "/dev/t_dev"		/* test device special file */
 
 #define T_MSG "this is a test string"
 
-static void setup(void);
-static void cleanup(void);
-
-struct test_case {
+static struct test_case {
 	char *desc;
 	char *path;
 	int flags;
 	mode_t mode;
 	int err;
-};
-struct test_case tc[] = {
-	/*
-	 * Test open(2) regular file
-	 */
+} tc[] = {
+	/* Test open(2) regular file */
 	{	/* open regular file O_RDONLY */
 		.desc = "Open regular file O_RDONLY",
 		.path = T_REG_EMPTY,

@@ -139,9 +102,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * Test open(2) directory
-	 */
+	/* Test open(2) directory */
 	{	/* open dir O_RDONLY */
 		.desc = "Open dir O_RDONLY",
 		.path = T_DIR,

@@ -163,9 +124,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = ENOTDIR,
 	},
-	/*
-	 * Test open(2) hard link
-	 */
+	/* Test open(2) hard link */
 	{	/* open hard link file O_RDONLY */
 		.desc = "Open hard link file O_RDONLY",
 		.path = T_LINK_REG,

@@ -187,9 +146,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * Test open(2) sym link
-	 */
+	/* Test open(2) sym link */
 	{	/* open sym link file O_RDONLY */
 		.desc = "Open sym link file O_RDONLY",
 		.path = T_SYMLINK_REG,

@@ -232,9 +189,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = EISDIR,
 	},
-	/*
-	 * Test open(2) device special
-	 */
+	/* * Test open(2) device special */
 	{	/* open device special file O_RDONLY */
 		.desc = "Open device special file O_RDONLY",
 		.path = T_DEV,

@@ -256,9 +211,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * Test open(2) non-existing file
-	 */
+	/* * Test open(2) non-existing file */
 	{	/* open non-existing regular file in existing dir */
 		.desc = "Open non-existing regular file in existing dir",
 		.path = T_DIR"/"T_NEW_REG,

@@ -266,9 +219,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = 0,
 	},
-	/*
-	 * test open(2) with O_CREAT
-	 */
+	/* test open(2) with O_CREAT */
 	{	/* open hard link file O_RDONLY | O_CREAT */
 		.desc = "Open link file O_RDONLY | O_CREAT",
 		.path = T_LINK_REG,

@@ -304,9 +255,7 @@ struct test_case tc[] = {
 		.mode = 0644,
 		.err = EISDIR,
 	},
-	/*
-	 * Other random open(2) tests
-	 */
+	/* Other random open(2) tests */
 	{	/* open regular file O_RDONLY | O_TRUNC */
 		.desc = "Open regular file O_RDONLY | O_TRUNC, "
 			"behaviour is undefined but should not oops or hang",

@@ -325,42 +274,22 @@ struct test_case tc[] = {
 	},
 };
 
-int TST_TOTAL = sizeof(tc) / sizeof(tc[0]);
-
-int main(int argc, char *argv[])
+static void verify_open(unsigned int n)
 {
-	int lc;
-	int i;
 	int fd;
-	int ret;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
+	TEST(open(tc[n].path, tc[n].flags, tc[n].mode));
+	fd = TEST_RETURN;
 
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		for (i = 0; i < TST_TOTAL; i++) {
-			TEST(open(tc[i].path, tc[i].flags, tc[i].mode));
-			fd = TEST_RETURN;
-
-			if (tc[i].err == -1 || TEST_ERRNO == tc[i].err) {
-				tst_resm(TPASS, "%s", tc[i].desc);
-			} else {
-				tst_resm(TFAIL | TTERRNO,
-					 "%s - expected errno %d - Got",
-					 tc[i].desc, tc[i].err);
-			}
-			if (fd > 0) {
-				ret = close(fd);
-				if (ret < 0)
-					tst_resm(TWARN, "%s - close failed: %s",
-						 tc[i].desc, strerror(errno));
-			}
-		}
+	if (tc[n].err == -1 || TEST_ERRNO == tc[n].err) {
+		tst_res(TPASS, "%s", tc[n].desc);
+	} else {
+		tst_res(TFAIL | TTERRNO, "%s - expected errno %d - Got",
+			tc[n].desc, tc[n].err);
 	}
 
-	cleanup();
-	tst_exit();
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static void setup(void)

@@ -368,36 +297,32 @@ static void setup(void)
 	int fd;
 	int ret;
 
-	tst_require_root();
-
-	tst_tmpdir();
-
-	/* Create test files */
-	fd = SAFE_OPEN(cleanup, T_REG, O_WRONLY | O_CREAT, 0644);
+	fd = SAFE_OPEN(T_REG, O_WRONLY | O_CREAT, 0644);
 	ret = write(fd, T_MSG, sizeof(T_MSG));
 	if (ret == -1) {
-		close(fd);
-		tst_brkm(TBROK | TERRNO, cleanup, "Write %s failed", T_REG);
+		SAFE_CLOSE(fd);
+		tst_brk(TBROK | TERRNO, "Write %s failed", T_REG);
 	}
-	close(fd);
-
-	fd = SAFE_CREAT(cleanup, T_REG_EMPTY, 0644);
-	close(fd);
-
-	SAFE_LINK(cleanup, T_REG, T_LINK_REG);
-	SAFE_SYMLINK(cleanup, T_REG, T_SYMLINK_REG);
-	SAFE_MKDIR(cleanup, T_DIR, 0755);
-	SAFE_SYMLINK(cleanup, T_DIR, T_SYMLINK_DIR);
+	SAFE_CLOSE(fd);
 
-	ret = mknod(T_DEV, S_IFCHR, makedev(1, 5));
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "Create char dev %s failed",
-			 T_DEV);
-
-	TEST_PAUSE;
+	SAFE_TOUCH(T_REG_EMPTY, 0644, NULL);
+	SAFE_LINK(T_REG, T_LINK_REG);
+	SAFE_SYMLINK(T_REG, T_SYMLINK_REG);
+	SAFE_MKDIR(T_DIR, 0755);
+	SAFE_SYMLINK(T_DIR, T_SYMLINK_DIR);
+	SAFE_MKNOD(T_DEV, S_IFCHR, makedev(1, 5));
 }
 
 static void cleanup(void)
 {
-	tst_rmdir();
+	if (!access(T_DEV, F_OK))
+		SAFE_UNLINK(T_DEV);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tc),
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = verify_open,
+	.needs_tmpdir = 1,
+};

Signature · 7 lines

1.8.3.1




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

We add .needs_devfs flag to prepare a suitable filesystem to test device special files and use ext2 instead of tmpfs(because tmpfs doesn't support extended attributes) as default filesystem if .dev_fs_type is not specified.

Hi!

Quoted 4 lines

We add .needs_devfs flag to prepare a suitable filesystem to test device special files and use ext2 instead of tmpfs(because tmpfs doesn't support extended attributes) as default filesystem if .dev_fs_type is not specified.

Can we do that only if the test temporary directory is not suitable for creating devices?

There are distributions out there that have /tmp residing on pretty much normal filesystems, also user can override TMPDIR to point to a path backed up by a regular fs. In these cases this will slow down these tests for no good reason.

So I would rather go for:

if tst_test->needs_devfs is set:

* mkdir tst_test->mntpoint if it does not exits
* check if tst_test->mntpoint is suitable for creating devices
* if not, mount some filesystem over it
- mounting tmpfs without nodev flag should be more than enough and if that fails for you, we can fall back to a regular filesystem

lib/tst_test.c +8 -0

+	if (tst_path_has_mnt_flags(NULL, abs_path, flags)) {
+		tst_res(TINFO, "%s isn't suitable for creating devices, "
+			"so mount tmpfs without nodev over it", abs_path);
+		SAFE_MOUNT(NULL, mntpoint, "tmpfs", 0, NULL);
+		mntpoint_mounted = 1;
+	}
+}
+
 static void prepare_device(void)
 {
 	if (tst_test->format_device) {

Hi!

lib/tst_test.c static int prepare_and_mount_ro_fs(const char *dev, · line 732

Quoted 6 lines

lib/tst_test.c +6 -0

+static void prepare_and_mount_dev_fs(const char *mntpoint)
+{
+	const char *flags[] = {"nodev", NULL};
+	char abs_path[PATH_MAX];
+
+	sprintf(abs_path, "%s/%s", tst_get_tmpdir(), mntpoint);

The tst_get_tmpdir() allocates memory and also we are sure that mntpoint is on the same filesystem as the path returned by tst_get_tmpdir().

So I would suggest something like this:

	char *tmpdir;
	int mounted_nodev;

	tmpdir = tst_get_tmpdir();
	mounted_nodev = tst_path_has_mnt_flags(NULL, tmpdir, flags);
	free(tmpdir);

	if (mounted_nodev) {
		...
	}

Quoted 12 lines

lib/tst_test.c +8 -0

+	if (tst_path_has_mnt_flags(NULL, abs_path, flags)) {
+		tst_res(TINFO, "%s isn't suitable for creating devices, "
+			"so mount tmpfs without nodev over it", abs_path);
+		SAFE_MOUNT(NULL, mntpoint, "tmpfs", 0, NULL);
+		mntpoint_mounted = 1;
+	}
+}
+
  static void prepare_device(void)
  {
  	if (tst_test->format_device) {

lib/tst_test.c static void do_setup(int argc, char *argv[]) · line 808

Quoted 10 lines

lib/tst_test.c +3 -2

@@ -786,11 +801,15 @@ static void do_setup(int argc, char *argv[])
  	if (tst_test->mntpoint)
  		SAFE_MKDIR(tst_test->mntpoint, 0777);
 
-	if ((tst_test->needs_rofs || tst_test->mount_device ||
-	     tst_test->all_filesystems)&&  !tst_test->mntpoint) {
+	if ((tst_test->needs_devfs || tst_test->needs_rofs ||
+	     tst_test->mount_device || tst_test->all_filesystems)&&
+	     !tst_test->mntpoint) {
  		tst_brk(TBROK, "tst_test->mntpoint must be set!");
  	}

I guess that we should also make sure only one of needs_rofs, needs_devfs or needs_device is set, because otherwise we would attempt to mount multiple filesystems over the mntpoint.

something as:

	if (!!tst_test->needs_rofs +
	    !!tst_test->needs_devfs +
	    !!tst_test->needs_device>  1) {
		tst_brk(TBROK, "Two or more of needs_{rofs, devfs, device} are set");
	}

lib/tst_test.c static void do_setup(int argc, char *argv[]) · line 815

Quoted 6 lines

lib/tst_test.c +3 -0

+	if (tst_test->needs_devfs)
+		prepare_and_mount_dev_fs(tst_test->mntpoint);
+
  	if (tst_test->needs_rofs) {
  		/* If we failed to mount read-only tmpfs. Fallback to
  		 * using a device with read-only filesystem.

Other than the minor nits this version looks fine.

Hi!

lib/tst_test.c static int prepare_and_mount_ro_fs(const char *dev, · line 739

Quoted 13 lines

lib/tst_test.c +13 -0

+static void prepare_and_mount_dev_fs(const char *mntpoint)
+{
+	const char *flags[] = {"nodev", NULL};
+	int mounted_nodev;
+
+	mounted_nodev = tst_path_has_mnt_flags(NULL, NULL, flags);
+	if (mounted_nodev) {
+		tst_res(TINFO, "tmpdir isn't suitable for creating devices, "
+			"so mount tmpfs without nodev on %s", mntpoint);
+		SAFE_MOUNT(NULL, mntpoint, "tmpfs", 0, NULL);
+		mntpoint_mounted = 1;
+	}
+}

That is even better than my version, nice :-).

There is a last nit to solve, the problem is that tst_path_has_mnt_flags() is defined in old/test.h, we have to move the definition to a separate header file (in a separate patch) so that it could be included in test.h, tst_path_has_mnt_flags.c, and tst_test.c.

I can do that or you can sent v5, your choice.

This is a patch set, and i have factored out tst_path_has_mnt_flags() by the first patch:
http://lists.linux.it/pipermail/ltp/2018-August/009013.html