diff mbox series

[v5,6/7] syscalls/swapon03: Simply this case

Message ID 20240226135336.19733-6-xuyang2018.jy@fujitsu.com
State Accepted
Headers show
Series [v5,1/7] libltpswap: Add tst_max_swapfiles API | expand

Commit Message

Yang Xu Feb. 26, 2024, 1:53 p.m. UTC
By moving swapfile create stage from verify_swaopon and
test EPERM error more accurate. Also use glibc wrapper by
using swapon/swapoff instead of call syscall number directly
because glibc/musl/binoic also support them since long time ago.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/swapon/swapon03.c | 167 +++++---------------
 1 file changed, 41 insertions(+), 126 deletions(-)

Comments

Petr Vorel Feb. 28, 2024, 7:24 a.m. UTC | #1
Hi Yang Xu,

> By moving swapfile create stage from verify_swaopon and
> test EPERM error more accurate. Also use glibc wrapper by
> using swapon/swapoff instead of call syscall number directly
> because glibc/musl/binoic also support them since long time ago.
s/binoic/bionic/

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr
Yang Xu Feb. 28, 2024, 8:15 a.m. UTC | #2
Hi Petr

> Hi Yang Xu,
> 
>> By moving swapfile create stage from verify_swaopon and
>> test EPERM error more accurate. Also use glibc wrapper by
>> using swapon/swapoff instead of call syscall number directly
>> because glibc/musl/binoic also support them since long time ago.
> s/binoic/bionic/
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks for your nice review. Sorry, when your review v4 patch, I don't 
solve all comment in v5. So I did it in my own ltp fork branch named as 
v5_max_swapfiles[1](such as swapfiles => swap file, snprint return 
value, else branch....).

ps: Only the 6th patch was modified, other patches aren't modified. If 
you see no problem, I plan to merge this patchset today.

[1]https://github.com/xuyang0410/ltp/commits/v5_max_swapfiles/

Best Regards
Yang Xu
> 
> Kind regards,
> Petr
Petr Vorel Feb. 28, 2024, 1:27 p.m. UTC | #3
Hi Yang Xu,

> Hi Petr

> > Hi Yang Xu,

> >> By moving swapfile create stage from verify_swaopon and
> >> test EPERM error more accurate. Also use glibc wrapper by
> >> using swapon/swapoff instead of call syscall number directly
> >> because glibc/musl/binoic also support them since long time ago.
> > s/binoic/bionic/

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> Thanks for your nice review. Sorry, when your review v4 patch, I don't 
> solve all comment in v5. So I did it in my own ltp fork branch named as 
> v5_max_swapfiles[1](such as swapfiles => swap file, snprint return 
> value, else branch....).

> ps: Only the 6th patch was modified, other patches aren't modified. If 
> you see no problem, I plan to merge this patchset today.

[2] looks good to me, please merge. Very nit (feel free to ignore):
snprintf(cmd_buffer, ...) is too long, renaming cmd_buffer just to buf would
shorten whole line.

Kind regards,
Petr

> [1]https://github.com/xuyang0410/ltp/commits/v5_max_swapfiles/

[2] https://github.com/xuyang0410/ltp/commit/9e83b1fb1d8f8631122c70a336aacd28f94d8343

> Best Regards
> Yang Xu

> > Kind regards,
> > Petr
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index 26385a5ae..30410e1cd 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -16,7 +16,7 @@ 
 #include <errno.h>
 #include <stdlib.h>
 #include <sys/wait.h>
-
+#include <sys/swap.h>
 #include "tst_test.h"
 #include "lapi/syscalls.h"
 #include "libswap.h"
@@ -24,88 +24,13 @@ 
 #define MNTPOINT	"mntpoint"
 #define TEST_FILE	MNTPOINT"/testswap"
 
-static int setup_swap(void);
-static int clean_swap(void);
-static int check_and_swapoff(const char *filename);
-
 static int swapfiles;
 
