ltp.lists.linux.it archive mirror
 help / color / mirror / Atom feed
* [LTP] [PATCH v2] Remove ltp_clone_quick usage from pidns suite
@ 2023-03-14 13:42 Andrea Cervesato
  2023-03-29  5:39 ` Petr Vorel
  2023-04-28 13:07 ` Cyril Hrubis
  0 siblings, 2 replies; 3+ messages in thread
From: Andrea Cervesato @ 2023-03-14 13:42 UTC (permalink / raw)
  To: ltp

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(-)

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,
+	},
 };
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [LTP] [PATCH v2] Remove ltp_clone_quick usage from pidns suite
  2023-03-14 13:42 [LTP] [PATCH v2] Remove ltp_clone_quick usage from pidns suite Andrea Cervesato
@ 2023-03-29  5:39 ` Petr Vorel
  2023-04-28 13:07 ` Cyril Hrubis
  1 sibling, 0 replies; 3+ messages in thread
From: Petr Vorel @ 2023-03-29  5:39 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi all,

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

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [LTP] [PATCH v2] Remove ltp_clone_quick usage from pidns suite
  2023-03-14 13:42 [LTP] [PATCH v2] Remove ltp_clone_quick usage from pidns suite Andrea Cervesato
  2023-03-29  5:39 ` Petr Vorel
@ 2023-04-28 13:07 ` Cyril Hrubis
  1 sibling, 0 replies; 3+ messages in thread
From: Cyril Hrubis @ 2023-04-28 13:07 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

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>

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-28 13:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-14 13:42 [LTP] [PATCH v2] Remove ltp_clone_quick usage from pidns suite Andrea Cervesato
2023-03-29  5:39 ` Petr Vorel
2023-04-28 13:07 ` Cyril Hrubis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).