| Message ID | 20250117122455.7989-1-chrubis@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] testcases/lib: Fix tst_ns_* helpers | expand |
On Fri, Jan 17, 2025 at 8:25 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Replaces SAFE_CLONE() with tst_clone() in the tst_ns_* helpers. > > The reason for the replacement is that SAFE_CLONE() uses > TST_RETRY_FUNC() which calls tst_multiply_timeout(). The problem with > that is that the tst_multiply_timeout() is a test library function that > started to print TINFO messages recently and that we rely on parsing the > output from the tst_ns_* helpers. > > The reason SAFE_CLONE() started to call TST_RETRY_FUNC() is that in the > case that we create new namespaces with the clone call, we may end up > creating them faster than kernel can clean them up which is described in: > > commit 7d882081a5613f44a12fc6b1c44267d4df0857a4 > Author: Petr Vorel <pvorel@suse.cz> > Date: Mon Mar 28 22:46:43 2022 +0200 > > lib: Retry safe_clone() on ENOSPC > > This combined with the newly introduced changes in the test library that > check for kernel debugging options that may need to adjust default > timeouts: > > commit 893ca0abe7e82851ff0e5d93c09b1098f2eff121 > Author: Li Wang <liwang@redhat.com> > Date: Sun Dec 22 15:22:49 2024 +0800 > > lib: multiply the timeout if detect slow kconfigs > > which adds tst_has_slow_kconfig() into the tst_multiply_timeout() causes > the TINFO messages to be printed. > > The reason why we can safely replace the SAFE_CLONE() with tst_clone() > here is that we are not creating new namspaces in the tst_ns_* helpers, > but rather than that cloning a new process to be executed inside of the > namespace, hence we do not need to retry on ENOSPC. > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > Reviewed-by: Li Wang <liwang@redhat.com> Nice work!
Hi Cyril, Li, > On Fri, Jan 17, 2025 at 8:25 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Replaces SAFE_CLONE() with tst_clone() in the tst_ns_* helpers. > > The reason for the replacement is that SAFE_CLONE() uses > > TST_RETRY_FUNC() which calls tst_multiply_timeout(). The problem with > > that is that the tst_multiply_timeout() is a test library function that > > started to print TINFO messages recently and that we rely on parsing the > > output from the tst_ns_* helpers. > > The reason SAFE_CLONE() started to call TST_RETRY_FUNC() is that in the > > case that we create new namespaces with the clone call, we may end up > > creating them faster than kernel can clean them up which is described in: > > commit 7d882081a5613f44a12fc6b1c44267d4df0857a4 > > Author: Petr Vorel <pvorel@suse.cz> > > Date: Mon Mar 28 22:46:43 2022 +0200 > > lib: Retry safe_clone() on ENOSPC > > This combined with the newly introduced changes in the test library that > > check for kernel debugging options that may need to adjust default > > timeouts: > > commit 893ca0abe7e82851ff0e5d93c09b1098f2eff121 > > Author: Li Wang <liwang@redhat.com> > > Date: Sun Dec 22 15:22:49 2024 +0800 > > lib: multiply the timeout if detect slow kconfigs > > which adds tst_has_slow_kconfig() into the tst_multiply_timeout() causes > > the TINFO messages to be printed. > > The reason why we can safely replace the SAFE_CLONE() with tst_clone() > > here is that we are not creating new namspaces in the tst_ns_* helpers, > > but rather than that cloning a new process to be executed inside of the > > namespace, hence we do not need to retry on ENOSPC. > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > Reviewed-by: Li Wang <liwang@redhat.com> > Nice work! Thanks for fix and review. This one fixes the problem, thus I merged it. I'll let you know about the other patch soon (I suspect that it does not catch other usage, some tools needs to parse stderr ...). Kind regards, Petr
diff --git a/testcases/lib/tst_ns_create.c b/testcases/lib/tst_ns_create.c index ce3707a60..bd7633b0b 100644 --- a/testcases/lib/tst_ns_create.c +++ b/testcases/lib/tst_ns_create.c @@ -23,12 +23,6 @@ #include "tst_test.h" #include "tst_ns_common.h" -extern struct tst_test *tst_test; - -static struct tst_test test = { - .forks_child = 1, /* Needed by SAFE_CLONE */ -}; - static void print_help(void) { int i; @@ -66,8 +60,6 @@ int main(int argc, char *argv[]) return 1; } - tst_test = &test; - while ((token = strsep(&argv[1], ","))) { struct param *p = get_param(token); @@ -80,7 +72,12 @@ int main(int argc, char *argv[]) args.flags |= p->flag; } - pid = SAFE_CLONE(&args); + pid = tst_clone(&args); + if (pid < 0) { + printf("clone() failed"); + return 1; + } + if (!pid) { child_fn(); return 0; diff --git a/testcases/lib/tst_ns_exec.c b/testcases/lib/tst_ns_exec.c index 6a8e39339..5d34e9ad5 100644 --- a/testcases/lib/tst_ns_exec.c +++ b/testcases/lib/tst_ns_exec.c @@ -20,12 +20,6 @@ #include "tst_test.h" #include "tst_ns_common.h" -extern struct tst_test *tst_test; - -static struct tst_test test = { - .forks_child = 1, /* Needed by SAFE_CLONE */ -}; - static int ns_fd[NS_TOTAL]; static int ns_fds; @@ -71,8 +65,6 @@ int main(int argc, char *argv[]) int i, status, pid; char *token; - tst_test = &test; - if (argc < 4) { print_help(); return 1; @@ -100,7 +92,12 @@ int main(int argc, char *argv[]) for (i = 0; i < ns_fds; i++) SAFE_SETNS(ns_fd[i], 0); - pid = SAFE_CLONE(&args); + pid = tst_clone(&args); + if (pid < 0) { + printf("clone() failed"); + return 1; + } + if (!pid) SAFE_EXECVP(argv[3], argv+3);
Replaces SAFE_CLONE() with tst_clone() in the tst_ns_* helpers. The reason for the replacement is that SAFE_CLONE() uses TST_RETRY_FUNC() which calls tst_multiply_timeout(). The problem with that is that the tst_multiply_timeout() is a test library function that started to print TINFO messages recently and that we rely on parsing the output from the tst_ns_* helpers. The reason SAFE_CLONE() started to call TST_RETRY_FUNC() is that in the case that we create new namespaces with the clone call, we may end up creating them faster than kernel can clean them up which is described in: commit 7d882081a5613f44a12fc6b1c44267d4df0857a4 Author: Petr Vorel <pvorel@suse.cz> Date: Mon Mar 28 22:46:43 2022 +0200 lib: Retry safe_clone() on ENOSPC This combined with the newly introduced changes in the test library that check for kernel debugging options that may need to adjust default timeouts: commit 893ca0abe7e82851ff0e5d93c09b1098f2eff121 Author: Li Wang <liwang@redhat.com> Date: Sun Dec 22 15:22:49 2024 +0800 lib: multiply the timeout if detect slow kconfigs which adds tst_has_slow_kconfig() into the tst_multiply_timeout() causes the TINFO messages to be printed. The reason why we can safely replace the SAFE_CLONE() with tst_clone() here is that we are not creating new namspaces in the tst_ns_* helpers, but rather than that cloning a new process to be executed inside of the namespace, hence we do not need to retry on ENOSPC. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- testcases/lib/tst_ns_create.c | 15 ++++++--------- testcases/lib/tst_ns_exec.c | 15 ++++++--------- 2 files changed, 12 insertions(+), 18 deletions(-)