-int testfiles = 3;
-static struct swap_testfile_t {
-	char *filename;
-} swap_testfiles[] = {
-	{"firstswapfile"},
-	{"secondswapfile"},
-	{"thirdswapfile"}
-};
-
-int expected_errno = EPERM;
-
-static void verify_swapon(void)
-{
-	if (setup_swap() < 0) {
-		clean_swap();
-		tst_brk(TBROK, "Setup failed, quitting the test");
-	}
-
-	TEST(tst_syscall(__NR_swapon, swap_testfiles[0].filename, 0));
-
-	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
-		tst_res(TPASS | TTERRNO, "swapon(2) got expected failure");
-	} else if (TST_RET < 0) {
-		tst_res(TFAIL | TTERRNO,
-			"swapon(2) failed to produce expected error "
-			"(%d). System reboot recommended.",
-			expected_errno);
-	} else {
-		/*
-		 * Probably the system supports MAX_SWAPFILES > 30, let's try with
-		 * MAX_SWAPFILES == 32. Call swapon sys call once again for 32
-		 * now we can't receive an error.
-		 */
-		TEST(tst_syscall(__NR_swapon, swap_testfiles[1].filename, 0));
-
-		/* Check return code (now we're expecting success) */
-		if (TST_RET < 0) {
-			tst_res(TFAIL | TTERRNO,
-				"swapon(2) got an unexpected failure");
-		} else {
-			/*
-			 * Call swapon sys call once again for 33 now we have to receive an error.
-			 */
-			TEST(tst_syscall(__NR_swapon, swap_testfiles[2].filename, 0));
-
-			/* Check return code (should be an error) */
-			if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
-				tst_res(TPASS,
-					"swapon(2) got expected failure;"
-					" Got errno = %d, probably your"
-					" MAX_SWAPFILES is 32",
-					expected_errno);
-			} else {
-				tst_res(TFAIL,
-					"swapon(2) failed to produce"
-					" expected error: %d, got %s."
-					" System reboot after execution of LTP"
-					" test suite is recommended.",
-					expected_errno, strerror(TST_ERR));
-			}
-		}
-	}
-
-	if (clean_swap() < 0)
-		tst_brk(TBROK, "Cleanup failed, quitting the test");
-}
-
-/*
- * Create 33 and activate 30 swapfiles.
- */
 static int setup_swap(void)
 {
 	pid_t pid;
 	int status;
 	int j, max_swapfiles, used_swapfiles;
-	int res = 0;
 	char filename[FILENAME_MAX];
 
 	SAFE_SETEUID(0);
@@ -127,16 +52,7 @@  static int setup_swap(void)
 			make_swapfile(filename, 10, 0);
 
 			/* turn on the swap file */
-			res = tst_syscall(__NR_swapon, filename, 0);
-			if (res != 0) {
-				if (errno == EPERM) {
-					printf("Successfully created %d swapfiles\n", j);
-					break;
-				} else {
-					printf("Failed to create swapfile: %s\n", filename);
-					exit(1);
-				}
-			}
+			TST_EXP_PASS_SILENT(swapon(filename, 0));
 		}
 		exit(0);
 	} else
@@ -145,13 +61,40 @@  static int setup_swap(void)
 	if (WEXITSTATUS(status))
 		tst_brk(TFAIL, "Failed to setup swaps");
 
-	/* Create all needed extra swapfiles for testing */
-	for (j = 0; j < testfiles; j++)
-		make_swapfile(swap_testfiles[j].filename, 10, 0);
+	tst_res(TINFO, "Successfully created %d swapfiles", swapfiles);
+	make_swapfile(TEST_FILE, 10, 0);
 
 	return 0;
 }
 
+/*
+ * Check if the file is at /proc/swaps and remove it giving swapoff
+ */
+static int check_and_swapoff(const char *filename)
+{
+	char cmd_buffer[256];
+	int rc = -1;
+
+	if (snprintf(cmd_buffer, sizeof(cmd_buffer),
+		"grep -q '%s.*file' /proc/swaps", filename) < 0) {
+		tst_res(TWARN, "sprintf() failed to create the command string");
+	} else {
+		rc = 0;
+		if (system(cmd_buffer) == 0) {
+			/* now we need to swapoff the file */
+			if (swapoff(filename) != 0) {
+				tst_res(TWARN, "Failed to turn off swap "
+					"file. system reboot after "
+					"execution of LTP test suite "
+					"is recommended");
+				rc = -1;
+			}
+		}
+	}
+
+	return rc;
+}
+
 /*
  * Turn off all swapfiles previously turned on
  */
@@ -169,49 +112,17 @@  static int clean_swap(void)
 		}
 	}
 
-	for (j = 0; j < testfiles; j++) {
-		if (check_and_swapoff(swap_testfiles[j].filename) != 0) {
-			tst_res(TWARN, "Failed to turn off swap file %s.",
-				 swap_testfiles[j].filename);
-			return -1;
-		}
+	if (check_and_swapoff("testfile") != 0) {
+		tst_res(TWARN, "Failed to turn off swap file testfile");
+		return -1;
 	}
 
 	return 0;
 }
 
-/*
- * Check if the file is at /proc/swaps and remove it giving swapoff
- */
-static int check_and_swapoff(const char *filename)
+static void verify_swapon(void)
 {
-	char cmd_buffer[256];
-	int rc = -1;
-
-	if (snprintf(cmd_buffer, sizeof(cmd_buffer),
-		     "grep -q '%s.*file' /proc/swaps", filename) < 0) {
-		tst_res(TWARN, "sprintf() failed to create the command string");
-	} else {
-
-		rc = 0;
-
-		if (system(cmd_buffer) == 0) {
-
-			/* now we need to swapoff the file */
-			if (tst_syscall(__NR_swapoff, filename) != 0) {
-
-				tst_res(TWARN, "Failed to turn off swap "
-					 "file. system reboot after "
-					 "execution of LTP test suite "
-					 "is recommended");
-				rc = -1;
-
-			}
-
-		}
-	}
-
-	return rc;
+	TST_EXP_FAIL(swapon(TEST_FILE, 0), EPERM, "swapon(%s, 0)", TEST_FILE);
 }
 
 static void setup(void)
@@ -220,6 +131,10 @@  static void setup(void)
 		tst_brk(TCONF, "swap not supported by kernel");
 
 	is_swap_supported(TEST_FILE);
+	if (setup_swap() < 0) {
+		clean_swap();
+		tst_brk(TBROK, "Setup failed, quitting the test");
+	}
 }
 
 static void cleanup(void)