[LTP] [PATCH 0/3] Add ioctl_ficlone testing suite

[LTP] [PATCH 0/3] Add ioctl_ficlone testing suite

[Andrea Cervesato]

This testing suite is testing ioctl() FICLONE functionalities.
In particular, file clone and error codes.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Andrea Cervesato (3):
      Add ioctl_ficlone01 test
      Add ioctl_ficlone02 test
      Add ioctl_ficlone03 test

 include/lapi/fs.h                                 |   4 +
 runtest/syscalls                                  |   4 +
 testcases/kernel/syscalls/ioctl/.gitignore        |   3 +
 testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c | 116 ++++++++++++++++++++++
 testcases/kernel/syscalls/ioctl/ioctl_ficlone02.c |  49 +++++++++
 testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c |  96 ++++++++++++++++++
 6 files changed, 272 insertions(+)
---
base-commit: 66517b89141fc455ed807f3b95e5260dcf9fb90f
change-id: 20240529-ioctl_ficlone-95c603f71d53

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


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

[LTP] [PATCH 1/3] Add ioctl_ficlone01 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies that ioctl() FICLONE feature clones file content
from one file to an another.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 include/lapi/fs.h                                 |   4 +
 runtest/syscalls                                  |   2 +
 testcases/kernel/syscalls/ioctl/.gitignore        |   1 +
 testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c | 116 ++++++++++++++++++++++
 4 files changed, 123 insertions(+)

diff --git a/include/lapi/fs.h b/include/lapi/fs.h
index 635979b02..cc3b7636c 100644
--- a/include/lapi/fs.h
+++ b/include/lapi/fs.h
@@ -48,6 +48,10 @@
 # define FS_VERITY_FL	   0x00100000 /* Verity protected inode */
 #endif
 
