Calling ksft_exit_* results in executing fewer tests than planned, which is wrong for ksft_exit_skip or suboptimal (because it results in a bail out) for ksft_exit_fail_msg.
Using ksft_test_result_skip instead skips only one test and lets the test plan proceed as promised by ksft_set_plan.
Paolo
Paolo Bonzini (2): selftests: pidfd: do not use ksft_exit_skip after ksft_set_plan selftests: pidfd: skip test if unshare fails with EPERM
tools/testing/selftests/pidfd/pidfd_test.c | 55 ++++++++++++++++++---- 1 file changed, 46 insertions(+), 9 deletions(-)
Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip instead.
The plan passed to ksft_set_plan was wrong, too, so fix it while at it.
Signed-off-by: Paolo Bonzini pbonzini@redhat.com Message-Id: 20200623001547.22255-5-pbonzini@redhat.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- tools/testing/selftests/pidfd/pidfd_test.c | 34 +++++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 7aff2d3b42c0..f65ad4e32353 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -8,6 +8,7 @@ #include <sched.h> #include <signal.h> #include <stdio.h> +#include <stdbool.h> #include <stdlib.h> #include <string.h> #include <syscall.h> @@ -27,6 +28,8 @@
#define MAX_EVENTS 5
+static bool have_pidfd_send_signal = false; + static pid_t pidfd_clone(int flags, int *pidfd, int (*fn)(void *)) { size_t stack_size = 1024; @@ -56,6 +59,13 @@ static int test_pidfd_send_signal_simple_success(void) int pidfd, ret; const char *test_name = "pidfd_send_signal send SIGUSR1";
+ if (!have_pidfd_send_signal) { + ksft_test_result_skip( + "%s test: pidfd_send_signal() syscall not supported\n", + test_name); + return 0; + } + pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC); if (pidfd < 0) ksft_exit_fail_msg( @@ -86,6 +96,13 @@ static int test_pidfd_send_signal_exited_fail(void) pid_t pid; const char *test_name = "pidfd_send_signal signal exited process";
+ if (!have_pidfd_send_signal) { + ksft_test_result_skip( + "%s test: pidfd_send_signal() syscall not supported\n", + test_name); + return 0; + } + pid = fork(); if (pid < 0) ksft_exit_fail_msg("%s test: Failed to create new process\n", @@ -137,6 +154,13 @@ static int test_pidfd_send_signal_recycled_pid_fail(void) pid_t pid1; const char *test_name = "pidfd_send_signal signal recycled pid";
+ if (!have_pidfd_send_signal) { + ksft_test_result_skip( + "%s test: pidfd_send_signal() syscall not supported\n", + test_name); + return 0; + } + ret = unshare(CLONE_NEWPID); if (ret < 0) ksft_exit_fail_msg("%s test: Failed to unshare pid namespace\n", @@ -325,15 +349,17 @@ static int test_pidfd_send_signal_syscall_support(void)
ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0); if (ret < 0) { - if (errno == ENOSYS) - ksft_exit_skip( + if (errno == ENOSYS) { + ksft_test_result_skip( "%s test: pidfd_send_signal() syscall not supported\n", test_name); - + return 0; + } ksft_exit_fail_msg("%s test: Failed to send signal\n", test_name); }
+ have_pidfd_send_signal = true; close(pidfd); ksft_test_result_pass( "%s test: pidfd_send_signal() syscall is supported. Tests can be executed\n", @@ -521,7 +547,7 @@ static void test_pidfd_poll_leader_exit(int use_waitpid) int main(int argc, char **argv) { ksft_print_header(); - ksft_set_plan(4); + ksft_set_plan(8);
test_pidfd_poll_exec(0); test_pidfd_poll_exec(1);
On 7/7/20 4:19 AM, Paolo Bonzini wrote:
Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip instead.
The plan passed to ksft_set_plan was wrong, too, so fix it while at it.
Signed-off-by: Paolo Bonzini pbonzini@redhat.com Message-Id: 20200623001547.22255-5-pbonzini@redhat.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com
tools/testing/selftests/pidfd/pidfd_test.c | 34 +++++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 7aff2d3b42c0..f65ad4e32353 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -8,6 +8,7 @@ #include <sched.h> #include <signal.h> #include <stdio.h> +#include <stdbool.h> #include <stdlib.h> #include <string.h> #include <syscall.h> @@ -27,6 +28,8 @@ #define MAX_EVENTS 5 +static bool have_pidfd_send_signal = false;
You don't need to initialize this to false. Rest looks good.
- static pid_t pidfd_clone(int flags, int *pidfd, int (*fn)(void *)) { size_t stack_size = 1024;
@@ -56,6 +59,13 @@ static int test_pidfd_send_signal_simple_success(void) int pidfd, ret; const char *test_name = "pidfd_send_signal send SIGUSR1";
- if (!have_pidfd_send_signal) {
ksft_test_result_skip(
"%s test: pidfd_send_signal() syscall not supported\n",
test_name);
return 0;
- }
- pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC); if (pidfd < 0) ksft_exit_fail_msg(
@@ -86,6 +96,13 @@ static int test_pidfd_send_signal_exited_fail(void) pid_t pid; const char *test_name = "pidfd_send_signal signal exited process";
- if (!have_pidfd_send_signal) {
ksft_test_result_skip(
"%s test: pidfd_send_signal() syscall not supported\n",
test_name);
return 0;
- }
- pid = fork(); if (pid < 0) ksft_exit_fail_msg("%s test: Failed to create new process\n",
@@ -137,6 +154,13 @@ static int test_pidfd_send_signal_recycled_pid_fail(void) pid_t pid1; const char *test_name = "pidfd_send_signal signal recycled pid";
- if (!have_pidfd_send_signal) {
ksft_test_result_skip(
"%s test: pidfd_send_signal() syscall not supported\n",
test_name);
return 0;
- }
- ret = unshare(CLONE_NEWPID); if (ret < 0) ksft_exit_fail_msg("%s test: Failed to unshare pid namespace\n",
@@ -325,15 +349,17 @@ static int test_pidfd_send_signal_syscall_support(void) ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0); if (ret < 0) {
if (errno == ENOSYS)
ksft_exit_skip(
if (errno == ENOSYS) {
ksft_test_result_skip( "%s test: pidfd_send_signal() syscall not supported\n", test_name);
return 0;
ksft_exit_fail_msg("%s test: Failed to send signal\n", test_name); }}
- have_pidfd_send_signal = true; close(pidfd); ksft_test_result_pass( "%s test: pidfd_send_signal() syscall is supported. Tests can be executed\n",
@@ -521,7 +547,7 @@ static void test_pidfd_poll_leader_exit(int use_waitpid) int main(int argc, char **argv) { ksft_print_header();
- ksft_set_plan(4);
- ksft_set_plan(8);
test_pidfd_poll_exec(0); test_pidfd_poll_exec(1);
thanks, -- Shuah
Similar to how ENOSYS causes a skip if pidfd_send_signal is not present, we can do the same for unshare if it fails with EPERM. This way, running the test without privileges causes four tests to skip but no early bail out.
Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- tools/testing/selftests/pidfd/pidfd_test.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index f65ad4e32353..dcc86e8f7a9f 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -162,15 +162,26 @@ static int test_pidfd_send_signal_recycled_pid_fail(void) }
ret = unshare(CLONE_NEWPID); - if (ret < 0) + if (ret < 0) { + if (errno == EPERM) { + ksft_test_result_skip("%s test: Unsharing pid namespace not permitted\n", + test_name); + return 0; + } ksft_exit_fail_msg("%s test: Failed to unshare pid namespace\n", test_name); + }
ret = unshare(CLONE_NEWNS); - if (ret < 0) - ksft_exit_fail_msg( - "%s test: Failed to unshare mount namespace\n", - test_name); + if (ret < 0) { + if (errno == EPERM) { + ksft_test_result_skip("%s test: Unsharing mount namespace not permitted\n", + test_name); + return 0; + } + ksft_exit_fail_msg("%s test: Failed to unshare mount namespace\n", + test_name); + }
ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0); if (ret < 0)
On Tue, Jul 07, 2020 at 06:19:34AM -0400, Paolo Bonzini wrote:
Calling ksft_exit_* results in executing fewer tests than planned, which is wrong for ksft_exit_skip or suboptimal (because it results in a bail out) for ksft_exit_fail_msg.
Using ksft_test_result_skip instead skips only one test and lets the test plan proceed as promised by ksft_set_plan.
Paolo
Thanks for fixing this, Paolo! Acked-by: Christian Brauner christian.brauner@ubuntu.com
Shuah, want me to take it or do you want to take it?
On 7/7/20 7:52 AM, Christian Brauner wrote:
On Tue, Jul 07, 2020 at 06:19:34AM -0400, Paolo Bonzini wrote:
Calling ksft_exit_* results in executing fewer tests than planned, which is wrong for ksft_exit_skip or suboptimal (because it results in a bail out) for ksft_exit_fail_msg.
Using ksft_test_result_skip instead skips only one test and lets the test plan proceed as promised by ksft_set_plan.
Paolo
Thanks for fixing this, Paolo! Acked-by: Christian Brauner christian.brauner@ubuntu.com
Shuah, want me to take it or do you want to take it?
I will apply it to my tree with Paolo's other patches in this series.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org