diff mbox series

[v5,10/12] kunit: Add 'kunit_shutdown' option

Message ID 20200626210917.358969-11-brendanhiggins@google.com
State New
Headers show
Series kunit: create a centralized executor to dispatch all KUnit tests | expand

Commit Message

Brendan Higgins June 26, 2020, 9:09 p.m. UTC
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(-)

Comments

Brendan Higgins Aug. 4, 2020, 8:18 p.m. UTC | #1
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 mbox series

Patch

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