On Wed, Jun 12, 2024, Muhammad Usama Anjum wrote: > The KSFT_FAIL, exit code must be used instead of exit(254).
This needs more justification. KVM selftests have worked just fine for 6+ years using exit(254), so stating they "must" use KSFT_FAIL is obviously not true. I'm not personally opposed to switching to KSFT_FAIL, but it is a potentially breaking change. E.g. some of Google's internal test infrastructure explicitly relies on the exit code being 254. I don't _think_ that infrastructure interacts with KVM selftests, nor do I think that forcing upstream KVM selftests to sacrifice TAP compliance just to play nice with someone's crusty test infrastructure is a good tradeoff, but this and all of the TAP compliance work needs to be done with more thought and care. > The 254 code here seems like anciant relic. As above, AFAICT it comes from Google's internal test infrastructure (KVM selftests came from Google). > Its even better if we use ksft_exit_fail_msg() which will print out "Bail > out" meaning the test exited without completing. This string is TAP protocol > specific. This is debatable and not obviously correct. The documentation says: Bail out! As an emergency measure a test script can decide that further tests are useless (e.g. missing dependencies) and testing should stop immediately. In that case the test script prints the magic words which suggests that a test should only emit "Bail out!" if it wants to stop entirely. We definitely don't want KVM selftests to bail out if a TEST_ASSERT() fails in one testcase. > Signed-off-by: Muhammad Usama Anjum <[email protected]> > --- > tools/testing/selftests/kvm/lib/assert.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/kvm/lib/assert.c > b/tools/testing/selftests/kvm/lib/assert.c > index 33651f5b3a7fd..db648a7ac429b 100644 > --- a/tools/testing/selftests/kvm/lib/assert.c > +++ b/tools/testing/selftests/kvm/lib/assert.c > @@ -87,7 +87,7 @@ test_assert(bool exp, const char *exp_str, > > if (errno == EACCES) > ksft_exit_skip("Access denied - Exiting\n"); > - exit(254); > + ksft_exit_fail_msg("\n"); > } > > return; > -- > 2.39.2 >
