All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v5 0/8] improvement work on libswap library
@ 2024-01-28  2:48 Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 1/8] libswap: add known swap supported fs check Li Wang
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

v4-->v5:
	* port swapon/off0[23].c to all_filesystems.
	* replace the tst_brk with tst_res+return in is_support_swap
	* customisze swapfile size if needed

Li Wang (7):
  libswap: add known swap supported fs check
  swapon01: Improving test with memory limits and swap reporting
  libswap: add function to prealloc contiguous file
  libswap: Introduce file contiguity check
  libswap: customize swapfile size
  swapon/off: enable all_filesystem in swap test
  libswap: Refactor is_swap_supported function to return status

Petr Vorel (1):
  swapon01: Test on all filesystems

 include/libswap.h                             |   4 +-
 libs/libltpswap/libswap.c                     | 180 ++++++++++++++++--
 testcases/kernel/syscalls/swapoff/swapoff01.c |   5 +-
 testcases/kernel/syscalls/swapoff/swapoff02.c |  11 +-
 testcases/kernel/syscalls/swapon/swapon01.c   |  18 +-
 testcases/kernel/syscalls/swapon/swapon02.c   |   8 +-
 testcases/kernel/syscalls/swapon/swapon03.c   |   8 +-
 7 files changed, 201 insertions(+), 33 deletions(-)

-- 
2.40.1


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 1/8] libswap: add known swap supported fs check
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 2/8] swapon01: Test on all filesystems Li Wang
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

This introduce an enhancement to the library's is_swap_supported
function to check for filesystem compatibility before attempting
to create and enable a swap file.  A list of supported filesystems
is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
against this list is performed to ensure that the swap operations
are only attempted on known compatible filesystems.

If the make_swapfile function fails, the error handling is now
more descriptive: it distinguishes between failures due to the
filesystem not supporting swap files and other types of failures.
Similarly, when attempting to enable the swap file with swapon,
the patch ensures that clearer error messages are provided in
cases where the operation is not supported by the filesystem.

Signed-off-by: Li Wang <liwang@redhat.com>
Tested-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 libs/libltpswap/libswap.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 13610709e..8c2ce6cd7 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -12,6 +12,17 @@
 #include "libswap.h"
 #include "lapi/syscalls.h"
 
+static const char *const swap_supported_fs[] = {
+	"ext2",
+	"ext3",
+	"ext4",
+	"xfs",
+	"vfat",
+	"exfat",
+	"ntfs",
+	NULL
+};
+
 /*
  * Make a swap file
  */
@@ -26,6 +37,7 @@ int make_swapfile(const char *swapfile, int safe)
 
 	/* make the file swapfile */
 	const char *argv[2 + 1];
+
 	argv[0] = "mkswap";
 	argv[1] = swapfile;
 	argv[2] = NULL;
@@ -40,13 +52,22 @@ int make_swapfile(const char *swapfile, int safe)
  */
 void is_swap_supported(const char *filename)
 {
+	int i, sw_support = 0;
 	int fibmap = tst_fibmap(filename);
 	long fs_type = tst_fs_type(filename);
 	const char *fstype = tst_fs_type_name(fs_type);
 
+	for (i = 0; swap_supported_fs[i]; i++) {
+		if (strstr(fstype, swap_supported_fs[i])) {
+			sw_support = 1;
+			break;
+		}
+	}
+
 	int ret = make_swapfile(filename, 1);
+
 	if (ret != 0) {
-		if (fibmap == 1)
+		if (fibmap == 1 && sw_support == 0)
 			tst_brk(TCONF, "mkswap on %s not supported", fstype);
 		else
 			tst_brk(TFAIL, "mkswap on %s failed", fstype);
@@ -56,7 +77,7 @@ void is_swap_supported(const char *filename)
 	if (TST_RET == -1) {
 		if (errno == EPERM)
 			tst_brk(TCONF, "Permission denied for swapon()");
-		else if (fibmap == 1 && errno == EINVAL)
+		else if (fibmap == 1 && errno == EINVAL && sw_support == 0)
 			tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
 		else
 			tst_brk(TFAIL | TTERRNO, "swapon() on %s failed", fstype);
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 2/8] swapon01: Test on all filesystems
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 1/8] libswap: add known swap supported fs check Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 3/8] swapon01: Improving test with memory limits and swap reporting Li Wang
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

From: Petr Vorel <pvorel@suse.cz>

Test on all filesystems to increase coverage.
Skip filesystems which does not support swap (currently bcachefs, btrfs
and tmpfs).

Tested on 5.10, 6.6 and 6.7.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Li Wang <liwang@redhat.com>
Tested-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/swapon/swapon01.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
index e59fb20a1..e1fe50459 100644
--- a/testcases/kernel/syscalls/swapon/swapon01.c
+++ b/testcases/kernel/syscalls/swapon/swapon01.c
@@ -8,6 +8,7 @@
  * [Description]
  *
  * Checks that swapon() succeds with swapfile.
+ * Testing on all filesystems which support swap file.
  */
 
 #include <unistd.h>
@@ -17,7 +18,8 @@
 #include "lapi/syscalls.h"
 #include "libswap.h"
 
