| Message ID | 20200626210917.358969-11-brendanhiggins@google.com (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | kunit: create a centralized executor to dispatch all KUnit tests | expand |
On Fri, Jun 26, 2020 at 02:09:15PM -0700, Brendan Higgins wrote: > From: David Gow <davidgow@google.com> > > Add a new kernel command-line option, 'kunit_shutdown', which allows the > user to specify that the kernel poweroff, halt, or reboot after > completing all KUnit tests; this is very handy for running KUnit tests > on UML or a VM so that the UML/VM process exits cleanly immediately > after running all tests without needing a special initramfs. > > Signed-off-by: David Gow <davidgow@google.com> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > --- > lib/kunit/executor.c | 20 ++++++++++++++++++++ > tools/testing/kunit/kunit_kernel.py | 2 +- > tools/testing/kunit/kunit_parser.py | 2 +- > 3 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c > index a95742a4ece73..38061d456afb2 100644 > --- a/lib/kunit/executor.c > +++ b/lib/kunit/executor.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > > +#include <linux/reboot.h> > #include <kunit/test.h> > > /* > @@ -11,6 +12,23 @@ extern struct kunit_suite * const * const __kunit_suites_end[]; > > #if IS_BUILTIN(CONFIG_KUNIT) > > +static char *kunit_shutdown; > +core_param(kunit_shutdown, kunit_shutdown, charp, 0644); > + > +static void kunit_handle_shutdown(void) > +{ > + if (!kunit_shutdown) > + return; > + > + if (!strcmp(kunit_shutdown, "poweroff")) > + kernel_power_off(); > + else if (!strcmp(kunit_shutdown, "halt")) > + kernel_halt(); > + else if (!strcmp(kunit_shutdown, "reboot")) > + kernel_restart(NULL); > + > +} If you have patches that do something just before the initrd, and then you add more patches to shut down immediately after an initrd, people may ask you to just use an initrd instead of filling the kernel with these changes... I mean, I get it, but it's not hard to make an initrd that poke a sysctl to start the tests... In fact, you don't even need a initrd to poke sysctls these days.
On Fri, Jun 26, 2020 at 2:40 PM Kees Cook <keescook@chromium.org> wrote: > > On Fri, Jun 26, 2020 at 02:09:15PM -0700, Brendan Higgins wrote: > > From: David Gow <davidgow@google.com> > > > > Add a new kernel command-line option, 'kunit_shutdown', which allows the > > user to specify that the kernel poweroff, halt, or reboot after > > completing all KUnit tests; this is very handy for running KUnit tests > > on UML or a VM so that the UML/VM process exits cleanly immediately > > after running all tests without needing a special initramfs. > > > > Signed-off-by: David Gow <davidgow@google.com> > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > > --- > > lib/kunit/executor.c | 20 ++++++++++++++++++++ > > tools/testing/kunit/kunit_kernel.py | 2 +- > > tools/testing/kunit/kunit_parser.py | 2 +- > > 3 files changed, 22 insertions(+), 2 deletions(-) > > > > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c > > index a95742a4ece73..38061d456afb2 100644 > > --- a/lib/kunit/executor.c > > +++ b/lib/kunit/executor.c > > @@ -1,5 +1,6 @@ > > // SPDX-License-Identifier: GPL-2.0 > > > > +#include <linux/reboot.h> > > #include <kunit/test.h> > > > > /* > > @@ -11,6 +12,23 @@ extern struct kunit_suite * const * const __kunit_suites_end[]; > > > > #if IS_BUILTIN(CONFIG_KUNIT) > > > > +static char *kunit_shutdown; > > +core_param(kunit_shutdown, kunit_shutdown, charp, 0644); > > + > > +static void kunit_handle_shutdown(void) > > +{ > > + if (!kunit_shutdown) > > + return; > > + > > + if (!strcmp(kunit_shutdown, "poweroff")) > > + kernel_power_off(); > > + else if (!strcmp(kunit_shutdown, "halt")) > > + kernel_halt(); > > + else if (!strcmp(kunit_shutdown, "reboot")) > > + kernel_restart(NULL); > > + > > +} > > If you have patches that do something just before the initrd, and then > you add more patches to shut down immediately after an initrd, people > may ask you to just use an initrd instead of filling the kernel with > these changes... > > I mean, I get it, but it's not hard to make an initrd that poke a sysctl > to start the tests... > > In fact, you don't even need a initrd to poke sysctls these days. True, but it is nice to not have to maintain an initramfs for each architecture that you want to test. Still, I see your point. If we can find a convenient way to distribute the needed dependencies for running KUnit on each non-UML architecture then I think I can abandon this change. So how about this: I will drop this patch from this patchset and move it up to the follow up patchset that adds multiarchitecture support to kunit_tool. There we can address the problem of how to best track the necessary dependencies including possibly intitramfss.
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index a95742a4ece73..38061d456afb2 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 +#include <linux/reboot.h> #include <kunit/test.h> /* @@ -11,6 +12,23 @@ extern struct kunit_suite * const * const __kunit_suites_end[]; #if IS_BUILTIN(CONFIG_KUNIT) +static char *kunit_shutdown; +core_param(kunit_shutdown, kunit_shutdown, charp, 0644); + +static void kunit_handle_shutdown(void) +{ + if (!kunit_shutdown) + return; + + if (!strcmp(kunit_shutdown, "poweroff")) + kernel_power_off(); + else if (!strcmp(kunit_shutdown, "halt")) + kernel_halt(); + else if (!strcmp(kunit_shutdown, "reboot")) + kernel_restart(NULL); + +} + static void kunit_print_tap_header(void) { struct kunit_suite * const * const *suites, * const *subsuite; @@ -37,6 +55,8 @@ int kunit_run_all_tests(void) suites++) __kunit_test_suites_init(*suites); + kunit_handle_shutdown(); + return 0; } diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 63dbda2d029f6..d6a575f92317c 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -172,7 +172,7 @@ class LinuxSourceTree(object): return self.validate_config(build_dir) def run_kernel(self, args=[], build_dir='', timeout=None): - args.extend(['mem=1G']) + args.extend(['mem=1G', 'kunit_shutdown=halt']) outfile = 'test.log' self._ops.linux_bin(args, timeout, build_dir, outfile) subprocess.call(['stty', 'sane']) diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 6d6d94a0ee7db..a8998a5effaad 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -49,7 +49,7 @@ class TestStatus(Enum): kunit_start_re = re.compile(r'TAP version [0-9]+$') kunit_end_re = re.compile('(List of all partitions:|' - 'Kernel panic - not syncing: VFS:)') + 'Kernel panic - not syncing: VFS:|reboot: System halted)') def isolate_kunit_output(kernel_output): started = False