+#ifndef FICLONE
+# define FICLONE		_IOW(0x94, 9, int)
+#endif
+
 /*
  * Helper function to get MAX_LFS_FILESIZE.
  * Missing PAGE_SHIFT on some libc prevents defining MAX_LFS_FILESIZE.
diff --git a/runtest/syscalls b/runtest/syscalls
index cf06ee563..07e940f8c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -591,6 +591,8 @@ ioctl_ns07 ioctl_ns07
 
 ioctl_sg01 ioctl_sg01
 
+ioctl_ficlone01 ioctl_ficlone01
+
 inotify_init1_01 inotify_init1_01
 inotify_init1_02 inotify_init1_02
 
diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
index 5fff7a61d..5404aa93f 100644
--- a/testcases/kernel/syscalls/ioctl/.gitignore
+++ b/testcases/kernel/syscalls/ioctl/.gitignore
@@ -22,3 +22,4 @@
 /ioctl_ns06
 /ioctl_ns07
 /ioctl_sg01
+/ioctl_ficlone01
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
new file mode 100644
index 000000000..29c1eb848
--- /dev/null
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that ioctl() FICLONE feature clones file content from
+ * one file to an another.
+ *
+ * [Algorithm]
+ *
+ * * populate source file
+ * * clone source content inside destination file
+ * * verify that source content has been cloned inside destination file
+ * * write a single byte inside destination file
+ * * verify that source content didn't change while destination did
+ */
+
+#include "tst_test.h"
+#include "lapi/fs.h"
+
+#define MNTPOINT "mnt"
+#define SRCPATH MNTPOINT "/file0"
+#define DSTPATH MNTPOINT "/file1"
+
+#define FILEDATA "qwerty"
+#define FILESIZE sizeof(FILEDATA)
+
+static int src_fd = -1;
+static int dst_fd = -1;
+
+static void run(void)
+{
+	char buff[FILESIZE];
+	struct stat src_stat;
+	struct stat dst_stat;
+
+	src_fd = SAFE_OPEN(SRCPATH, O_CREAT | O_RDWR, 0640);
+	dst_fd = SAFE_OPEN(DSTPATH, O_CREAT | O_RDWR, 0640);
+
+	tst_res(TINFO, "Writing data inside src file");
+
+	SAFE_WRITE(1, src_fd, FILEDATA, FILESIZE);
+	SAFE_FSYNC(src_fd);
+
+	TST_EXP_PASS(ioctl(dst_fd, FICLONE, src_fd));
+	if (TST_RET == -1)
+		return;
+
+	SAFE_FSYNC(dst_fd);
+
+	tst_res(TINFO, "Verifing that data is cloned between files");
+
+	SAFE_FSTAT(src_fd, &src_stat);
+	SAFE_FSTAT(dst_fd, &dst_stat);
+
+	TST_EXP_EXPR(src_stat.st_ino != dst_stat.st_ino,
+		"inode is different. %lu != %lu",
+		src_stat.st_ino,
+		dst_stat.st_ino);
+
+	TST_EXP_EQ_LI(src_stat.st_size, dst_stat.st_size);
+
+	SAFE_READ(0, dst_fd, buff, FILESIZE);
+
+	TST_EXP_EXPR(!strncmp(buff, FILEDATA, FILESIZE),
+		"dst file has the src file content (\"%s\" - %ld bytes)",
+		buff,
+		FILESIZE);
+
+	tst_res(TINFO, "Writing a byte inside dst file");
+
+	SAFE_WRITE(SAFE_WRITE_ALL, dst_fd, "a", 1);
+	SAFE_FSYNC(dst_fd);
+
+	tst_res(TINFO, "Verifing that src file content didn't change");
+
+	SAFE_FSTAT(src_fd, &src_stat);
+	SAFE_FSTAT(dst_fd, &dst_stat);
+
+	TST_EXP_EQ_LI(dst_stat.st_size, src_stat.st_size + 1);
+
+	SAFE_READ(0, src_fd, buff, FILESIZE);
+
+	TST_EXP_EXPR(!strncmp(buff, FILEDATA, FILESIZE),
+		"src file content didn't change");
+
+	SAFE_CLOSE(src_fd);
+	SAFE_CLOSE(dst_fd);
+	src_fd = -1;
+	dst_fd = -1;
+
+	remove(SRCPATH);
+	remove(DSTPATH);
+}
+
+static void cleanup(void)
+{
+	if (src_fd != -1)
+		SAFE_CLOSE(src_fd);
+
+	if (dst_fd != -1)
+		SAFE_CLOSE(dst_fd);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.cleanup = cleanup,
+	.min_kver = "4.5",
+	.needs_root = 1,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.dev_fs_type = "btrfs",
+};

-- 
2.35.3


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

[LTP] [PATCH 2/3] Add ioctl_ficlone02 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies that ioctl() FICLONE feature correctly raises
EOPNOTSUPP when unsupported filesystem is used. In particular,
filesystems which don't support copy-on-write.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                  |  1 +
 testcases/kernel/syscalls/ioctl/.gitignore        |  1 +
 testcases/kernel/syscalls/ioctl/ioctl_ficlone02.c | 49 +++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 07e940f8c..b049530d3 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -592,6 +592,7 @@ ioctl_ns07 ioctl_ns07
 ioctl_sg01 ioctl_sg01
 
 ioctl_ficlone01 ioctl_ficlone01
+ioctl_ficlone02 ioctl_ficlone02
 
 inotify_init1_01 inotify_init1_01
 inotify_init1_02 inotify_init1_02
diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
index 5404aa93f..3d25fdfb2 100644
--- a/testcases/kernel/syscalls/ioctl/.gitignore
+++ b/testcases/kernel/syscalls/ioctl/.gitignore
@@ -23,3 +23,4 @@
 /ioctl_ns07
 /ioctl_sg01
 /ioctl_ficlone01
