diff mbox series

[v2,1/2] Fix unlink09 test

Message ID 20240605-unlink09-v2-1-bfa0bbdc8753@suse.com
State Superseded
Headers show
Series Fix unlink09 test | expand

Commit Message

Andrea Cervesato June 5, 2024, 2:40 p.m. UTC 
From: Andrea Cervesato <andrea.cervesato@suse.com>

This patch will fix unlink09 test by checking for filesystems which
are not supporting inode attributes. At the same time, it removes the
read-only filesystem unlink() check in order to add .all_filesystems
support.

Fixes: 2cf78f47a6 (unlink: Add error tests for EPERM and EROFS)
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/syscalls/unlink/unlink09.c | 94 +++++++++++++++--------------
 1 file changed, 50 insertions(+), 44 deletions(-)

Comments

Petr Vorel June 19, 2024, 3:50 p.m. UTC | #1 
Hi Andrea,

> This patch will fix unlink09 test by checking for filesystems which
> are not supporting inode attributes. At the same time, it removes the
> read-only filesystem unlink() check in order to add .all_filesystems
> support.

...
> -	TST_EXP_FAIL(unlink(tc->filename), tc->expected_errno, "%s", tc->desc);
> +	TST_EXP_FAIL(unlink(tc->filename), EPERM, "%s", tc->desc);

>  	/* If unlink() succeeded unexpectedly, test file should be restored. */
>  	if (!TST_RET) {
> -		if (tc->fd) {
> -			*(tc->fd) = SAFE_OPEN(tc->filename, O_CREAT, 0600);
> -			if (tc->flag) {
> -				SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS, &attr);
> -				attr |= tc->flag;
> -				SAFE_IOCTL(*(tc->fd), FS_IOC_SETFLAGS, &attr);
> -			}
> -		} else {
> -			SAFE_TOUCH(tc->filename, 0600, 0);
> -		}
> +		*(tc->fd) = SAFE_CREAT(tc->filename, 0600);

FYI, when forcing NTFS, we get EINVAL, reported by Avinesh:

LTP_SINGLE_FS_TYPE=ntfs ./unlink09
tst_test.c:1120: TINFO: Mounting /dev/loop2 to /tmp/LTP_unlqPsIkB/mnt fstyp=ntfs flags=0
tst_test.c:1120: TINFO: Trying FUSE...
unlink09.c:73: TBROK: ioctl(3,((((1U) << (((0+8)+8)+14)) | ((('f')) << (0+8)) | (((2)) << 0) | ((((sizeof(long)))) << ((0+8)+8)))),...) failed: EINVAL (22)
unlink09.c:42: TWARN: ioctl(3,((((2U) << (((0+8)+8)+14)) | ((('f')) << (0+8)) | (((1)) << 0) | ((((sizeof(long)))) << ((0+8)+8)))),...) failed: EINVAL (22)
unlink09.c:49: TWARN: ioctl(3,((((1U) << (((0+8)+8)+14)) | ((('f')) << (0+8)) | (((2)) << 0) | ((((sizeof(long)))) << ((0+8)+8)))),...) failed: EINVAL (22)

I tried SAFE_OPEN with O_RDWR | O_CREAT (patch from Avinesh), but it did not help.
https://lore.kernel.org/ltp/20240603124653.31967-1-akumar@suse.de/
https://patchwork.ozlabs.org/project/ltp/patch/20240601195149.17570-1-akumar@suse.de/

It's not ENOTTY, thus it's not checked by inode attributes check.

> +		setup_inode_flag(*(tc->fd), tc->flag, 0);
>  	}
>  }