-#define SWAP_FILE "swapfile01"
+#define MNTPOINT	"mntpoint"
+#define SWAP_FILE	MNTPOINT"/swapfile01"
 
 static void verify_swapon(void)
 {
@@ -36,8 +38,10 @@ static void setup(void)
 }
 
 static struct tst_test test = {
+	.mntpoint = MNTPOINT,
+	.mount_device = 1,
 	.needs_root = 1,
-	.needs_tmpdir = 1,
+	.all_filesystems = 1,
 	.test_all = verify_swapon,
 	.setup = setup
 };
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 3/8] swapon01: Improving test with memory limits and swap reporting
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 1/8] libswap: add known swap supported fs check Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 2/8] swapon01: Test on all filesystems Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 4/8] libswap: add function to prealloc contiguous file Li Wang
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

This is target to create a more robust and controlled environment to test
the swapon system call. By introducing memory limits through cgroups and
filling memory with a known pattern, the test can better assess swapon
behavior when the system experiences memory pressure.

Additionally, the reporting of "SwapCached" memory before turning off the
swap file provides a clearer understanding of the swap system's state in
response to the test conditions.

Signed-off-by: Li Wang <liwang@redhat.com>
Tested-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/swapon/swapon01.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
index e1fe50459..a74a5171e 100644
--- a/testcases/kernel/syscalls/swapon/swapon01.c
+++ b/testcases/kernel/syscalls/swapon/swapon01.c
@@ -20,11 +20,15 @@
 
 #define MNTPOINT	"mntpoint"
 #define SWAP_FILE	MNTPOINT"/swapfile01"
+#define TESTMEM		(1UL<<30)
 
 static void verify_swapon(void)
 {
 	TST_EXP_PASS(tst_syscall(__NR_swapon, SWAP_FILE, 0));
 
+	tst_pollute_memory(TESTMEM, 0x41);
+	tst_res(TINFO, "SwapCached: %ld Kb", SAFE_READ_MEMINFO("SwapCached:"));
+
 	if (TST_PASS && tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
 		tst_brk(TBROK | TERRNO,
 				"Failed to turn off swapfile, system reboot recommended");
@@ -35,6 +39,9 @@ static void setup(void)
 {
 	is_swap_supported(SWAP_FILE);
 	make_swapfile(SWAP_FILE, 0);
+
+	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
+	SAFE_CG_PRINTF(tst_cg, "memory.max", "%lu", TESTMEM);
 }
 
 static struct tst_test test = {
@@ -42,6 +49,7 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.needs_root = 1,
 	.all_filesystems = 1,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
 	.test_all = verify_swapon,
 	.setup = setup
 };
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 4/8] libswap: add function to prealloc contiguous file
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
                   ` (2 preceding siblings ...)
  2024-01-28  2:48 ` [LTP] [PATCH v5 3/8] swapon01: Improving test with memory limits and swap reporting Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 5/8] libswap: Introduce file contiguity check Li Wang
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

The improve makes several key updates to the swap file handling
in the libswap.c file:

It incorporates support for the Btrfs filesystem, which is now
recognized as a valid filesystem for swap files.

A new function, set_nocow_attr, is added to apply the FS_NOCOW_FL
flag to files on Btrfs filesystems.

Introduces a new prealloc_contiguous_file function. This method
preallocates a contiguous block of space for the swap file during
its creation, rather than filling the file with data as was done
previously.

Modifications to the make_swapfile function are made to utilize
prealloc_contiguous_file for creating the swap file, ensuring
the file is created with contiguous space on the filesystem.

Signed-off-by: Li Wang <liwang@redhat.com>
Tested-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 libs/libltpswap/libswap.c | 54 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 8c2ce6cd7..e606e3f03 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -4,6 +4,7 @@
  * Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
  */
 
+#include <linux/fs.h>
 #include <errno.h>
 
 #define TST_NO_DEFAULT_MAIN
@@ -13,6 +14,7 @@
 #include "lapi/syscalls.h"
 
 static const char *const swap_supported_fs[] = {
+	"btrfs",
 	"ext2",
 	"ext3",
 	"ext4",
@@ -23,6 +25,50 @@ static const char *const swap_supported_fs[] = {
 	NULL
 };
 
+static void set_nocow_attr(const char *filename)
+{
+	int fd;
+	int attrs;
+
+	tst_res(TINFO, "FS_NOCOW_FL attribute set on %s", filename);
+
+	fd = SAFE_OPEN(filename, O_RDONLY);
+
+	SAFE_IOCTL(fd, FS_IOC_GETFLAGS, &attrs);
+
+	attrs |= FS_NOCOW_FL;
+
+	SAFE_IOCTL(fd, FS_IOC_SETFLAGS, &attrs);
+
+	SAFE_CLOSE(fd);
+}
+
+static int prealloc_contiguous_file(const char *path, size_t bs, size_t bcount)
+{
+	int fd;
+
+	fd = open(path, O_CREAT|O_WRONLY|O_TRUNC, 0600);
+	if (fd < 0)
+		return -1;
+
+	/* Btrfs file need set 'nocow' attribute */
+	if (tst_fs_type(path) == TST_BTRFS_MAGIC)
+		set_nocow_attr(path);
+
+	if (tst_prealloc_size_fd(fd, bs, bcount)) {
+		close(fd);
+		unlink(path);
+		return -1;
+	}
+
+	if (close(fd) < 0) {
+		unlink(path);
+		return -1;
+	}
+
+	return 0;
+}
+
 /*
  * Make a swap file
  */
@@ -32,9 +78,15 @@ int make_swapfile(const char *swapfile, int safe)
 		tst_brk(TBROK, "Insufficient disk space to create swap file");
 
 	/* create file */
-	if (tst_fill_file(swapfile, 0, sysconf(_SC_PAGESIZE), 10) != 0)
+	if (prealloc_contiguous_file(swapfile, sysconf(_SC_PAGESIZE), 10) != 0)
 		tst_brk(TBROK, "Failed to create swapfile");
 
+	/* Full the file to make old xfs happy*/
+	if (tst_fs_type(swapfile) == TST_XFS_MAGIC) {
+		if (tst_fill_file(swapfile, 0, sysconf(_SC_PAGESIZE), 10) != 0)
+			tst_brk(TBROK, "Failed to create swapfile");
+	}
+
 	/* make the file swapfile */
 	const char *argv[2 + 1];
 
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 5/8] libswap: Introduce file contiguity check
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
                   ` (3 preceding siblings ...)
  2024-01-28  2:48 ` [LTP] [PATCH v5 4/8] libswap: add function to prealloc contiguous file Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 6/8] libswap: customize swapfile size Li Wang
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

