ltp.lists.linux.it archive mirror
 help / color / mirror / Atom feed
* [LTP] [PATCH v1] Remove ltp_clone_quick usage from pidns suite
@ 2023-02-28 12:43 Andrea Cervesato via ltp
  2023-03-13 10:04 ` Richard Palethorpe
  0 siblings, 1 reply; 3+ messages in thread
From: Andrea Cervesato via ltp @ 2023-02-28 12:43 UTC (permalink / raw)
  To: ltp

Replaced ltp_clone_quick with SAFE_CLONE inside the pidns testing
suite.

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

diff --git a/testcases/kernel/containers/pidns/pidns01.c b/testcases/kernel/containers/pidns/pidns01.c
index 5080b6fad..998da7d95 100644
--- a/testcases/kernel/containers/pidns/pidns01.c
+++ b/testcases/kernel/containers/pidns/pidns01.c
@@ -17,7 +17,7 @@
 #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;
 
@@ -26,20 +26,20 @@ static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
 
 	TST_EXP_PASS(cpid == 1);
 	TST_EXP_PASS(ppid == 0);
-
-	return 0;
 }
 
 static void run(void)
 {
-	int ret;
+	const struct tst_clone_args args = { CLONE_NEWNS, 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,
 };
diff --git a/testcases/kernel/containers/pidns/pidns02.c b/testcases/kernel/containers/pidns/pidns02.c
index b8913d3f6..57102f652 100644
--- a/testcases/kernel/containers/pidns/pidns02.c
+++ b/testcases/kernel/containers/pidns/pidns02.c
@@ -16,7 +16,7 @@
 #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;
 
@@ -25,20 +25,20 @@ static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
 
 	TST_EXP_PASS(sid == 1);
 	TST_EXP_PASS(pgid == 1);
-
-	return 0;
 }
 
 static void run(void)
 {
-	int ret;
+	const struct tst_clone_args args = { CLONE_NEWNS, 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,
 };
diff --git a/testcases/kernel/containers/pidns/pidns03.c b/testcases/kernel/containers/pidns/pidns03.c
index 122ba7891..50e69a9f3 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,6 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	.forks_child = 1,
 	.needs_tmpdir = 1,
 };
diff --git a/testcases/kernel/containers/pidns/pidns12.c b/testcases/kernel/containers/pidns/pidns12.c
index fb1ec90ca..2631a7451 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,6 @@ static void run(void)
 static struct tst_test test = {
 	.test_all = run,
 	.needs_root = 1,
+	.forks_child = 1,
 	.needs_checkpoints = 1,
 };
diff --git a/testcases/kernel/containers/pidns/pidns20.c b/testcases/kernel/containers/pidns/pidns20.c
index 9f369699a..c3d63d125 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,6 @@ static void run(void)
 static struct tst_test test = {
 	.test_all = run,
 	.needs_root = 1,
+	.forks_child = 1,
 	.needs_checkpoints = 1,
 };
-- 
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 v1] Remove ltp_clone_quick usage from pidns suite
  2023-02-28 12:43 [LTP] [PATCH v1] Remove ltp_clone_quick usage from pidns suite Andrea Cervesato via ltp
@ 2023-03-13 10:04 ` Richard Palethorpe
  2023-03-29  5:33   ` Petr Vorel
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Palethorpe @ 2023-03-13 10:04 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hello,

Andrea Cervesato via ltp <ltp@lists.linux.it> writes:

> Replaced ltp_clone_quick with SAFE_CLONE inside the pidns testing
> suite.
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  testcases/kernel/containers/pidns/pidns01.c | 14 ++++++------
>  testcases/kernel/containers/pidns/pidns02.c | 14 ++++++------
>  testcases/kernel/containers/pidns/pidns03.c | 14 ++++++------
>  testcases/kernel/containers/pidns/pidns12.c | 18 +++++++++-------
>  testcases/kernel/containers/pidns/pidns20.c | 24 +++++++++++----------
>  5 files changed, 44 insertions(+), 40 deletions(-)
>
> diff --git a/testcases/kernel/containers/pidns/pidns01.c b/testcases/kernel/containers/pidns/pidns01.c
> index 5080b6fad..998da7d95 100644
> --- a/testcases/kernel/containers/pidns/pidns01.c
> +++ b/testcases/kernel/containers/pidns/pidns01.c
> @@ -17,7 +17,7 @@
>  #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;
>  
> @@ -26,20 +26,20 @@ static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
>  
>  	TST_EXP_PASS(cpid == 1);
>  	TST_EXP_PASS(ppid == 0);
> -
> -	return 0;
>  }
>  
>  static void run(void)
>  {
> -	int ret;
> +	const struct tst_clone_args args = { CLONE_NEWNS, SIGCHLD };

At some point CLONE_NEWNS was substituted for CLONE_NEWPID. It still
passes because TST_EXP_PASS expects a zero return val. It should be
TST_EXP_EXPR or similar.

>  
> -	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,

We need to check PID namespaces are supported in the kernel config.

>  };
> diff --git a/testcases/kernel/containers/pidns/pidns02.c b/testcases/kernel/containers/pidns/pidns02.c
> index b8913d3f6..57102f652 100644
> --- a/testcases/kernel/containers/pidns/pidns02.c
> +++ b/testcases/kernel/containers/pidns/pidns02.c
> @@ -16,7 +16,7 @@
>  #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;
>  
> @@ -25,20 +25,20 @@ static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
>  
>  	TST_EXP_PASS(sid == 1);
>  	TST_EXP_PASS(pgid == 1);
> -
> -	return 0;
>  }
>  
>  static void run(void)
>  {
> -	int ret;
> +	const struct tst_clone_args args = { CLONE_NEWNS, SIGCHLD };

Same issue here.

>  
> -	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,

Also needs config check

>  };
> diff --git a/testcases/kernel/containers/pidns/pidns03.c b/testcases/kernel/containers/pidns/pidns03.c
> index 122ba7891..50e69a9f3 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,6 @@ static struct tst_test test = {
>  	.setup = setup,
>  	.cleanup = cleanup,
>  	.needs_root = 1,
> +	.forks_child = 1,
>  	.needs_tmpdir = 1,

Need config check.

>  };
> diff --git a/testcases/kernel/containers/pidns/pidns12.c b/testcases/kernel/containers/pidns/pidns12.c
> index fb1ec90ca..2631a7451 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,6 @@ static void run(void)
>  static struct tst_test test = {
>  	.test_all = run,
>  	.needs_root = 1,
> +	.forks_child = 1,
>  	.needs_checkpoints = 1,

Needs config check

>  };
> diff --git a/testcases/kernel/containers/pidns/pidns20.c b/testcases/kernel/containers/pidns/pidns20.c
> index 9f369699a..c3d63d125 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,6 @@ static void run(void)
>  static struct tst_test test = {
>  	.test_all = run,
>  	.needs_root = 1,
> +	.forks_child = 1,
>  	.needs_checkpoints = 1,

Needs config check

>  };
> -- 
> 2.35.3


-- 
Thank you,
Richard.

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

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

* Re: [LTP] [PATCH v1] Remove ltp_clone_quick usage from pidns suite
  2023-03-13 10:04 ` Richard Palethorpe
@ 2023-03-29  5:33   ` Petr Vorel
  0 siblings, 0 replies; 3+ messages in thread
From: Petr Vorel @ 2023-03-29  5:33 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: ltp

Hi all,

> >  static void run(void)
> >  {
> > -	int ret;
> > +	const struct tst_clone_args args = { CLONE_NEWNS, SIGCHLD };

> At some point CLONE_NEWNS was substituted for CLONE_NEWPID. It still
> passes because TST_EXP_PASS expects a zero return val. It should be
> TST_EXP_EXPR or similar.

Good catch. I suppose copy paste error. These were introduced during testsuite
rewrite, later Andrea fixed the docparse description in 3fc912172, but not the
actual value in tst_clone_args.

Kind regards,
Petr

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

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

end of thread, other threads:[~2023-03-29  5:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-28 12:43 [LTP] [PATCH v1] Remove ltp_clone_quick usage from pidns suite Andrea Cervesato via ltp
2023-03-13 10:04 ` Richard Palethorpe
2023-03-29  5:33   ` Petr Vorel

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