+/ioctl_ficlone02
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone02.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone02.c
new file mode 100644
index 000000000..950b1109b
--- /dev/null
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone02.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that ioctl() FICLONE feature correctly raises EOPNOTSUPP
+ * when an unsupported filesystem is used. In particular, filesystems which
+ * don't support copy-on-write.
+ */
+
+#include "tst_test.h"
+#include "lapi/fs.h"
+
+#define MNTPOINT "mnt"
+#define SRCPATH MNTPOINT "/file0"
+#define DSTPATH MNTPOINT "/file1"
+
+static void run(void)
+{
+	int src_fd;
+	int dst_fd;
+
+	src_fd = SAFE_OPEN(SRCPATH, O_CREAT | O_RDWR, 0640);
+	dst_fd = SAFE_OPEN(DSTPATH, O_CREAT | O_RDWR, 0640);
+
+	TST_EXP_FAIL(ioctl(dst_fd, FICLONE, src_fd), EOPNOTSUPP);
+
+	SAFE_CLOSE(src_fd);
+	SAFE_CLOSE(dst_fd);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.min_kver = "4.5",
+	.needs_root = 1,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *[]) {
+		"btrfs",
+		"overlayfs",
+		"nfs",
+		"xfs",
+		NULL,
+	},
+};

-- 
2.35.3


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

[LTP] [PATCH 3/3] Add ioctl_ficlone03 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies that ioctl() FICLONE feature correctly raises
exceptions when it's supposed to.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                  |  1 +
 testcases/kernel/syscalls/ioctl/.gitignore        |  1 +
 testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c | 96 +++++++++++++++++++++++
 3 files changed, 98 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index b049530d3..452a7a071 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -593,6 +593,7 @@ ioctl_sg01 ioctl_sg01
 
 ioctl_ficlone01 ioctl_ficlone01
 ioctl_ficlone02 ioctl_ficlone02
+ioctl_ficlone03 ioctl_ficlone03
 
 inotify_init1_01 inotify_init1_01
 inotify_init1_02 inotify_init1_02
diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
index 3d25fdfb2..d0b470714 100644
--- a/testcases/kernel/syscalls/ioctl/.gitignore
+++ b/testcases/kernel/syscalls/ioctl/.gitignore
@@ -24,3 +24,4 @@
 /ioctl_sg01
 /ioctl_ficlone01
 /ioctl_ficlone02
+/ioctl_ficlone03
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
new file mode 100644
index 000000000..c6f9806a3
--- /dev/null
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that ioctl() FICLONE feature correctly raises exceptions
+ * when it's supposed to.
+ */
+
+#include "tst_test.h"
+#include "lapi/fs.h"
+
+#define MNTPOINT "mnt"
+
+static int invalid_fd = -1;
+static int rw_file = -1;
+static int ro_file = -1;
+static int wo_file = -1;
+static int dir_fd = -1;
+static int immut_fd = -1;
+static int mnt_file = -1;
+
+static struct tcase {
+	int *src_fd;
+	int *dst_fd;
+	int errno_exp;
+	char *msg;
+} tcases[] = {
+	{&invalid_fd, &rw_file, EBADF, "invalid source"},
+	{&rw_file, &invalid_fd, EBADF, "invalid destination"},
+	{&rw_file, &ro_file, EBADF, "read-only destination"},
+	{&wo_file, &rw_file, EBADF, "write-only source"},
+	{&rw_file, &dir_fd, EISDIR, "source is a directory"},
+	{&dir_fd, &rw_file, EISDIR, "destination is a directory"},
+	{&rw_file, &immut_fd, EPERM, "destination is immutable"},
+	{&rw_file, &mnt_file, EXDEV, "destination is on a different mount"},
+	{&mnt_file, &rw_file, EXDEV, "source is on a different mount"},
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	TST_EXP_FAIL(ioctl(*tc->dst_fd, FICLONE, *tc->src_fd),
+		tc->errno_exp,
+		"%s", tc->msg);
+}
+
+static void setup(void)
+{
+	int attr;
+
+	rw_file = SAFE_OPEN("ok_only", O_CREAT | O_RDWR, 0640);
+	ro_file = SAFE_OPEN("rd_only", O_CREAT | O_RDONLY, 0640);
+	wo_file = SAFE_OPEN("rw_only", O_CREAT | O_WRONLY, 0640);
+
+	SAFE_MKDIR("mydir", 0640);
+	dir_fd = SAFE_OPEN("mydir", O_DIRECTORY, 0640);
+
+	attr = FS_IMMUTABLE_FL;
+	immut_fd = SAFE_OPEN("immutable", O_CREAT | O_RDWR, 0640);
+	SAFE_IOCTL(immut_fd, FS_IOC_SETFLAGS, &attr);
+
+	mnt_file = SAFE_OPEN(MNTPOINT"/file", O_CREAT | O_RDWR, 0640);
+}
+
+static void cleanup(void)
+{
+	int attr;
+
+	SAFE_IOCTL(immut_fd, FS_IOC_GETFLAGS, &attr);
+	attr &= ~FS_IMMUTABLE_FL;
+	SAFE_IOCTL(immut_fd, FS_IOC_SETFLAGS, &attr);
+	SAFE_CLOSE(immut_fd);
+
+	SAFE_CLOSE(rw_file);
+	SAFE_CLOSE(ro_file);
+	SAFE_CLOSE(wo_file);
+	SAFE_CLOSE(dir_fd);
+	SAFE_CLOSE(mnt_file);
+}
+
+static struct tst_test test = {
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.min_kver = "4.5",
+	.needs_root = 1,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.dev_fs_type = "btrfs",
+};