This patch introduces a new function file_is_contiguous to the
libltpswap library to determine if a swap file is stored in a
contiguous block of disk space, which is a typical requirement
for swap files in Linux. The function performs a series of checks
using the fiemap structure to assess the contiguity of the file
and logs the result.

It is integrated into the is_swap_supported function to replace
the previous tst_fibmap check, providing a more reliable method
for verifying that a file suitable for swap is indeed contiguous.

Signed-off-by: Li Wang <liwang@redhat.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>
---
 libs/libltpswap/libswap.c | 70 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index e606e3f03..95d4f59b0 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -6,6 +6,8 @@
 
 #include <linux/fs.h>
 #include <errno.h>
+#include <linux/fiemap.h>
+#include <stdlib.h>
 
 #define TST_NO_DEFAULT_MAIN
 
@@ -69,6 +71,61 @@ static int prealloc_contiguous_file(const char *path, size_t bs, size_t bcount)
 	return 0;
 }
 
+static int file_is_contiguous(const char *filename)
+{
+	int fd, contiguous = 0;
+	struct fiemap *fiemap;
+
+	if (tst_fibmap(filename) == 0) {
+		contiguous = 1;
+		goto out;
+	}
+
+	if (tst_fs_type(filename) == TST_TMPFS_MAGIC)
+		goto out;
+
+	fd = SAFE_OPEN(filename, O_RDONLY);
+
+	fiemap = (struct fiemap *)SAFE_MALLOC(sizeof(struct fiemap) + sizeof(struct fiemap_extent));
+	memset(fiemap, 0, sizeof(struct fiemap) + sizeof(struct fiemap_extent));
+
+	fiemap->fm_start = 0;
+	fiemap->fm_length = ~0;
+	fiemap->fm_flags = 0;
+	fiemap->fm_extent_count = 1;
+
+	SAFE_IOCTL(fd, FS_IOC_FIEMAP, fiemap);
+
+	/*
+	 * fiemap->fm_mapped_extents != 1:
+	 *   This checks if the file does not have exactly one extent. If there are more
+	 *   or zero extents, the file is not stored in a single contiguous block.
+	 *
+	 * fiemap->fm_extents[0].fe_logical != 0:
+	 *   This checks if the first extent does not start at the logical offset 0 of
+	 *   the file. If it doesn't, it indicates that the file's first block of data
+	 *   is not at the beginning of the file, which implies non-contiguity.
+	 *
+	 * (fiemap->fm_extents[0].fe_flags & FIEMAP_EXTENT_LAST) != FIEMAP_EXTENT_LAST:
+	 *   This checks if the first extent does not have the FIEMAP_EXTENT_LAST flag set.
+	 *   If the flag isn't set, it means that this extent is not the last one, suggesting
+	 *   that there are more extents and the file is not contiguous.
+	 */
+	if (fiemap->fm_mapped_extents != 1 ||
+		fiemap->fm_extents[0].fe_logical != 0 ||
+		(fiemap->fm_extents[0].fe_flags & FIEMAP_EXTENT_LAST) != FIEMAP_EXTENT_LAST) {
+
+		tst_res(TINFO, "File '%s' is not contiguous", filename);
+		contiguous = 0;
+	}
+
+	SAFE_CLOSE(fd);
+	free(fiemap);
+
+out:
+	return contiguous;
+}
+
 /*
  * Make a swap file
  */
