diff mbox series

[v2] Remove ltp_clone_quick usage from pidns suite

Message ID 20230314134242.9203-1-andrea.cervesato@suse.de
State Superseded
Headers show
Series [v2] Remove ltp_clone_quick usage from pidns suite | expand

Commit Message

Andrea Cervesato March 14, 2023, 1:42 p.m. UTC 
From: Andrea Cervesato <andrea.cervesato@suse.com>

Replaced ltp_clone_quick with SAFE_CLONE inside the pidns testing
suite. Fixed also errors in pidns0[12] where in previous patches we
used CLONE_NEWNS instead of CLONE_NEWPID.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/containers/pidns/pidns01.c | 24 ++++++++++--------
 testcases/kernel/containers/pidns/pidns02.c | 26 +++++++++++--------
 testcases/kernel/containers/pidns/pidns03.c | 18 +++++++------
 testcases/kernel/containers/pidns/pidns12.c | 22 ++++++++++------
 testcases/kernel/containers/pidns/pidns20.c | 28 +++++++++++++--------
 5 files changed, 72 insertions(+), 46 deletions(-)

Comments

Petr Vorel March 29, 2023, 5:39 a.m. UTC | #1 
Hi all,

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

Kind regards,
Petr
Cyril Hrubis April 28, 2023, 1:07 p.m. UTC | #2 
Hi!
> diff --git a/testcases/kernel/containers/pidns/pidns02.c b/testcases/kernel/containers/pidns/pidns02.c
> index b8913d3f6..5372aeef9 100644
> --- a/testcases/kernel/containers/pidns/pidns02.c
> +++ b/testcases/kernel/containers/pidns/pidns02.c
> @@ -7,7 +7,7 @@
>  /*\
>   * [Description]
>   *
> - * Clone a process with CLONE_NEWNS flag and check:
> + * Clone a process with CLONE_NEWPID flag and check:
>   *
>   * - child session ID must be 1
>   * - parent process group ID must be 1
> @@ -16,29 +16,35 @@
>  #include "tst_test.h"
>  #include "lapi/sched.h"
>  
> -static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
> +static void child_func(void)
>  {
>  	pid_t sid, pgid;
>  
> +	SAFE_SETSID();

Can we please avoid the setsid() here? Once we do that we do not
actually test that the sid and pgid are initialized to something
meaningful. It makes much more sense to check if sid and pgid are 0,
since init process has no parent and ppid is 0 then also the sid and
pgid may make sense to be initialized to 0 since they are "inherited"
from nonexistent parent.

Or we can as well do:

	TST_EXP_EQ_LI(getsid(0), 0);
	TST_EXP_EQ_LI(getpgid(0), 0);

	tst_res(TINFO, "setsid()");
	SAFE_SETSID();

	TST_EXP_EQ_LI(getsid(0), 1);
	TST_EXP_EQ_LI(getpgid(0), 1);

That way we check both the initial values and that setsid works as
expected.

The rest looks good. With the pidns02.c fixed:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
diff mbox series

Patch

diff --git a/testcases/kernel/containers/pidns/pidns01.c b/testcases/kernel/containers/pidns/pidns01.c
index 5080b6fad..6b5ec48ac 100644
--- a/testcases/kernel/containers/pidns/pidns01.c
+++ b/testcases/kernel/containers/pidns/pidns01.c
@@ -8,7 +8,7 @@ 
 /*\
  * [Description]
  *
- * Clone a process with CLONE_NEWNS flag and check:
+ * Clone a process with CLONE_NEWPID flag and check:
  *
  * - child process ID must be 1
  * - parent process ID must be 0
@@ -17,29 +17,33 @@ 
 #include "tst_test.h"
 #include "lapi/sched.h"
 
-static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
+static void child_func(void)
 {
 	pid_t cpid, ppid;
 
 	cpid = getpid();
 	ppid = getppid();
 
-	TST_EXP_PASS(cpid == 1);
-	TST_EXP_PASS(ppid == 0);
-
-	return 0;
+	TST_EXP_EQ_LI(cpid, 1);
+	TST_EXP_EQ_LI(ppid, 0);
 }
 
 static void run(void)
 {
-	int ret;
+	const struct tst_clone_args args = { CLONE_NEWPID, SIGCHLD };
 
-	ret = ltp_clone_quick(CLONE_NEWNS | SIGCHLD, child_func, NULL);
-	if (ret < 0)
-		tst_brk(TBROK | TERRNO, "clone failed");
+	if (!SAFE_CLONE(&args)) {
+		child_func();
+		return;
+	}
 }
 
 static struct tst_test test = {
 	.test_all = run,
 	.needs_root = 1,
+	.forks_child = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_PID_NS",
+		NULL,
+	},
 };
diff --git a/testcases/kernel/containers/pidns/pidns02.c b/testcases/kernel/containers/pidns/pidns02.c
index b8913d3f6..5372aeef9 100644
--- a/testcases/kernel/containers/pidns/pidns02.c
+++ b/testcases/kernel/containers/pidns/pidns02.c
@@ -7,7 +7,7 @@ 
 /*\
  * [Description]
  *
- * Clone a process with CLONE_NEWNS flag and check:
+ * Clone a process with CLONE_NEWPID flag and check:
  *
  * - child session ID must be 1
  * - parent process group ID must be 1
@@ -16,29 +16,35 @@ 
 #include "tst_test.h"
 #include "lapi/sched.h"
 
-static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
+static void child_func(void)
 {
 	pid_t sid, pgid;
 
+	SAFE_SETSID();
+
 	sid = getsid(0);
 	pgid = getpgid(0);
 
-	TST_EXP_PASS(sid == 1);
-	TST_EXP_PASS(pgid == 1);
-
-	return 0;
+	TST_EXP_EQ_LI(sid, 1);
+	TST_EXP_EQ_LI(pgid, 1);
 }
 
 static void run(void)
 {
-	int ret;
+	const struct tst_clone_args args = { CLONE_NEWPID, SIGCHLD };
 
-	ret = ltp_clone_quick(CLONE_NEWNS | SIGCHLD, child_func, NULL);
-	if (ret < 0)
-		tst_brk(TBROK | TERRNO, "clone failed");
+	if (!SAFE_CLONE(&args)) {
+		child_func();
+		return;
+	}
 }
 
 static struct tst_test test = {
 	.test_all = run,
 	.needs_root = 1,
+	.forks_child = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_PID_NS",
+		NULL,
+	},
 };
diff --git a/testcases/kernel/containers/pidns/pidns03.c b/testcases/kernel/containers/pidns/pidns03.c
index 122ba7891..d0d26c8a5 100644
--- a/testcases/kernel/containers/pidns/pidns03.c
+++ b/testcases/kernel/containers/pidns/pidns03.c
@@ -17,7 +17,7 @@ 
 
 #define PROCDIR "proc"
 
-static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
+static void child_func(void)
 {
 	char proc_self[10];
 
@@ -28,8 +28,6 @@  static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
 	SAFE_UMOUNT(PROCDIR);
 
 	TST_EXP_PASS(strcmp(proc_self, "1"), PROCDIR"/self contains 1:");
-
-	return 0;
 }
 
 static void setup(void)
@@ -45,11 +43,12 @@  static void cleanup(void)
 
 static void run(void)
 {
-	int ret;
+	const struct tst_clone_args args = { CLONE_NEWPID, SIGCHLD };
 
-	ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
-	if (ret < 0)
-		tst_brk(TBROK | TERRNO, "clone failed");
+	if (!SAFE_CLONE(&args)) {
+		child_func();
+		return;
+	}
 }
 
 static struct tst_test test = {
@@ -57,5 +56,10 @@  static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	.forks_child = 1,
 	.needs_tmpdir = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_PID_NS",
+		NULL,
+	},
 };
diff --git a/testcases/kernel/containers/pidns/pidns12.c b/testcases/kernel/containers/pidns/pidns12.c
index fb1ec90ca..ab035f33d 100644
--- a/testcases/kernel/containers/pidns/pidns12.c
+++ b/testcases/kernel/containers/pidns/pidns12.c
@@ -25,7 +25,7 @@  static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LT
 	sig_pid = si->si_pid;
 }
 
-static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
+static void child_func(void)
 {
 	struct sigaction sa;
 
@@ -41,21 +41,22 @@  static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
 	TST_CHECKPOINT_WAKE_AND_WAIT(0);
 
 	TST_EXP_EQ_LI(sig_pid, 0);
-
-	return 0;
 }
 
 static void run(void)
 {
-	int ret;
+	const struct tst_clone_args args = { CLONE_NEWPID, SIGCHLD };
+	int pid;
 
-	ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
-	if (ret < 0)
-		tst_brk(TBROK | TERRNO, "clone failed");
+	pid = SAFE_CLONE(&args);
+	if (!pid) {
+		child_func();
+		return;
+	}
 
 	TST_CHECKPOINT_WAIT(0);
 
-	SAFE_KILL(ret, SIGUSR1);
+	SAFE_KILL(pid, SIGUSR1);
 
 	TST_CHECKPOINT_WAKE(0);
 }
@@ -63,5 +64,10 @@  static void run(void)
 static struct tst_test test = {
 	.test_all = run,
 	.needs_root = 1,
+	.forks_child = 1,
 	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_PID_NS",
+		NULL,
+	},
 };
diff --git a/testcases/kernel/containers/pidns/pidns20.c b/testcases/kernel/containers/pidns/pidns20.c
index 9f369699a..4d0924c4e 100644
--- a/testcases/kernel/containers/pidns/pidns20.c
+++ b/testcases/kernel/containers/pidns/pidns20.c
@@ -26,7 +26,7 @@  static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LT
 	signals++;
 }
 
-static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
+static void child_func(void)
 {
 	struct sigaction sa;
 	sigset_t newset;
@@ -37,7 +37,7 @@  static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
 
 	if (cpid != 1 || ppid != 0) {
 		tst_res(TFAIL, "Got unexpected result of cpid=%d ppid=%d", cpid, ppid);
-		return 0;
+		return;
 	}
 
 	SAFE_SIGEMPTYSET(&newset);
@@ -56,30 +56,31 @@  static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
 
 	if (signals != 1) {
 		tst_res(TFAIL, "Received %d signals", signals);
-		return 0;
+		return;
 	}
 
 	if (last_signo != SIGUSR1) {
 		tst_res(TFAIL, "Received %s signal", tst_strsig(last_signo));
-		return 0;
+		return;
 	}
 
 	tst_res(TPASS, "Received SIGUSR1 signal after unblock");
-
-	return 0;
 }
 
 static void run(void)
 {
-	int ret;
+	const struct tst_clone_args args = { CLONE_NEWPID, SIGCHLD };
+	int pid;
 
-	ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
-	if (ret < 0)
-		tst_brk(TBROK | TERRNO, "clone failed");
+	pid = SAFE_CLONE(&args);
+	if (!pid) {
+		child_func();
+		return;
+	}
 
 	TST_CHECKPOINT_WAIT(0);
 
-	SAFE_KILL(ret, SIGUSR1);
+	SAFE_KILL(pid, SIGUSR1);
 
 	TST_CHECKPOINT_WAKE(0);
 }
@@ -87,5 +88,10 @@  static void run(void)
 static struct tst_test test = {
 	.test_all = run,
 	.needs_root = 1,
+	.forks_child = 1,
 	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_PID_NS",
+		NULL,
+	},
 };