-- 
2.35.3


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

Re: [LTP] [PATCH 1/3] Add ioctl_ficlone01 test

[Cyril Hrubis]

Hi!
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that ioctl() FICLONE feature clones file content from
> + * one file to an another.
> + *
> + * [Algorithm]
> + *
> + * * populate source file
> + * * clone source content inside destination file
> + * * verify that source content has been cloned inside destination file
> + * * write a single byte inside destination file
> + * * verify that source content didn't change while destination did

This is very minor but I find dashes to be a better choice for lists
inside of C comments.

> + */
> +
> +#include "tst_test.h"
> +#include "lapi/fs.h"
> +
> +#define MNTPOINT "mnt"
> +#define SRCPATH MNTPOINT "/file0"
> +#define DSTPATH MNTPOINT "/file1"
> +
> +#define FILEDATA "qwerty"
> +#define FILESIZE sizeof(FILEDATA)
> +
> +static int src_fd = -1;
> +static int dst_fd = -1;
> +
> +static void run(void)
> +{
> +	char buff[FILESIZE];
> +	struct stat src_stat;
> +	struct stat dst_stat;
> +
> +	src_fd = SAFE_OPEN(SRCPATH, O_CREAT | O_RDWR, 0640);
> +	dst_fd = SAFE_OPEN(DSTPATH, O_CREAT | O_RDWR, 0640);
> +
> +	tst_res(TINFO, "Writing data inside src file");
> +
> +	SAFE_WRITE(1, src_fd, FILEDATA, FILESIZE);
> +	SAFE_FSYNC(src_fd);
> +
> +	TST_EXP_PASS(ioctl(dst_fd, FICLONE, src_fd));
> +	if (TST_RET == -1)
> +		return;
> +
> +	SAFE_FSYNC(dst_fd);
> +
> +	tst_res(TINFO, "Verifing that data is cloned between files");
> +
> +	SAFE_FSTAT(src_fd, &src_stat);
> +	SAFE_FSTAT(dst_fd, &dst_stat);
> +
> +	TST_EXP_EXPR(src_stat.st_ino != dst_stat.st_ino,
> +		"inode is different. %lu != %lu",
> +		src_stat.st_ino,
> +		dst_stat.st_ino);
> +
> +	TST_EXP_EQ_LI(src_stat.st_size, dst_stat.st_size);
> +
> +	SAFE_READ(0, dst_fd, buff, FILESIZE);
> +
> +	TST_EXP_EXPR(!strncmp(buff, FILEDATA, FILESIZE),
> +		"dst file has the src file content (\"%s\" - %ld bytes)",
> +		buff,
> +		FILESIZE);
> +
> +	tst_res(TINFO, "Writing a byte inside dst file");
> +
> +	SAFE_WRITE(SAFE_WRITE_ALL, dst_fd, "a", 1);
> +	SAFE_FSYNC(dst_fd);
> +
> +	tst_res(TINFO, "Verifing that src file content didn't change");
> +
> +	SAFE_FSTAT(src_fd, &src_stat);
> +	SAFE_FSTAT(dst_fd, &dst_stat);
> +
> +	TST_EXP_EQ_LI(dst_stat.st_size, src_stat.st_size + 1);
> +
> +	SAFE_READ(0, src_fd, buff, FILESIZE);
> +
> +	TST_EXP_EXPR(!strncmp(buff, FILEDATA, FILESIZE),
> +		"src file content didn't change");

So you append to the file but then only read the initial part? That does
not sound right. I guess that easiest solution is to seek to the start
of the file or od pwrite() so that we overwrite the first byte rather
than appending.

Or at least check the return value from the read.

> +	SAFE_CLOSE(src_fd);
> +	SAFE_CLOSE(dst_fd);
> +	src_fd = -1;
> +	dst_fd = -1;

This is not needed, the SAFE_CLOSE() macro sets the fd to -1.

> +	remove(SRCPATH);
> +	remove(DSTPATH);

Please use SAFE_UNLINK() instead.

> +}
> +
> +static void cleanup(void)
> +{
> +	if (src_fd != -1)
> +		SAFE_CLOSE(src_fd);
> +
> +	if (dst_fd != -1)
> +		SAFE_CLOSE(dst_fd);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.cleanup = cleanup,
> +	.min_kver = "4.5",
> +	.needs_root = 1,
> +	.mount_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.dev_fs_type = "btrfs",


I suppose that we need .use_filesystems or similar and convert the
dev_fs_type to an array so that we can run this test on xfs as well...

> +};
> 
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 2/3] Add ioctl_ficlone02 test

