| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFd5g44OL0BQ74ZGEnYzsLYVXEbOB-hBiNAf1+VLO=DoZwKOvA@mail.gmail.com>
Date: Fri, 30 Apr 2021 13:01:38 -0700
From: Brendan Higgins <brendanhiggins@...gle.com>
To: Daniel Latypov <dlatypov@...gle.com>
Cc: shuah <shuah@...nel.org>, David Gow <davidgow@...gle.com>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>,
KUnit Development <kunit-dev@...glegroups.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Jonathan Corbet <corbet@....net>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
Stephen Boyd <sboyd@...nel.org>,
Kees Cook <keescook@...omium.org>,
Frank Rowand <frowand.list@...il.com>
Subject: Re: [RFC v2 3/4] kunit: tool: add support for QEMU
On Thu, Apr 29, 2021 at 4:40 PM Daniel Latypov <dlatypov@...gle.com> wrote:
>
> On Thu, Apr 29, 2021 at 1:51 PM Brendan Higgins
> <brendanhiggins@...gle.com> wrote:
> >
> > Add basic support to run QEMU via kunit_tool. Add support for i386,
> > x86_64, arm, arm64, and a bunch more.
>
> Hmmm, I'm wondering if I'm seeing unrelated breakages.
> Applied these patches on top of 55ba0fe059a5 ("Merge tag
> 'for-5.13-tag' of
> git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux")
>
> $ make mrproper
> $ rm -rf .kunit/* # just in case
> $ ./tools/testing/kunit/kunit.py run --arch=arm64
> ...
Huh, did you use a arm64 cross compiler? Maybe that's your issue.
> ERROR:root:arch/arm64/Makefile:25: ld does not support
> --fix-cortex-a53-843419; kernel may be susceptible to erratum
> arch/arm64/Makefile:44: Detected assembler with broken .inst;
> disassembly will be unreliable
> gcc: error: unrecognized command-line option ‘-mlittle-endian’
>
> So it seems like my version of ld might be out of date, but my gcc is 10.2.1
> Is there a minimum version of the toolchain this would expect that we
> can call out in the commit message (and in the Documentation)?
>
> --arch=x86_64 worked just fine for me, however, which is mainly what I
> was interested in.
>
> I've mainly just left some nits and comments regarding typechecking.
>
> >
> > Signed-off-by: Brendan Higgins <brendanhiggins@...gle.com>
> > ---
> > tools/testing/kunit/kunit.py | 33 +++-
> > tools/testing/kunit/kunit_config.py | 2 +-
> > tools/testing/kunit/kunit_kernel.py | 207 +++++++++++++++++++++----
> > tools/testing/kunit/kunit_tool_test.py | 15 +-
> > 4 files changed, 217 insertions(+), 40 deletions(-)
> >
> > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> > index d5144fcb03acd..07ef80062873b 100755
> > --- a/tools/testing/kunit/kunit.py
> > +++ b/tools/testing/kunit/kunit.py
> > @@ -70,10 +70,10 @@ def build_tests(linux: kunit_kernel.LinuxSourceTree,
> > kunit_parser.print_with_timestamp('Building KUnit Kernel ...')
> >
> > build_start = time.time()
> > - success = linux.build_um_kernel(request.alltests,
> > - request.jobs,
> > - request.build_dir,
> > - request.make_options)
> > + success = linux.build_kernel(request.alltests,
> > + request.jobs,
> > + request.build_dir,
> > + request.make_options)
> > build_end = time.time()
> > if not success:
> > return KunitResult(KunitStatus.BUILD_FAILURE,
> > @@ -187,6 +187,14 @@ def add_common_opts(parser) -> None:
> > help='Path to Kconfig fragment that enables KUnit tests',
> > metavar='kunitconfig')
> >
> > + parser.add_argument('--arch',
> > + help='Specifies the architecture to run tests under.',
>
> optional: perhaps add "(e.g. x86_64, um)"
> Most people using this would be able to infer that, but I prefer
> strings that are basically enums to document examples of useful
> values.
Good point, probably want to mention that I take the string name from
the argument passed into the ARCH make variable.
> (And x86 is probably the one most people want to use this flag for).
Yep, but Linux refers to it as i386. Just trying to be consistent.
> > + type=str, default='um', metavar='arch')
> > +
> > + parser.add_argument('--cross_compile',
> > + help='Sets make\'s CROSS_COMPILE variable.',
> > + metavar='cross_compile')
> > +
> > def add_build_opts(parser) -> None:
> > parser.add_argument('--jobs',
> > help='As in the make command, "Specifies the number of '
> > @@ -268,7 +276,10 @@ def main(argv, linux=None):
> > os.mkdir(cli_args.build_dir)
> >
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch,
> > + cross_compile=cli_args.cross_compile)
> >
> > request = KunitRequest(cli_args.raw_output,
> > cli_args.timeout,
> > @@ -287,7 +298,9 @@ def main(argv, linux=None):
> > os.mkdir(cli_args.build_dir)
> >
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch)
> >
> > request = KunitConfigRequest(cli_args.build_dir,
> > cli_args.make_options)
> > @@ -299,7 +312,9 @@ def main(argv, linux=None):
> > sys.exit(1)
> > elif cli_args.subcommand == 'build':
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch)
> >
> > request = KunitBuildRequest(cli_args.jobs,
> > cli_args.build_dir,
> > @@ -313,7 +328,9 @@ def main(argv, linux=None):
> > sys.exit(1)
> > elif cli_args.subcommand == 'exec':
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch)
> >
> > exec_request = KunitExecRequest(cli_args.timeout,
> > cli_args.build_dir,
> > diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
> > index 1e2683dcc0e7a..07d76be392544 100644
> > --- a/tools/testing/kunit/kunit_config.py
> > +++ b/tools/testing/kunit/kunit_config.py
> > @@ -53,7 +53,7 @@ class Kconfig(object):
> > return True
> >
> > def write_to_file(self, path: str) -> None:
> > - with open(path, 'w') as f:
> > + with open(path, 'a+') as f:
>
> I might be missing something, but do we need this?
>
> w => a means we wouldn't truncate the file if it exists. I can imagine
> I'm missing something that makes it necessary.
> + => opens for read/write: we don't do any reads with f afaict.
Yeah, probably only needs to be a, not a+. But we do want append for
adding the arch configs. Idea is that we want to append those onto
whatever a base kunitconfig.
Nevertheless, your concern is valid, might be a better way to do this. Thoughts?
> > for entry in self.entries():
> > f.write(str(entry) + '\n')
> >
> > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > index 7d5b77967d48f..b8b3b76aaa17e 100644
> > --- a/tools/testing/kunit/kunit_kernel.py
> > +++ b/tools/testing/kunit/kunit_kernel.py
> > @@ -15,6 +15,8 @@ from typing import Iterator
> >
> > from contextlib import ExitStack
> >
> > +from collections import namedtuple
> > +
> > import kunit_config
> > import kunit_parser
> >
> > @@ -40,6 +42,10 @@ class BuildError(Exception):
> > class LinuxSourceTreeOperations(object):
> > """An abstraction over command line operations performed on a source tree."""
> >
> > + def __init__(self, linux_arch, cross_compile):
>
> nit: let's annotate these as `linux_arch: str, cross_compile: str` (or
> is it Optional[str] here?)
> I can see a reader thinking arch might be an enum and that
> cross_compile might be a bool.
Fair. Will do.
> > + self._linux_arch = linux_arch
> > + self._cross_compile = cross_compile
> > +
> > def make_mrproper(self) -> None:
> > try:
> > subprocess.check_output(['make', 'mrproper'], stderr=subprocess.STDOUT)
> > @@ -48,12 +54,18 @@ class LinuxSourceTreeOperations(object):
> > except subprocess.CalledProcessError as e:
> > raise ConfigError(e.output.decode())
> >
> > + def make_arch_qemuconfig(self, build_dir):
> > + pass
> > +
> > def make_olddefconfig(self, build_dir, make_options) -> None:
> > - command = ['make', 'ARCH=um', 'olddefconfig']
> > + command = ['make', 'ARCH=' + self._linux_arch, 'olddefconfig']
> > + if self._cross_compile:
> > + command += ['CROSS_COMPILE=' + self._cross_compile]
> > if make_options:
> > command.extend(make_options)
> > if build_dir:
> > command += ['O=' + build_dir]
> > + print(' '.join(command))
> > try:
> > subprocess.check_output(command, stderr=subprocess.STDOUT)
> > except OSError as e:
> > @@ -61,6 +73,154 @@ class LinuxSourceTreeOperations(object):
> > except subprocess.CalledProcessError as e:
> > raise ConfigError(e.output.decode())
> >
> > + def make(self, jobs, build_dir, make_options) -> None:
> > + command = ['make', 'ARCH=' + self._linux_arch, '--jobs=' + str(jobs)]
> > + if make_options:
> > + command.extend(make_options)
> > + if self._cross_compile:
> > + command += ['CROSS_COMPILE=' + self._cross_compile]
> > + if build_dir:
> > + command += ['O=' + build_dir]
> > + print(' '.join(command))
> > + try:
> > + proc = subprocess.Popen(command,
> > + stderr=subprocess.PIPE,
> > + stdout=subprocess.DEVNULL)
> > + except OSError as e:
> > + raise BuildError('Could not call execute make: ' + e)
>
> pytype complains about this.
> You'd want `+ str(e)`
Got it. Will do. (I should probably also run pytype.)
> > + except subprocess.CalledProcessError as e:
> > + raise BuildError(e.output)
> > + _, stderr = proc.communicate()
> > + if proc.returncode != 0:
> > + raise BuildError(stderr.decode())
> > + if stderr: # likely only due to build warnings
> > + print(stderr.decode())
> > +
> > + def run(self, params, timeout, build_dir, outfile) -> None:
> > + pass
> > +
> > +
> > +QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
> > + 'qemuconfig',
> > + 'qemu_arch',
> > + 'kernel_path',
> > + 'kernel_command_line',
> > + 'extra_qemu_params'])
> > +
> > +
> > +QEMU_ARCHS = {
> > + 'i386' : QemuArchParams(linux_arch='i386',
> > + qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
> > + qemu_arch='x86_64',
> > + kernel_path='arch/x86/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=['']),
> > + 'x86_64' : QemuArchParams(linux_arch='x86_64',
> > + qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
> > + qemu_arch='x86_64',
> > + kernel_path='arch/x86/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=['']),
> > + 'arm' : QemuArchParams(linux_arch='arm',
> > + qemuconfig='''CONFIG_ARCH_VIRT=y
> > +CONFIG_SERIAL_AMBA_PL010=y
> > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > +CONFIG_SERIAL_AMBA_PL011=y
> > +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']),
> > + 'arm64' : QemuArchParams(linux_arch='arm64',
> > + qemuconfig='''CONFIG_SERIAL_AMBA_PL010=y
> > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > +CONFIG_SERIAL_AMBA_PL011=y
> > +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']),
> > + 'alpha' : QemuArchParams(linux_arch='alpha',
> > + qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
> > + qemu_arch='alpha',
> > + kernel_path='arch/alpha/boot/vmlinux',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=['']),
> > + 'powerpc' : QemuArchParams(linux_arch='powerpc',
> > + qemuconfig='CONFIG_PPC64=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_HVC_CONSOLE=y',
> > + qemu_arch='ppc64',
> > + kernel_path='vmlinux',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=['-M pseries', '-cpu power8']),
> > + 'riscv' : QemuArchParams(linux_arch='riscv',
> > + qemuconfig='CONFIG_SOC_VIRT=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_SERIAL_OF_PLATFORM=y\nCONFIG_SERIAL_EARLYCON_RISCV_SBI=y',
> > + qemu_arch='riscv64',
> > + kernel_path='arch/riscv/boot/Image',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=['-machine virt', '-cpu rv64', '-bios opensbi-riscv64-generic-fw_dynamic.bin']),
> > + 's390' : QemuArchParams(linux_arch='s390',
> > + qemuconfig='CONFIG_EXPERT=y\nCONFIG_TUNE_ZEC12=y\nCONFIG_NUMA=y\nCONFIG_MODULES=y',
> > + qemu_arch='s390x',
> > + kernel_path='arch/s390/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[
> > + '-machine s390-ccw-virtio',
> > + '-cpu qemu',]),
> > + 'sparc' : QemuArchParams(linux_arch='sparc',
> > + qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
> > + qemu_arch='sparc',
> > + kernel_path='arch/sparc/boot/zImage',
> > + kernel_command_line='console=ttyS0 mem=256M',
> > + extra_qemu_params=['-m 256']),
> > +}
>
> Oh my.
> I don't know enough to say if there's a better way of doing this.
Yeah, I know it's gross, but I did not want to put too much effort
into until I got some feedback on it.
> But I think we should probably split this out into a separate python
> file, if this mapping remains necessary.
> E.g. in a qemu_configs.py file or the like.
Definitely an improvement. Any other thoughts on how to make this look
less gross?
>
> > +
> > +class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
>
> I've called out the two errors pytype gives already, but mypy is even
> worse about this new class :(
>
> $ mypy tools/testing/kunit/*.py
> tools/testing/kunit/kunit_kernel.py:90: error: Unsupported operand
> types for + ("str" and "OSError")
> tools/testing/kunit/kunit_kernel.py:278: error: Incompatible types in
> assignment (expression has type "LinuxSourceTreeOperationsQemu",
> variable has type "Optional[LinuxSourceTreeOperationsUml]")
> tools/testing/kunit/kunit_kernel.py:298: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute
> "make_mrproper"
> tools/testing/kunit/kunit_kernel.py:324: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute
> "make_arch_qemuconfig"
> tools/testing/kunit/kunit_kernel.py:325: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute
> "make_olddefconfig"
> tools/testing/kunit/kunit_kernel.py:352: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute
> "make_allyesconfig"
> tools/testing/kunit/kunit_kernel.py:353: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute
> "make_arch_qemuconfig"
> tools/testing/kunit/kunit_kernel.py:354: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute
> "make_olddefconfig"
> tools/testing/kunit/kunit_kernel.py:355: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make"
> tools/testing/kunit/kunit_kernel.py:368: error: Item "None" of
> "Optional[LinuxSourceTreeOperationsUml]" has no attribute "run"
>
> So to make up for mypy being less smart, we can do this and get it down 2 errors
Sounds good. Will do.
> diff --git a/tools/testing/kunit/kunit_kernel.py
> b/tools/testing/kunit/kunit_kernel.py
> index b8b3b76aaa17..a0aaa28db4c1 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -11,7 +11,7 @@ import subprocess
> import os
> import shutil
> import signal
> -from typing import Iterator
> +from typing import Iterator, Union
>
> from contextlib import ExitStack
>
> @@ -269,15 +269,16 @@ class LinuxSourceTree(object):
>
> def __init__(self, build_dir: str, load_config=True,
> kunitconfig_path='', arch=None, cross_compile=None) -> None:
> signal.signal(signal.SIGINT, self.signal_handler)
> - self._ops = None
> + ops = None # type: Union[None,
> LinuxSourceTreeOperationsUml, LinuxSourceTreeOperationsQemu]
> if arch is None or arch == 'um':
> self._arch = 'um'
> - self._ops = LinuxSourceTreeOperationsUml()
> + ops = LinuxSourceTreeOperationsUml()
> elif arch in QEMU_ARCHS:
> self._arch = arch
> - self._ops =
> LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch],
> cross_compile=cross_compile)
> + ops =
> LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch],
> cross_compile=cross_compile)
> else:
> raise ConfigError(arch + ' is not a valid arch')
> + self._ops = ops
>
> if not load_config:
> return
>
> > +
> > + def __init__(self, qemu_arch_params, cross_compile):
> > + super().__init__(linux_arch=qemu_arch_params.linux_arch,
> > + cross_compile=cross_compile)
> > + self._qemuconfig = qemu_arch_params.qemuconfig
> > + self._qemu_arch = qemu_arch_params.qemu_arch
> > + self._kernel_path = qemu_arch_params.kernel_path
> > + print(self._kernel_path)
> looks like a leftover debugging print statement?
Oh yeah. Whoops.
> > + self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
> > + self._extra_qemu_params = qemu_arch_params.extra_qemu_params
> > +
> > + def make_arch_qemuconfig(self, build_dir):
> > + qemuconfig = kunit_config.Kconfig()
> > + qemuconfig.parse_from_string(self._qemuconfig)
> > + qemuconfig.write_to_file(get_kconfig_path(build_dir))
> > +
> > + def run(self, params, timeout, build_dir, outfile):
> > + kernel_path = os.path.join(build_dir, self._kernel_path)
> > + qemu_command = ['qemu-system-' + self._qemu_arch,
> > + '-nodefaults',
> > + '-m', '1024',
> > + '-kernel', kernel_path,
> > + '-append', '\'' + ' '.join(params + [self._kernel_command_line]) + '\'',
> > + '-no-reboot',
> > + '-nographic',
> > + '-serial stdio'] + self._extra_qemu_params
> > + print(' '.join(qemu_command))
>
> ditto, a debug statement?
> Though this one could be useful to actually print out for the user if
> we include some more context in the message.
Yeah, this was initially a debug, but then I ended up finding it
pretty useful since it spits out a full qemu command that you can just
run outside of the tool, so I left it in.
> > + with open(outfile, 'w') as output:
> > + process = subprocess.Popen(' '.join(qemu_command),
> > + stdin=subprocess.PIPE,
> > + stdout=output,
> > + stderr=subprocess.STDOUT,
> > + text=True, shell=True)
> > + try:
> > + process.wait(timeout=timeout)
> > + except Exception as e:
> > + print(e)
> > + process.terminate()
> > + return process
> > +
> > +class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> > + """An abstraction over command line operations performed on a source tree."""
> > +
> > + def __init__(self):
> > + super().__init__(linux_arch='um', cross_compile=None)
> > +
> > def make_allyesconfig(self, build_dir, make_options) -> None:
> > kunit_parser.print_with_timestamp(
> > 'Enabling all CONFIGs for UML...')
> > @@ -83,32 +243,16 @@ class LinuxSourceTreeOperations(object):
> > kunit_parser.print_with_timestamp(
> > 'Starting Kernel with all configs takes a few minutes...')
> >
> > - def make(self, jobs, build_dir, make_options) -> None:
> > - command = ['make', 'ARCH=um', '--jobs=' + str(jobs)]
> > - if make_options:
> > - command.extend(make_options)
> > - if build_dir:
> > - command += ['O=' + build_dir]
> > - try:
> > - proc = subprocess.Popen(command,
> > - stderr=subprocess.PIPE,
> > - stdout=subprocess.DEVNULL)
> > - except OSError as e:
> > - raise BuildError('Could not call make command: ' + str(e))
> > - _, stderr = proc.communicate()
> > - if proc.returncode != 0:
> > - raise BuildError(stderr.decode())
> > - if stderr: # likely only due to build warnings
> > - print(stderr.decode())
> > -
> > - def linux_bin(self, params, timeout, build_dir) -> None:
> > + def run(self, params, timeout, build_dir, outfile):
> > """Runs the Linux UML binary. Must be named 'linux'."""
> > linux_bin = get_file_path(build_dir, 'linux')
> > outfile = get_outfile_path(build_dir)
> > with open(outfile, 'w') as output:
> > process = subprocess.Popen([linux_bin] + params,
> > + stdin=subprocess.PIPE,
> > stdout=output,
> > - stderr=subprocess.STDOUT)
> > + stderr=subprocess.STDOUT,
> > + text=True)
> > process.wait(timeout)
> >
> > def get_kconfig_path(build_dir) -> str:
> > @@ -123,10 +267,17 @@ def get_outfile_path(build_dir) -> str:
> > class LinuxSourceTree(object):
> > """Represents a Linux kernel source tree with KUnit tests."""
> >
> > - def __init__(self, build_dir: str, load_config=True, kunitconfig_path='') -> None:
> > + def __init__(self, build_dir: str, load_config=True, kunitconfig_path='', arch=None, cross_compile=None) -> None:
> > signal.signal(signal.SIGINT, self.signal_handler)
> > -
> > - self._ops = LinuxSourceTreeOperations()
> > + self._ops = None
> > + if arch is None or arch == 'um':
> > + self._arch = 'um'
> > + self._ops = LinuxSourceTreeOperationsUml()
> > + elif arch in QEMU_ARCHS:
> > + self._arch = arch
> > + self._ops = LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile)
> > + else:
> > + raise ConfigError(arch + ' is not a valid arch')
> >
> > if not load_config:
> > return
> > @@ -170,6 +321,7 @@ class LinuxSourceTree(object):
> > os.mkdir(build_dir)
> > self._kconfig.write_to_file(kconfig_path)
> > try:
> > + self._ops.make_arch_qemuconfig(build_dir)
> > self._ops.make_olddefconfig(build_dir, make_options)
> > except ConfigError as e:
> > logging.error(e)
> > @@ -192,10 +344,13 @@ class LinuxSourceTree(object):
> > print('Generating .config ...')
> > return self.build_config(build_dir, make_options)
> >
> > - def build_um_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
> > + def build_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
> > try:
> > if alltests:
> > + if self._arch != 'um':
> > + raise ConfigError('Only the "um" arch is supported for alltests')
> > self._ops.make_allyesconfig(build_dir, make_options)
>
> Minor note: pytype doesn't like this.
> The code is correct, but pytype can't figure out that ops can't be the
> QEMU variant.
>
> Pytype recognizes comments like " # pytype: disable=attribute-error"
> but mypy doesn't.
> So I don't know that there's a way we can make both of them happy :/
Hmmm...I could just add make_allyesconfig() to the base class and make
it raise. That might be cleaner too.
>
> > + self._ops.make_arch_qemuconfig(build_dir)
> > self._ops.make_olddefconfig(build_dir, make_options)
> > self._ops.make(jobs, build_dir, make_options)
> > except (ConfigError, BuildError) as e:
> > @@ -209,8 +364,8 @@ class LinuxSourceTree(object):
> > args.extend(['mem=1G', 'console=tty','kunit_shutdown=halt'])
> > if filter_glob:
> > args.append('kunit.filter_glob='+filter_glob)
> > - self._ops.linux_bin(args, timeout, build_dir)
> > outfile = get_outfile_path(build_dir)
> > + self._ops.run(args, timeout, build_dir, outfile)
> > subprocess.call(['stty', 'sane'])
> > with open(outfile, 'r') as file:
> > for line in file:
> > diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> > index 1ad3049e90698..25e8be95a575d 100755
> > --- a/tools/testing/kunit/kunit_tool_test.py
> > +++ b/tools/testing/kunit/kunit_tool_test.py
> > @@ -297,7 +297,7 @@ class KUnitMainTest(unittest.TestCase):
> >
> > self.linux_source_mock = mock.Mock()
> > self.linux_source_mock.build_reconfig = mock.Mock(return_value=True)
> > - self.linux_source_mock.build_um_kernel = mock.Mock(return_value=True)
> > + self.linux_source_mock.build_kernel = mock.Mock(return_value=True)
> > self.linux_source_mock.run_kernel = mock.Mock(return_value=all_passed_log)
> >
> > def test_config_passes_args_pass(self):
> > @@ -308,7 +308,7 @@ class KUnitMainTest(unittest.TestCase):
> > def test_build_passes_args_pass(self):
> > kunit.main(['build'], self.linux_source_mock)
> > self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
> > - self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, '.kunit', None)
> > + self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, '.kunit', None)
> > self.assertEqual(self.linux_source_mock.run_kernel.call_count, 0)
> >
> > def test_exec_passes_args_pass(self):
> > @@ -390,7 +390,7 @@ class KUnitMainTest(unittest.TestCase):
> > def test_build_builddir(self):
> > build_dir = '.kunit'
> > kunit.main(['build', '--build_dir', build_dir], self.linux_source_mock)
> > - self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, build_dir, None)
> > + self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, build_dir, None)
> >
> > def test_exec_builddir(self):
> > build_dir = '.kunit'
> > @@ -404,14 +404,19 @@ class KUnitMainTest(unittest.TestCase):
> > mock_linux_init.return_value = self.linux_source_mock
> > kunit.main(['run', '--kunitconfig=mykunitconfig'])
> > # Just verify that we parsed and initialized it correctly here.
> > - mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
> > + mock_linux_init.assert_called_once_with('.kunit',
> > + kunitconfig_path='mykunitconfig',
> > + arch='um',
> > + cross_compile=None)
> >
> > @mock.patch.object(kunit_kernel, 'LinuxSourceTree')
> > def test_config_kunitconfig(self, mock_linux_init):
> > mock_linux_init.return_value = self.linux_source_mock
> > kunit.main(['config', '--kunitconfig=mykunitconfig'])
> > # Just verify that we parsed and initialized it correctly here.
> > - mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
> > + mock_linux_init.assert_called_once_with('.kunit',
> > + kunitconfig_path='mykunitconfig',
> > + arch='um')
> >
> > if __name__ == '__main__':
> > unittest.main()
> > --
> > 2.31.1.498.g6c1eba8ee3d-goog
> >
Powered by blists - more mailing lists