> @@ -96,7 +101,8 @@ static struct tst_test test = {
>  	.tcnt = ARRAY_SIZE(tcases),
>  	.cleanup = cleanup,
>  	.test = verify_unlink,
> -	.needs_rofs = 1,
> -	.mntpoint = DIR_EROFS,
> +	.mntpoint = MNTPOINT,
>  	.needs_root = 1,
> +	.all_filesystems = 1,
> +	.format_device = 1,

Test fails on exfat (on various kernel versions):

tst_test.c:1694: TINFO: === Testing on exfat ===
tst_test.c:1107: TINFO: Formatting /dev/loop0 with exfat opts='' extra opts=''
tst_test.c:1121: TINFO: Mounting /dev/loop0 to /tmp/LTP_unl7cHxvL/mnt fstyp=exfat flags=0
unlink09.c:69: TBROK: Inode attributes not supported by 'exfat'

i.e. different error than on ntfs, but the same as on vfat (which is also
disabled). I wonder why NTFS is different. Or is it fuse relevant?

With added exfat LGTM (I can change this before merge.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/unlink/unlink09.c b/testcases/kernel/syscalls/unlink/unlink09.c
index cc4b4a07e..220a810cf 100644
--- a/testcases/kernel/syscalls/unlink/unlink09.c
+++ b/testcases/kernel/syscalls/unlink/unlink09.c
@@ -2,92 +2,97 @@ 
 /*
  * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
  * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
 /*\
  * [Description]
  *
- * Verify that unlink(2) fails with
- *
- * - EPERM when target file is marked as immutable or append-only
- * - EROFS when target file is on a read-only filesystem.
+ * Verify that unlink(2) fails with EPERM when target file is marked as
+ * immutable or append-only.
  */
 
 #include <sys/ioctl.h>
 #include "tst_test.h"
 #include "lapi/fs.h"
 
-#define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
-#define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
-#define DIR_EROFS "erofs"
-#define TEST_EROFS "erofs/test_erofs"
+#define MNTPOINT "mnt"
+#define TEST_EPERM_IMMUTABLE MNTPOINT"/test_eperm_immutable"
+#define TEST_EPERM_APPEND_ONLY MNTPOINT"/test_eperm_append_only"
 
-static int fd_immutable;
-static int fd_append_only;
+static int fd_immutable = -1;
+static int fd_append_only = -1;
 
 static struct test_case_t {
 	char *filename;
 	int *fd;
 	int flag;
-	int expected_errno;
 	char *desc;
 } tcases[] = {
-	{TEST_EPERM_IMMUTABLE, &fd_immutable, FS_IMMUTABLE_FL, EPERM,
+	{TEST_EPERM_IMMUTABLE, &fd_immutable, FS_IMMUTABLE_FL,
 		"target file is immutable"},
-	{TEST_EPERM_APPEND_ONLY, &fd_append_only, FS_APPEND_FL, EPERM,
+	{TEST_EPERM_APPEND_ONLY, &fd_append_only, FS_APPEND_FL,
 		"target file is append-only"},
-	{TEST_EROFS, NULL, 0, EROFS, "target file in read-only filesystem"},
 };
 
+static void setup_inode_flag(const int fd, const int flag, const int reset)
+{
+	int attr;
+
+	SAFE_IOCTL(fd, FS_IOC_GETFLAGS, &attr);
+
+	if (reset)
+		attr &= ~flag;
+	else
+		attr |= flag;
+
+	SAFE_IOCTL(fd, FS_IOC_SETFLAGS, &attr);
+}
+
 static void setup(void)
 {
 	int attr;
 
-	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
-	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
+	fd_immutable = SAFE_CREAT(TEST_EPERM_IMMUTABLE, 0600);
+	TEST(ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr));
+
+	if (TST_RET == -1 && TST_ERR == ENOTTY) {
+		SAFE_CLOSE(fd_immutable);
+
+		tst_brk(TBROK, "Inode attributes not supported by '%s'",
+			tst_device->fs_type);
+	}
+
 	attr |= FS_IMMUTABLE_FL;
 	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
 
-	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
-	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
-	attr |= FS_APPEND_FL;
-	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
+	fd_append_only = SAFE_CREAT(TEST_EPERM_APPEND_ONLY, 0600);
+	setup_inode_flag(fd_append_only, FS_APPEND_FL, 0);
 }
 
 static void cleanup(void)
 {
-	int attr;
-
-	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
-	attr &= ~FS_IMMUTABLE_FL;
-	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
-	SAFE_CLOSE(fd_immutable);
+	if (fd_immutable != -1) {
+		setup_inode_flag(fd_immutable, FS_IMMUTABLE_FL, 1);
+		SAFE_CLOSE(fd_immutable);
+	}
 
-	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
-	attr &= ~FS_APPEND_FL;
-	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
-	SAFE_CLOSE(fd_append_only);
+	if (fd_append_only != -1) {
+		setup_inode_flag(fd_append_only, FS_APPEND_FL, 1);
+		SAFE_CLOSE(fd_append_only);
+	}
 }
 
 static void verify_unlink(unsigned int i)
 {
 	struct test_case_t *tc = &tcases[i];
-	int attr;
 
-	TST_EXP_FAIL(unlink(tc->filename), tc->expected_errno, "%s", tc->desc);
+	TST_EXP_FAIL(unlink(tc->filename), EPERM, "%s", tc->desc);
 
 	/* If unlink() succeeded unexpectedly, test file should be restored. */
 	if (!TST_RET) {
-		if (tc->fd) {
-			*(tc->fd) = SAFE_OPEN(tc->filename, O_CREAT, 0600);
-			if (tc->flag) {
-				SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS, &attr);
-				attr |= tc->flag;
-				SAFE_IOCTL(*(tc->fd), FS_IOC_SETFLAGS, &attr);
-			}
-		} else {
-			SAFE_TOUCH(tc->filename, 0600, 0);
-		}
+		*(tc->fd) = SAFE_CREAT(tc->filename, 0600);
+		setup_inode_flag(*(tc->fd), tc->flag, 0);
 	}
 }
 
@@ -96,7 +101,8 @@  static struct tst_test test = {
 	.tcnt = ARRAY_SIZE(tcases),
 	.cleanup = cleanup,
 	.test = verify_unlink,
-	.needs_rofs = 1,
-	.mntpoint = DIR_EROFS,
+	.mntpoint = MNTPOINT,
 	.needs_root = 1,
+	.all_filesystems = 1,
+	.format_device = 1,
 };