[Cyril Hrubis]

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

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 3/3] Add ioctl_ficlone03 test

[Cyril Hrubis]

Hi!
>  inotify_init1_01 inotify_init1_01
>  inotify_init1_02 inotify_init1_02
> diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
> index 3d25fdfb2..d0b470714 100644
> --- a/testcases/kernel/syscalls/ioctl/.gitignore
> +++ b/testcases/kernel/syscalls/ioctl/.gitignore
> @@ -24,3 +24,4 @@
>  /ioctl_sg01
>  /ioctl_ficlone01
>  /ioctl_ficlone02
> +/ioctl_ficlone03
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
> new file mode 100644
> index 000000000..c6f9806a3
> --- /dev/null
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that ioctl() FICLONE feature correctly raises exceptions
> + * when it's supposed to.
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/fs.h"
> +
> +#define MNTPOINT "mnt"
> +
> +static int invalid_fd = -1;
> +static int rw_file = -1;
> +static int ro_file = -1;
> +static int wo_file = -1;
> +static int dir_fd = -1;
> +static int immut_fd = -1;
> +static int mnt_file = -1;
> +
> +static struct tcase {
> +	int *src_fd;
> +	int *dst_fd;
> +	int errno_exp;
> +	char *msg;
> +} tcases[] = {
> +	{&invalid_fd, &rw_file, EBADF, "invalid source"},
> +	{&rw_file, &invalid_fd, EBADF, "invalid destination"},

Can we move these invalid_fd tests into a separate test and use tst_fd
to loop over all kinds of invalid input and output file descriptors?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 1/3] Add ioctl_ficlone01 test

