Commit 3f0a50f3 authored by Daniel Latypov's avatar Daniel Latypov Committed by Shuah Khan
Browse files

kunit: tool: stop using a shell to run kernel under QEMU 



Note: this potentially breaks custom qemu_configs if people are using
them! But the fix for them is simple, don't specify multiple arguments
in one string and don't add on a redundant ''.

It feels a bit iffy to be using a shell in the first place.

There's the usual shenanigans where people could pass in arbitrary shell
commands via --kernel_arg (since we're just adding '' around the
kernel_cmdline) or via a custom qemu_config.
This isn't too much of a concern given the nature of this script (and
the qemu_config file is in python, you can do w/e you want already).

But it does have some other drawbacks.

One example of a kunit-specific pain point:
If the relevant qemu binary is missing, we get output like this:
> /bin/sh: line 1: qemu-system-aarch64: command not found
This in turn results in our KTAP parser complaining about
missing/invalid KTAP, but we don't directly show the error!
It's even more annoying to debug when you consider --raw_output only
shows KUnit output by default, i.e. you need --raw_output=all to see it.

Whereas directly invoking the binary, Python will raise a
FileNotFoundError for us, which is a noisier but more clear.

Making this change requires
* splitting parameters like ['-m 256'] into ['-m', '256'] in
  kunit/qemu_configs/*.py
* change [''] to [] in kunit/qemu_configs/*.py since otherwise
  QEMU fails w/ 'Device needs media, but drive is empty'
* dropping explicit quoting of the kernel cmdline
* using shlex.quote() when we print what command we're running
  so the user can copy-paste and run it

Signed-off-by: default avatarDaniel Latypov <dlatypov@google.com>
Reviewed-by: default avatarBrendan Higgins <brendanhiggins@google.com>
Signed-off-by: default avatarShuah Khan <skhan@linuxfoundation.org>
parent c2497643

tools/testing/kunit/kunit_kernel.py

+10 −8
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ import importlib.util 
import logging

import subprocess

import os

import shlex

import shutil

import signal

import threading
@@ -118,16 +119,17 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): 
				'-nodefaults',

				'-m', '1024',

				'-kernel', kernel_path,

				'-append', '\'' + ' '.join(params + [self._kernel_command_line]) + '\'',

				'-append', ' '.join(params + [self._kernel_command_line]),

				'-no-reboot',

				'-nographic',

				'-serial stdio'] + self._extra_qemu_params

		print('Running tests with:\n$', ' '.join(qemu_command))

		return subprocess.Popen(' '.join(qemu_command),

				'-serial', 'stdio'] + self._extra_qemu_params

		# Note: shlex.join() does what we want, but requires python 3.8+.

		print('Running tests with:\n$', ' '.join(shlex.quote(arg) for arg in qemu_command))

		return subprocess.Popen(qemu_command,

					stdin=subprocess.PIPE,

					stdout=subprocess.PIPE,

					stderr=subprocess.STDOUT,

					   text=True, shell=True, errors='backslashreplace')

					text=True, errors='backslashreplace')



class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):

	"""An abstraction over command line operations performed on a source tree."""

tools/testing/kunit/qemu_configs/alpha.py

+1 −1
Original line number Diff line number Diff line
@@ -7,4 +7,4 @@ CONFIG_SERIAL_8250_CONSOLE=y''', 
			   qemu_arch='alpha',

			   kernel_path='arch/alpha/boot/vmlinux',

			   kernel_command_line='console=ttyS0',

			   extra_qemu_params=[''])
 
			   extra_qemu_params=[])

tools/testing/kunit/qemu_configs/arm.py

+1 −1
Original line number Diff line number Diff line
@@ -10,4 +10,4 @@ CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''', 
			   qemu_arch='arm',

			   kernel_path='arch/arm/boot/zImage',

			   kernel_command_line='console=ttyAMA0',

			   extra_qemu_params=['-machine virt'])
 
			   extra_qemu_params=['-machine', 'virt'])

tools/testing/kunit/qemu_configs/arm64.py

+1 −1
Original line number Diff line number Diff line
@@ -9,4 +9,4 @@ CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''', 
			   qemu_arch='aarch64',

			   kernel_path='arch/arm64/boot/Image.gz',

			   kernel_command_line='console=ttyAMA0',

			   extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
 
			   extra_qemu_params=['-machine', 'virt', '-cpu', 'cortex-a57'])

tools/testing/kunit/qemu_configs/i386.py

+1 −1
Original line number Diff line number Diff line
@@ -7,4 +7,4 @@ CONFIG_SERIAL_8250_CONSOLE=y''', 
			   qemu_arch='x86_64',

			   kernel_path='arch/x86/boot/bzImage',

			   kernel_command_line='console=ttyS0',

			   extra_qemu_params=[''])
 
			   extra_qemu_params=[])