@@ -105,10 +162,15 @@ int make_swapfile(const char *swapfile, int safe)
 void is_swap_supported(const char *filename)
 {
 	int i, sw_support = 0;
-	int fibmap = tst_fibmap(filename);
+	int ret = make_swapfile(filename, 1);
+	int fi_contiguous = file_is_contiguous(filename);
 	long fs_type = tst_fs_type(filename);
 	const char *fstype = tst_fs_type_name(fs_type);
 
+	if (fs_type == TST_BTRFS_MAGIC &&
+			tst_kvercmp(5, 0, 0) < 0)
+		tst_brk(TCONF, "Swapfile on Btrfs (kernel < 5.0) not implemented");
+
 	for (i = 0; swap_supported_fs[i]; i++) {
 		if (strstr(fstype, swap_supported_fs[i])) {
 			sw_support = 1;
@@ -116,10 +178,8 @@ void is_swap_supported(const char *filename)
 		}
 	}
 
-	int ret = make_swapfile(filename, 1);
-
 	if (ret != 0) {
-		if (fibmap == 1 && sw_support == 0)
+		if (fi_contiguous == 0 && sw_support == 0)
 			tst_brk(TCONF, "mkswap on %s not supported", fstype);
 		else
 			tst_brk(TFAIL, "mkswap on %s failed", fstype);
@@ -129,7 +189,7 @@ void is_swap_supported(const char *filename)
 	if (TST_RET == -1) {
 		if (errno == EPERM)
 			tst_brk(TCONF, "Permission denied for swapon()");
-		else if (fibmap == 1 && errno == EINVAL && sw_support == 0)
+		else if (errno == EINVAL && fi_contiguous == 0 && sw_support == 0)
 			tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
 		else
 			tst_brk(TFAIL | TTERRNO, "swapon() on %s failed", fstype);
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 6/8] libswap: customize swapfile size
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
                   ` (4 preceding siblings ...)
  2024-01-28  2:48 ` [LTP] [PATCH v5 5/8] libswap: Introduce file contiguity check Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 7/8] swapon/off: enable all_filesystem in swap test Li Wang
  2024-01-28  2:48 ` [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status Li Wang
  7 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

The key change is the modification of the make_swapfile function to
accept an additional parameter blocks, specifying the number of
blocks to allocate for the swap file. This change allows for more
granular control over the size of swap files created during tests.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 include/libswap.h                             |  2 +-
 libs/libltpswap/libswap.c                     | 23 +++++++++++++------
 testcases/kernel/syscalls/swapoff/swapoff01.c |  5 +---
 testcases/kernel/syscalls/swapon/swapon01.c   |  2 +-
 testcases/kernel/syscalls/swapon/swapon02.c   |  4 ++--
 testcases/kernel/syscalls/swapon/swapon03.c   |  4 ++--
 6 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/include/libswap.h b/include/libswap.h
index d4b5301a5..e67d65756 100644
--- a/include/libswap.h
+++ b/include/libswap.h
@@ -14,7 +14,7 @@
 /*
  * Make a swap file
  */
-int make_swapfile(const char *swapfile, int safe);
+int make_swapfile(const char *swapfile, int blocks, int safe);
 
 /*
  * Check swapon/swapoff support status of filesystems or files
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 95d4f59b0..8aecad48d 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -4,6 +4,7 @@
  * Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
  */
 
+#include <sys/statvfs.h>
 #include <linux/fs.h>
 #include <errno.h>
 #include <linux/fiemap.h>
@@ -129,19 +130,27 @@ out:
 /*
  * Make a swap file
  */
-int make_swapfile(const char *swapfile, int safe)
+int make_swapfile(const char *swapfile, int blocks, int safe)
 {
-	if (!tst_fs_has_free(".", sysconf(_SC_PAGESIZE) * 10, TST_BYTES))
+	struct statvfs fs_info;
+	unsigned long blk_size;
+
+	if (statvfs(".", &fs_info) == -1)
+		return -1;
+
+	blk_size = fs_info.f_bsize;
+
+	if (!tst_fs_has_free(".", blk_size * blocks, TST_BYTES))
 		tst_brk(TBROK, "Insufficient disk space to create swap file");
 
 	/* create file */
-	if (prealloc_contiguous_file(swapfile, sysconf(_SC_PAGESIZE), 10) != 0)
+	if (prealloc_contiguous_file(swapfile, blk_size, blocks) != 0)
 		tst_brk(TBROK, "Failed to create swapfile");
 
-	/* Full the file to make old xfs happy*/
+	/* Fill the file if needed (specific to old xfs filesystems) */
 	if (tst_fs_type(swapfile) == TST_XFS_MAGIC) {
-		if (tst_fill_file(swapfile, 0, sysconf(_SC_PAGESIZE), 10) != 0)
-			tst_brk(TBROK, "Failed to create swapfile");
+		if (tst_fill_file(swapfile, 0, blk_size, blocks) != 0)
+			tst_brk(TBROK, "Failed to fill swapfile");
 	}
 
 	/* make the file swapfile */
@@ -162,7 +171,7 @@ int make_swapfile(const char *swapfile, int safe)
 void is_swap_supported(const char *filename)
 {
 	int i, sw_support = 0;
-	int ret = make_swapfile(filename, 1);
+	int ret = make_swapfile(filename, 10, 1);
 	int fi_contiguous = file_is_contiguous(filename);
 	long fs_type = tst_fs_type(filename);
 	const char *fstype = tst_fs_type_name(fs_type);
diff --git a/testcases/kernel/syscalls/swapoff/swapoff01.c b/testcases/kernel/syscalls/swapoff/swapoff01.c
index b27eecdad..e3b445d05 100644
--- a/testcases/kernel/syscalls/swapoff/swapoff01.c
+++ b/testcases/kernel/syscalls/swapoff/swapoff01.c
@@ -44,11 +44,8 @@ static void setup(void)
 		tst_brk(TBROK,
 			"Insufficient disk space to create swap file");
 
-	if (tst_fill_file("swapfile01", 0x00, 1024, 65536))
+	if (make_swapfile("swapfile01", 65536, 1))
 		tst_brk(TBROK, "Failed to create file for swap");
-
-	if (system("mkswap swapfile01 > tmpfile 2>&1") != 0)
-		tst_brk(TBROK, "Failed to make swapfile");
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
index a74a5171e..d406e4bd9 100644
--- a/testcases/kernel/syscalls/swapon/swapon01.c
+++ b/testcases/kernel/syscalls/swapon/swapon01.c
@@ -38,7 +38,7 @@ static void verify_swapon(void)
 static void setup(void)
 {
 	is_swap_supported(SWAP_FILE);
-	make_swapfile(SWAP_FILE, 0);
+	make_swapfile(SWAP_FILE, 10, 0);
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 	SAFE_CG_PRINTF(tst_cg, "memory.max", "%lu", TESTMEM);
diff --git a/testcases/kernel/syscalls/swapon/swapon02.c b/testcases/kernel/syscalls/swapon/swapon02.c
index fceea77be..f5b0d6d56 100644
--- a/testcases/kernel/syscalls/swapon/swapon02.c
+++ b/testcases/kernel/syscalls/swapon/swapon02.c
@@ -44,8 +44,8 @@ static void setup(void)
 	is_swap_supported("./tstswap");
 
 	SAFE_TOUCH("notswap", 0777, NULL);
-	make_swapfile("swapfile01", 0);
-	make_swapfile("alreadyused", 0);
+	make_swapfile("swapfile01", 10, 0);
+	make_swapfile("alreadyused", 10, 0);
 
 	if (tst_syscall(__NR_swapon, "alreadyused", 0))
 		tst_res(TWARN | TERRNO, "swapon(alreadyused) failed");
diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index dc633ebc6..e13009111 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -155,7 +155,7 @@ static int setup_swap(void)
 			}
 
 			/* Create the swapfile */
-			make_swapfile(filename, 0);
+			make_swapfile(filename, 10, 0);
 
 			/* turn on the swap file */
 			res = tst_syscall(__NR_swapon, filename, 0);
@@ -178,7 +178,7 @@ static int setup_swap(void)
 
 	/* Create all needed extra swapfiles for testing */
 	for (j = 0; j < testfiles; j++)
-		make_swapfile(swap_testfiles[j].filename, 0);
+		make_swapfile(swap_testfiles[j].filename, 10, 0);
 
 	return 0;
 }
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 7/8] swapon/off: enable all_filesystem in swap test
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
                   ` (5 preceding siblings ...)
  2024-01-28  2:48 ` [LTP] [PATCH v5 6/8] libswap: customize swapfile size Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-29  8:03   ` Petr Vorel
  2024-01-28  2:48 ` [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status Li Wang
  7 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/syscalls/swapoff/swapoff02.c | 11 ++++++-----
 testcases/kernel/syscalls/swapon/swapon02.c   |  4 ++++
 testcases/kernel/syscalls/swapon/swapon03.c   |  4 ++++
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/swapoff/swapoff02.c b/testcases/kernel/syscalls/swapoff/swapoff02.c
index cd940b1e7..53060b320 100644
--- a/testcases/kernel/syscalls/swapoff/swapoff02.c
+++ b/testcases/kernel/syscalls/swapoff/swapoff02.c
@@ -18,6 +18,8 @@
 #include "lapi/syscalls.h"
 #include "libswap.h"
 
+#define MNTPOINT	"mntpoint"
+
 static int setup01(void);
 static void cleanup01(void);
 
@@ -84,14 +86,13 @@ static void setup(void)
 
 	is_swap_supported("./tstswap");
 
-	if (!tst_fs_has_free(".", 1, TST_KB))
-		tst_brk(TBROK, "Insufficient disk space to create swap file");
-
-	if (tst_fill_file("./swapfile01", 0x00, 1024, 1))
-		tst_brk(TBROK, "Failed to create swapfile");
+	if (make_swapfile("swapfile01", 10, 1))
+		tst_brk(TBROK, "Failed to create file for swap");
 }
 
 static struct tst_test test = {
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
 	.needs_root = 1,
 	.needs_tmpdir = 1,
 	.test = verify_swapoff,
diff --git a/testcases/kernel/syscalls/swapon/swapon02.c b/testcases/kernel/syscalls/swapon/swapon02.c
index f5b0d6d56..acb789fc4 100644
--- a/testcases/kernel/syscalls/swapon/swapon02.c
+++ b/testcases/kernel/syscalls/swapon/swapon02.c
@@ -20,6 +20,8 @@
 #include "lapi/syscalls.h"
 #include "libswap.h"
 
+#define MNTPOINT	"mntpoint"
+
 static uid_t nobody_uid;
 static int do_swapoff;
 
@@ -78,6 +80,8 @@ static void verify_swapon(unsigned int i)
 }
 
 static struct tst_test test = {
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
 	.needs_root = 1,
 	.needs_tmpdir = 1,
 	.test = verify_swapon,
diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index e13009111..249cb12c3 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -22,6 +22,8 @@
 #include "swaponoff.h"
 #include "libswap.h"
 
+#define MNTPOINT	"mntpoint"
+
 static int setup_swap(void);
 static int clean_swap(void);
 static int check_and_swapoff(const char *filename);
@@ -265,6 +267,8 @@ static void cleanup(void)
 }
 
 static struct tst_test test = {
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
 	.needs_root = 1,
 	.needs_tmpdir = 1,
 	.forks_child = 1,
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status
  2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
                   ` (6 preceding siblings ...)
  2024-01-28  2:48 ` [LTP] [PATCH v5 7/8] swapon/off: enable all_filesystem in swap test Li Wang
@ 2024-01-28  2:48 ` Li Wang
  2024-01-29  8:03   ` Petr Vorel
  7 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2024-01-28  2:48 UTC (permalink / raw)
  To: ltp

This updates the is_swap_supported function in the libltpswap
to return an integer status instead of void, allowing the function
to communicate success or failure to the caller. It introduces
checks and returns 0 on various failure conditions while logging
the failure without aborting the test case.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 include/libswap.h         |  2 +-
 libs/libltpswap/libswap.c | 28 ++++++++++++++++++----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/include/libswap.h b/include/libswap.h
index e67d65756..1e09db031 100644
--- a/include/libswap.h
+++ b/include/libswap.h
@@ -20,5 +20,5 @@ int make_swapfile(const char *swapfile, int blocks, int safe);
  * Check swapon/swapoff support status of filesystems or files
  * we are testing on.
  */
-void is_swap_supported(const char *filename);
+int is_swap_supported(const char *filename);
 #endif /* __LIBSWAP_H__ */
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 8aecad48d..14b7d9e3a 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -168,7 +168,7 @@ int make_swapfile(const char *swapfile, int blocks, int safe)
  * Check swapon/swapoff support status of filesystems or files
  * we are testing on.
  */
-void is_swap_supported(const char *filename)
+int is_swap_supported(const char *filename)
 {
 	int i, sw_support = 0;
 	int ret = make_swapfile(filename, 10, 1);
@@ -188,23 +188,31 @@ void is_swap_supported(const char *filename)
 	}
 
 	if (ret != 0) {
-		if (fi_contiguous == 0 && sw_support == 0)
+		if (fi_contiguous == 0 && sw_support == 0) {
 			tst_brk(TCONF, "mkswap on %s not supported", fstype);
-		else
-			tst_brk(TFAIL, "mkswap on %s failed", fstype);
+		} else {
+			tst_res(TFAIL, "mkswap on %s failed", fstype);
+			return 0;
+		}
 	}
 
 	TEST(tst_syscall(__NR_swapon, filename, 0));
 	if (TST_RET == -1) {
-		if (errno == EPERM)
+		if (errno == EPERM) {
 			tst_brk(TCONF, "Permission denied for swapon()");
-		else if (errno == EINVAL && fi_contiguous == 0 && sw_support == 0)
+		} else if (errno == EINVAL && fi_contiguous == 0 && sw_support == 0) {
 			tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
-		else
-			tst_brk(TFAIL | TTERRNO, "swapon() on %s failed", fstype);
+		} else {
+			tst_res(TFAIL | TTERRNO, "swapon() on %s failed", fstype);
+			return 0;
+		}
 	}
 
 	TEST(tst_syscall(__NR_swapoff, filename, 0));
-	if (TST_RET == -1)
-		tst_brk(TFAIL | TTERRNO, "swapoff on %s failed", fstype);
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "swapoff on %s failed", fstype);
+		return 0;
+	}
+
+	return (TST_RET == 0);
 }
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [LTP] [PATCH v5 7/8] swapon/off: enable all_filesystem in swap test
  2024-01-28  2:48 ` [LTP] [PATCH v5 7/8] swapon/off: enable all_filesystem in swap test Li Wang
@ 2024-01-29  8:03   ` Petr Vorel
  2024-01-29  8:26     ` Li Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2024-01-29  8:03 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hi Li,

> +++ b/testcases/kernel/syscalls/swapoff/swapoff02.c
> @@ -18,6 +18,8 @@
>  #include "lapi/syscalls.h"
>  #include "libswap.h"

> +#define MNTPOINT	"mntpoint"
> +
>  static int setup01(void);
>  static void cleanup01(void);

> @@ -84,14 +86,13 @@ static void setup(void)

>  	is_swap_supported("./tstswap");
This needs to be run on MNTPOINT, see swapon01.c (change from second commit I
made: "swapon01: Test on all filesystems").

The same problem is in other tests. Otherwise we check always on TMPDIR
(which TCONF when /tmp is tmpfs).

Kind regards,
Petr

# ./swapoff02
tst_device.c:96: TINFO: Found free device 0 '/dev/loop0'
tst_test.c:1709: TINFO: LTP version: 20230929-307-g5485ddaaf
tst_test.c:1593: TINFO: Timeout per run is 0h 00m 30s
tst_supported_fs_types.c:97: TINFO: Kernel supports ext2
tst_supported_fs_types.c:62: TINFO: mkfs.ext2 does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports ext3
tst_supported_fs_types.c:62: TINFO: mkfs.ext3 does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports ext4
tst_supported_fs_types.c:62: TINFO: mkfs.ext4 does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports xfs
tst_supported_fs_types.c:62: TINFO: mkfs.xfs does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports btrfs
tst_supported_fs_types.c:62: TINFO: mkfs.btrfs does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports bcachefs
tst_supported_fs_types.c:62: TINFO: mkfs.bcachefs does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports vfat
tst_supported_fs_types.c:62: TINFO: mkfs.vfat does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports exfat
tst_supported_fs_types.c:62: TINFO: mkfs.exfat does exist
tst_supported_fs_types.c:132: TINFO: FUSE does support ntfs
tst_supported_fs_types.c:62: TINFO: mkfs.ntfs does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports tmpfs
tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
tst_test.c:1669: TINFO: === Testing on ext2 ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on ext3 ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on ext4 ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on xfs ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on btrfs ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on bcachefs ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on vfat ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on exfat ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on ntfs ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented
tst_test.c:1669: TINFO: === Testing on tmpfs ===
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:202: TCONF: Swapfile on tmpfs not implemented

Summary:
passed   0
failed   0
broken   0
skipped  10
warnings 0

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status
  2024-01-28  2:48 ` [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status Li Wang
@ 2024-01-29  8:03   ` Petr Vorel
  2024-01-29 11:36     ` Li Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2024-01-29  8:03 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

> This updates the is_swap_supported function in the libltpswap
> to return an integer status instead of void, allowing the function
> to communicate success or failure to the caller. It introduces
> checks and returns 0 on various failure conditions while logging
> the failure without aborting the test case.

> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  include/libswap.h         |  2 +-
>  libs/libltpswap/libswap.c | 28 ++++++++++++++++++----------
>  2 files changed, 19 insertions(+), 11 deletions(-)

> diff --git a/include/libswap.h b/include/libswap.h
> index e67d65756..1e09db031 100644
> --- a/include/libswap.h
> +++ b/include/libswap.h
> @@ -20,5 +20,5 @@ int make_swapfile(const char *swapfile, int blocks, int safe);
>   * Check swapon/swapoff support status of filesystems or files
>   * we are testing on.
>   */
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks for further improving it. Few comments to fix old issues, when we are at
it (and plan to merge after the release). But feel free to ignore.

BTW do you plan to use it somewhere?

testcases/kernel/syscalls/swapoff/swapoff01.c

nit: I would comment here the return code.
Also maybe use bool (from stdbool.h)? The advantage is that we don't think what
the return code is (sometimes we use syscalls approach when 0 is success,
otherwise - like here - 0 is failure). Slowly converting to bool would be the
best.

> -void is_swap_supported(const char *filename);
> +int is_swap_supported(const char *filename);
>  #endif /* __LIBSWAP_H__ */
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 8aecad48d..14b7d9e3a 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -168,7 +168,7 @@ int make_swapfile(const char *swapfile, int blocks, int safe)
>   * Check swapon/swapoff support status of filesystems or files
>   * we are testing on.
>   */
nit: Although I did not like doc being just at the header, I now prefer it now
to have it just at the header because we don't have to sync the same text on two
places.

Kind regards,
Petr

> -void is_swap_supported(const char *filename)
> +int is_swap_supported(const char *filename)
>  {
>  	int i, sw_support = 0;
>  	int ret = make_swapfile(filename, 10, 1);
...

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [LTP] [PATCH v5 7/8] swapon/off: enable all_filesystem in swap test
  2024-01-29  8:03   ` Petr Vorel
@ 2024-01-29  8:26     ` Li Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2024-01-29  8:26 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On Mon, Jan 29, 2024 at 4:03 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > +++ b/testcases/kernel/syscalls/swapoff/swapoff02.c
> > @@ -18,6 +18,8 @@
> >  #include "lapi/syscalls.h"
> >  #include "libswap.h"
>
> > +#define MNTPOINT     "mntpoint"
> > +
> >  static int setup01(void);
> >  static void cleanup01(void);
>
> > @@ -84,14 +86,13 @@ static void setup(void)
>
> >       is_swap_supported("./tstswap");
> This needs to be run on MNTPOINT, see swapon01.c (change from second
> commit I
> made: "swapon01: Test on all filesystems").
>
> The same problem is in other tests. Otherwise we check always on TMPDIR
> (which TCONF when /tmp is tmpfs).
>

Oh yes, you're right, I overlooked that. Thanks.

-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status
  2024-01-29  8:03   ` Petr Vorel
@ 2024-01-29 11:36     ` Li Wang
  2024-01-29 12:22       ` Petr Vorel
  0 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2024-01-29 11:36 UTC (permalink / raw)
  To: Petr Vorel, Cyril Hrubis; +Cc: ltp

Hi Petr,

On Mon, Jan 29, 2024 at 4:03 PM Petr Vorel <pvorel@suse.cz> wrote:

> > This updates the is_swap_supported function in the libltpswap
> > to return an integer status instead of void, allowing the function
> > to communicate success or failure to the caller. It introduces
> > checks and returns 0 on various failure conditions while logging
> > the failure without aborting the test case.
>
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > ---
> >  include/libswap.h         |  2 +-
> >  libs/libltpswap/libswap.c | 28 ++++++++++++++++++----------
> >  2 files changed, 19 insertions(+), 11 deletions(-)
>
> > diff --git a/include/libswap.h b/include/libswap.h
> > index e67d65756..1e09db031 100644
> > --- a/include/libswap.h
> > +++ b/include/libswap.h
> > @@ -20,5 +20,5 @@ int make_swapfile(const char *swapfile, int blocks,
> int safe);
> >   * Check swapon/swapoff support status of filesystems or files
> >   * we are testing on.
> >   */
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Thanks for further improving it. Few comments to fix old issues, when we
> are at
> it (and plan to merge after the release). But feel free to ignore.
>
> BTW do you plan to use it somewhere?
>
> testcases/kernel/syscalls/swapoff/swapoff01.c
>
> nit: I would comment here the return code.
> Also maybe use bool (from stdbool.h)? The advantage is that we don't think
> what
> the return code is (sometimes we use syscalls approach when 0 is success,
> otherwise - like here - 0 is failure). Slowly converting to bool would be
> the
> best.
>

Good point, actually I am tired of figuring out the return 0 means 'FAIL' or
'SUCCESS' in the lib, it messes a lot in some functions. And as you suggest,
bool value should be a good way to resolve this.

But the bool type is not built into the language prior to the C99 standard.
The C99 standard introduced a _Bool type and the header <stdbool.h>,
which defines bool as a macro for _Bool.

I am not sure if LTP nowadays only supports C99 and later.
or should we make use of bool safely (or use syscalls approach
0 == success) in our patch?

@Cyril Hrubis <chrubis@suse.cz> what do you think?




> > @@ -168,7 +168,7 @@ int make_swapfile(const char *swapfile, int blocks,
> int safe)
> >   * Check swapon/swapoff support status of filesystems or files
> >   * we are testing on.
> >   */
> nit: Although I did not like doc being just at the header, I now prefer it
> now
> to have it just at the header because we don't have to sync the same text
> on two
> places.
>

Agree.


-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status
  2024-01-29 11:36     ` Li Wang
@ 2024-01-29 12:22       ` Petr Vorel
  0 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2024-01-29 12:22 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

> Hi Petr,

> On Mon, Jan 29, 2024 at 4:03 PM Petr Vorel <pvorel@suse.cz> wrote:

> > > This updates the is_swap_supported function in the libltpswap
> > > to return an integer status instead of void, allowing the function
> > > to communicate success or failure to the caller. It introduces
> > > checks and returns 0 on various failure conditions while logging
> > > the failure without aborting the test case.

> > > Signed-off-by: Li Wang <liwang@redhat.com>
> > > ---
> > >  include/libswap.h         |  2 +-
> > >  libs/libltpswap/libswap.c | 28 ++++++++++++++++++----------
> > >  2 files changed, 19 insertions(+), 11 deletions(-)

> > > diff --git a/include/libswap.h b/include/libswap.h
> > > index e67d65756..1e09db031 100644
> > > --- a/include/libswap.h
> > > +++ b/include/libswap.h
> > > @@ -20,5 +20,5 @@ int make_swapfile(const char *swapfile, int blocks,
> > int safe);
> > >   * Check swapon/swapoff support status of filesystems or files
> > >   * we are testing on.
> > >   */
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > Thanks for further improving it. Few comments to fix old issues, when we
> > are at
> > it (and plan to merge after the release). But feel free to ignore.

> > BTW do you plan to use it somewhere?

> > testcases/kernel/syscalls/swapoff/swapoff01.c

> > nit: I would comment here the return code.
> > Also maybe use bool (from stdbool.h)? The advantage is that we don't think
> > what
> > the return code is (sometimes we use syscalls approach when 0 is success,
> > otherwise - like here - 0 is failure). Slowly converting to bool would be
> > the
> > best.


> Good point, actually I am tired of figuring out the return 0 means 'FAIL' or
> 'SUCCESS' in the lib, it messes a lot in some functions. And as you suggest,
> bool value should be a good way to resolve this.

> But the bool type is not built into the language prior to the C99 standard.
> The C99 standard introduced a _Bool type and the header <stdbool.h>,
> which defines bool as a macro for _Bool.

> I am not sure if LTP nowadays only supports C99 and later.
> or should we make use of bool safely (or use syscalls approach
> 0 == success) in our patch?

FYI now compile with -std=gnu99, see include/mk/config.mk.in
dc7be30e2 ("config: Explicitly set gnu99")
So I would say LTP supports C99 with GNU extensions.

> @Cyril Hrubis <chrubis@suse.cz> what do you think?

FYI we already use stdbool.h and bool on some places in the library.

Kind regards,
Petr



> > > @@ -168,7 +168,7 @@ int make_swapfile(const char *swapfile, int blocks,
> > int safe)
> > >   * Check swapon/swapoff support status of filesystems or files
> > >   * we are testing on.
> > >   */
> > nit: Although I did not like doc being just at the header, I now prefer it
> > now
> > to have it just at the header because we don't have to sync the same text
> > on two
> > places.


> Agree.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-01-29 12:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-28  2:48 [LTP] [PATCH v5 0/8] improvement work on libswap library Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 1/8] libswap: add known swap supported fs check Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 2/8] swapon01: Test on all filesystems Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 3/8] swapon01: Improving test with memory limits and swap reporting Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 4/8] libswap: add function to prealloc contiguous file Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 5/8] libswap: Introduce file contiguity check Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 6/8] libswap: customize swapfile size Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 7/8] swapon/off: enable all_filesystem in swap test Li Wang
2024-01-29  8:03   ` Petr Vorel
2024-01-29  8:26     ` Li Wang
2024-01-28  2:48 ` [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status Li Wang
2024-01-29  8:03   ` Petr Vorel
2024-01-29 11:36     ` Li Wang
2024-01-29 12:22       ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.