[Andrea Cervesato via ltp]

Hi!

On 5/30/24 12:48, Cyril Hrubis wrote:
> Hi!
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * This test verifies that ioctl() FICLONE feature clones file content from
>> + * one file to an another.
>> + *
>> + * [Algorithm]
>> + *
>> + * * populate source file
>> + * * clone source content inside destination file
>> + * * verify that source content has been cloned inside destination file
>> + * * write a single byte inside destination file
>> + * * verify that source content didn't change while destination did
> This is very minor but I find dashes to be a better choice for lists
> inside of C comments.
>
>> + */
>> +
>> +#include "tst_test.h"
>> +#include "lapi/fs.h"
>> +
>> +#define MNTPOINT "mnt"
>> +#define SRCPATH MNTPOINT "/file0"
>> +#define DSTPATH MNTPOINT "/file1"
>> +
>> +#define FILEDATA "qwerty"
>> +#define FILESIZE sizeof(FILEDATA)
>> +
>> +static int src_fd = -1;
>> +static int dst_fd = -1;
>> +
>> +static void run(void)
>> +{
>> +	char buff[FILESIZE];
>> +	struct stat src_stat;
>> +	struct stat dst_stat;
>> +
>> +	src_fd = SAFE_OPEN(SRCPATH, O_CREAT | O_RDWR, 0640);
>> +	dst_fd = SAFE_OPEN(DSTPATH, O_CREAT | O_RDWR, 0640);
>> +
>> +	tst_res(TINFO, "Writing data inside src file");
>> +
>> +	SAFE_WRITE(1, src_fd, FILEDATA, FILESIZE);
>> +	SAFE_FSYNC(src_fd);
>> +
>> +	TST_EXP_PASS(ioctl(dst_fd, FICLONE, src_fd));
>> +	if (TST_RET == -1)
>> +		return;
>> +
>> +	SAFE_FSYNC(dst_fd);
>> +
>> +	tst_res(TINFO, "Verifing that data is cloned between files");
>> +
>> +	SAFE_FSTAT(src_fd, &src_stat);
>> +	SAFE_FSTAT(dst_fd, &dst_stat);
>> +
>> +	TST_EXP_EXPR(src_stat.st_ino != dst_stat.st_ino,
>> +		"inode is different. %lu != %lu",
>> +		src_stat.st_ino,
>> +		dst_stat.st_ino);
>> +
>> +	TST_EXP_EQ_LI(src_stat.st_size, dst_stat.st_size);
>> +
>> +	SAFE_READ(0, dst_fd, buff, FILESIZE);
>> +
>> +	TST_EXP_EXPR(!strncmp(buff, FILEDATA, FILESIZE),
>> +		"dst file has the src file content (\"%s\" - %ld bytes)",
>> +		buff,
>> +		FILESIZE);
>> +
>> +	tst_res(TINFO, "Writing a byte inside dst file");
>> +
>> +	SAFE_WRITE(SAFE_WRITE_ALL, dst_fd, "a", 1);
>> +	SAFE_FSYNC(dst_fd);
>> +
>> +	tst_res(TINFO, "Verifing that src file content didn't change");
>> +
>> +	SAFE_FSTAT(src_fd, &src_stat);
>> +	SAFE_FSTAT(dst_fd, &dst_stat);
>> +
>> +	TST_EXP_EQ_LI(dst_stat.st_size, src_stat.st_size + 1);
>> +
>> +	SAFE_READ(0, src_fd, buff, FILESIZE);
>> +
>> +	TST_EXP_EXPR(!strncmp(buff, FILEDATA, FILESIZE),
>> +		"src file content didn't change");
> So you append to the file but then only read the initial part? That does
> not sound right. I guess that easiest solution is to seek to the start
> of the file or od pwrite() so that we overwrite the first byte rather
> than appending.
>
> Or at least check the return value from the read.
>
>> +	SAFE_CLOSE(src_fd);
>> +	SAFE_CLOSE(dst_fd);
>> +	src_fd = -1;
>> +	dst_fd = -1;
> This is not needed, the SAFE_CLOSE() macro sets the fd to -1.
>
>> +	remove(SRCPATH);
>> +	remove(DSTPATH);
> Please use SAFE_UNLINK() instead.
>
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (src_fd != -1)
>> +		SAFE_CLOSE(src_fd);
>> +
>> +	if (dst_fd != -1)
>> +		SAFE_CLOSE(dst_fd);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +	.cleanup = cleanup,
>> +	.min_kver = "4.5",
>> +	.needs_root = 1,
>> +	.mount_device = 1,
>> +	.mntpoint = MNTPOINT,
>> +	.dev_fs_type = "btrfs",
>
> I suppose that we need .use_filesystems or similar and convert the
> dev_fs_type to an array so that we can run this test on xfs as well...
This might be tricky to implement, since we need to adapt .dev_fs_ops as 
well..
>> +};
>>
>> -- 
>> 2.35.3
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp

Andrea


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

Re: [LTP] [PATCH 3/3] Add ioctl_ficlone03 test

[Andrea Cervesato via ltp]

Hi,

On 5/30/24 12:52, Cyril Hrubis wrote:
> Hi!
>>   inotify_init1_01 inotify_init1_01
>>   inotify_init1_02 inotify_init1_02
>> diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
>> index 3d25fdfb2..d0b470714 100644
>> --- a/testcases/kernel/syscalls/ioctl/.gitignore
>> +++ b/testcases/kernel/syscalls/ioctl/.gitignore
>> @@ -24,3 +24,4 @@
>>   /ioctl_sg01
>>   /ioctl_ficlone01
>>   /ioctl_ficlone02
>> +/ioctl_ficlone03
>> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
>> new file mode 100644
>> index 000000000..c6f9806a3
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
>> @@ -0,0 +1,96 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * This test verifies that ioctl() FICLONE feature correctly raises exceptions
>> + * when it's supposed to.
>> + */
>> +
>> +#include "tst_test.h"
>> +#include "lapi/fs.h"
>> +
>> +#define MNTPOINT "mnt"
>> +
>> +static int invalid_fd = -1;
>> +static int rw_file = -1;
>> +static int ro_file = -1;
>> +static int wo_file = -1;
>> +static int dir_fd = -1;
>> +static int immut_fd = -1;
>> +static int mnt_file = -1;
>> +
>> +static struct tcase {
>> +	int *src_fd;
>> +	int *dst_fd;
>> +	int errno_exp;
>> +	char *msg;
>> +} tcases[] = {
>> +	{&invalid_fd, &rw_file, EBADF, "invalid source"},
>> +	{&rw_file, &invalid_fd, EBADF, "invalid destination"},
> Can we move these invalid_fd tests into a separate test and use tst_fd
> to loop over all kinds of invalid input and output file descriptors?
>
I don't know if it's worth to loop over all possible cases. Each one of 
them has a specific test case.
Also, invalid_fd test cases would look so simple that it makes more 
sense to keep it here.

Andrea


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

Re: [LTP] [PATCH 1/3] Add ioctl_ficlone01 test

[Cyril Hrubis]

Hi!
> > I suppose that we need .use_filesystems or similar and convert the
> > dev_fs_type to an array so that we can run this test on xfs as well...
> This might be tricky to implement, since we need to adapt .dev_fs_ops as 
> well..

I guess that we need to do a bigger surgery and put all the device
related flags into a structure, e.g.

struct tst_fs {
	const char *dev_fs_type;

	const char *const *mkfs_opts;
        const char *const *mkfs_extra_opts;

	unsigned int mnt_flags;
	const char *mnt_data;
};

struct tst_test {
	...
	struct tst_fs filesystems[];
	...
};

That way we can specify all the options per a filesystems. I can try to
do prepare the patchset next week.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 3/3] Add ioctl_ficlone03 test

[Cyril Hrubis]

Hi!
> > Can we move these invalid_fd tests into a separate test and use tst_fd
> > to loop over all kinds of invalid input and output file descriptors?
> >
> I don't know if it's worth to loop over all possible cases. Each one of 
> them has a specific test case.

This was actually requested by kernel developers, to feed the syscalls
with all kind of unexpected file descriptors.

> Also, invalid_fd test cases would look so simple that it makes more 
> sense to keep it here.

Fair enough, we can keep these two here, but we need a tst_fd test as
well.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 1/3] Add ioctl_ficlone01 test

[Andrea Cervesato via ltp]

Ok thank you

On 5/31/24 10:01, Cyril Hrubis wrote:
> Hi!
>>> I suppose that we need .use_filesystems or similar and convert the
>>> dev_fs_type to an array so that we can run this test on xfs as well...
>> This might be tricky to implement, since we need to adapt .dev_fs_ops as
>> well..
> I guess that we need to do a bigger surgery and put all the device
> related flags into a structure, e.g.
>
> struct tst_fs {
> 	const char *dev_fs_type;
>
> 	const char *const *mkfs_opts;
>          const char *const *mkfs_extra_opts;
>
> 	unsigned int mnt_flags;
> 	const char *mnt_data;
> };
>
> struct tst_test {
> 	...
> 	struct tst_fs filesystems[];
> 	...
> };
>
> That way we can specify all the options per a filesystems. I can try to
> do prepare the patchset next week.
>


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

Re: [LTP] [PATCH 2/3] Add ioctl_ficlone02 test

[Andrea Cervesato via ltp]

Hi!

Thanks for review. I will send a v2 just to add FICLONERANGE as well, 
otherwise
we need an another identical test for that feature as well.

I will do the same for ioctl_ficlone03 as well.

Andrea

On 5/30/24 12:49, Cyril Hrubis wrote:
> Hi!
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
>


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

Re: [LTP] [PATCH 3/3] Add ioctl_ficlone03 test

[Andrea Cervesato via ltp]

Hi!

On 5/31/24 10:03, Cyril Hrubis wrote:
> Hi!
>>> Can we move these invalid_fd tests into a separate test and use tst_fd
>>> to loop over all kinds of invalid input and output file descriptors?
>>>
>> I don't know if it's worth to loop over all possible cases. Each one of
>> them has a specific test case.
> This was actually requested by kernel developers, to feed the syscalls
> with all kind of unexpected file descriptors.
This is what fuzz testing is doing already, isn't it?
>> Also, invalid_fd test cases would look so simple that it makes more
>> sense to keep it here.
> Fair enough, we can keep these two here, but we need a tst_fd test as
> well.
>
Andrea


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

Re: [LTP] [PATCH 3/3] Add ioctl_ficlone03 test

[Andrea Cervesato via ltp]

On 5/31/24 10:03, Cyril Hrubis wrote:
> Hi!
>>> Can we move these invalid_fd tests into a separate test and use tst_fd
>>> to loop over all kinds of invalid input and output file descriptors?
>>>
>> I don't know if it's worth to loop over all possible cases. Each one of
>> them has a specific test case.
> This was actually requested by kernel developers, to feed the syscalls
> with all kind of unexpected file descriptors.
>
>> Also, invalid_fd test cases would look so simple that it makes more
>> sense to keep it here.
> Fair enough, we can keep these two here, but we need a tst_fd test as
> well.
>
I'm trying to create a test using tst_fs but it seems quite 
unpredictable. For example, if the given paths are located on a 
different mount, EXDEV is raised.
But we don't have any control on it and it can happen with literally 
anything (signalfd, memfd, devices, etc) on the SUT we are testing.

So that ends up skipping 90% of the provided file descriptors. I don't 
know if I really want to follow this direction.

Andrea


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