* [RFC PATCH] kunit: tool: Enable virtio/PCI by default on UML
@ 2022-06-22 3:53 David Gow
2022-06-22 15:59 ` Daniel Latypov
2022-06-23 19:30 ` Brendan Higgins
0 siblings, 2 replies; 3+ messages in thread
From: David Gow @ 2022-06-22 3:53 UTC (permalink / raw)
To: Daniel Latypov, Brendan Higgins
Cc: Shuah Khan, kunit-dev, linux-kselftest, linux-kernel,
José Expósito, David Gow
There are several tests which depend on PCI, and hence need a bunch of
extra options to run under UML. This makes it awkward to give
configuration instructions (whether in documentation, or as part of a
.kunitconfig file), as two separate, incompatible sets of config options
are required for UML and "most other architectures".
For non-UML architectures, it's possible to add default kconfig options
via the qemu_config python files, but there's no equivalent for UML. Add
a new tools/testing/kunit/configs/arch_uml.config file containing extra
kconfig options to use on UML.
Signed-off-by: David Gow <davidgow@google.com>
---
It's really ugly to have to type:
--kconfig_add CONFIG_VIRTIO_UML=y
--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
when running many tests under UML, particularly since it isn't required
on other architectures.
This came up in discussion with Daniel this morning, and while the
ability to repeat the --kunitconfig flag would go some way to alleviate
this, having to add:
--kunitconfig ./tools/testing/kunit/config/uml_pci.kunitconfig
isn't all that much better.
So it seems like adding something by default would be nice.
This implementation is not perfect (in particular, there's no easy way
of _disabling_ these options now, though [1] probably will help). The
'arch_uml.config' filename can be bikeshedded, too.
Thoughts?
---
tools/testing/kunit/configs/arch_uml.config | 5 +++++
tools/testing/kunit/kunit_kernel.py | 11 ++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/kunit/configs/arch_uml.config
diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
new file mode 100644
index 000000000000..e824ce43b05a
--- /dev/null
+++ b/tools/testing/kunit/configs/arch_uml.config
@@ -0,0 +1,5 @@
+# Config options which are added to UML builds by default
+
+# Enable virtio/pci, as a lot of tests require it.
+CONFIG_VIRTIO_UML=y
+CONFIG_UML_PCI_OVER_VIRTIO=y
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 3539efaf99ba..05e7b1e188d7 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -26,6 +26,7 @@ KUNITCONFIG_PATH = '.kunitconfig'
OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig'
DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config'
BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
+UML_KCONFIG_PATH = 'tools/testing/kunit/configs/arch_uml.config'
OUTFILE_PATH = 'test.log'
ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
@@ -53,7 +54,7 @@ class LinuxSourceTreeOperations:
except subprocess.CalledProcessError as e:
raise ConfigError(e.output.decode())
- def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> None:
pass
def make_allyesconfig(self, build_dir: str, make_options) -> None:
@@ -109,7 +110,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
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, base_kunitconfig: kunit_config.Kconfig) -> None:
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> None:
kconfig = kunit_config.parse_from_string(self._kconfig)
base_kunitconfig.merge_in_entries(kconfig)
@@ -137,6 +138,10 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
def __init__(self, cross_compile=None):
super().__init__(linux_arch='um', cross_compile=cross_compile)
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> None:
+ kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
+ base_kunitconfig.merge_in_entries(kconfig)
+
def make_allyesconfig(self, build_dir: str, make_options) -> None:
kunit_parser.print_with_timestamp(
'Enabling all CONFIGs for UML...')
@@ -313,7 +318,7 @@ class LinuxSourceTree:
return self.build_config(build_dir, make_options)
existing_kconfig = kunit_config.parse_file(kconfig_path)
- self._ops.make_arch_qemuconfig(self._kconfig)
+ self._ops.make_arch_config(self._kconfig)
if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
return True
print('Regenerating .config ...')
--
2.37.0.rc0.104.g0611611a94-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] kunit: tool: Enable virtio/PCI by default on UML
2022-06-22 3:53 [RFC PATCH] kunit: tool: Enable virtio/PCI by default on UML David Gow
@ 2022-06-22 15:59 ` Daniel Latypov
2022-06-23 19:30 ` Brendan Higgins
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Latypov @ 2022-06-22 15:59 UTC (permalink / raw)
To: David Gow
Cc: Brendan Higgins, Shuah Khan, kunit-dev, linux-kselftest,
linux-kernel, José Expósito
On Tue, Jun 21, 2022 at 8:53 PM David Gow <davidgow@google.com> wrote:
>
> There are several tests which depend on PCI, and hence need a bunch of
> extra options to run under UML. This makes it awkward to give
> configuration instructions (whether in documentation, or as part of a
> .kunitconfig file), as two separate, incompatible sets of config options
> are required for UML and "most other architectures".
>
> For non-UML architectures, it's possible to add default kconfig options
> via the qemu_config python files, but there's no equivalent for UML. Add
> a new tools/testing/kunit/configs/arch_uml.config file containing extra
> kconfig options to use on UML.
>
> Signed-off-by: David Gow <davidgow@google.com>
> ---
>
> It's really ugly to have to type:
> --kconfig_add CONFIG_VIRTIO_UML=y
> --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> when running many tests under UML, particularly since it isn't required
> on other architectures.
>
> This came up in discussion with Daniel this morning, and while the
> ability to repeat the --kunitconfig flag would go some way to alleviate
> this, having to add:
> --kunitconfig ./tools/testing/kunit/config/uml_pci.kunitconfig
> isn't all that much better.
>
> So it seems like adding something by default would be nice.
>
> This implementation is not perfect (in particular, there's no easy way
> of _disabling_ these options now, though [1] probably will help). The
I assume the missing link for [1] is
https://lore.kernel.org/linux-kselftest/20220520224200.3764027-1-dlatypov@google.com/
Note that we'll have to update one of these patches depending on the
order they go in.
[1] had to change make_arch_qemuconfig() to ensure that arch/qemu
configs have *lower* priority than user-specified overrides.
So we'll either need to do the same here or update [1] for this to work out.
> 'arch_uml.config' filename can be bikeshedded, too.
>
> Thoughts?
I was initially against effectively hard-coding these in, but it feels
like making CONFIG_PCI=y work by default on UML is worth it.
We've talked about architecture-specific options in kunitconfig files
and it's only ever been about PCI. Nothing else has come up yet, so
this patch would eliminate that concern for now.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] kunit: tool: Enable virtio/PCI by default on UML
2022-06-22 3:53 [RFC PATCH] kunit: tool: Enable virtio/PCI by default on UML David Gow
2022-06-22 15:59 ` Daniel Latypov
@ 2022-06-23 19:30 ` Brendan Higgins
1 sibling, 0 replies; 3+ messages in thread
From: Brendan Higgins @ 2022-06-23 19:30 UTC (permalink / raw)
To: David Gow
Cc: Daniel Latypov, Shuah Khan, kunit-dev, linux-kselftest,
linux-kernel, José Expósito
On Tue, Jun 21, 2022 at 8:53 PM David Gow <davidgow@google.com> wrote:
>
> There are several tests which depend on PCI, and hence need a bunch of
> extra options to run under UML. This makes it awkward to give
> configuration instructions (whether in documentation, or as part of a
> .kunitconfig file), as two separate, incompatible sets of config options
> are required for UML and "most other architectures".
>
> For non-UML architectures, it's possible to add default kconfig options
> via the qemu_config python files, but there's no equivalent for UML. Add
> a new tools/testing/kunit/configs/arch_uml.config file containing extra
> kconfig options to use on UML.
>
> Signed-off-by: David Gow <davidgow@google.com>
> ---
>
> It's really ugly to have to type:
> --kconfig_add CONFIG_VIRTIO_UML=y
> --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> when running many tests under UML, particularly since it isn't required
> on other architectures.
>
> This came up in discussion with Daniel this morning, and while the
> ability to repeat the --kunitconfig flag would go some way to alleviate
> this, having to add:
> --kunitconfig ./tools/testing/kunit/config/uml_pci.kunitconfig
> isn't all that much better.
>
> So it seems like adding something by default would be nice.
>
> This implementation is not perfect (in particular, there's no easy way
> of _disabling_ these options now, though [1] probably will help). The
> 'arch_uml.config' filename can be bikeshedded, too.
>
> Thoughts?
I am supportive of adding the PCI dependency as a default for UML; I
agree that there is a common desire to use PCI for tests and enabling
it on UML is wonky.
One additional note: It looks like this patch breaks kunit_tool:
https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/21871
The python error in the log seems legitimate:
https://storage.googleapis.com/oss-prow/pr-logs/pull/linux-review.googlesource.com_linux_kernel_git_torvalds_linux/21871/linux-kernel-mailing-list-presubmit/1539456886976286720/build-log.txt
This line in particular:
AttributeError: 'LinuxSourceTreeOperationsUml' object has no attribute
'make_arch_qemuconfig'
> ---
> tools/testing/kunit/configs/arch_uml.config | 5 +++++
> tools/testing/kunit/kunit_kernel.py | 11 ++++++++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/kunit/configs/arch_uml.config
>
> diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
> new file mode 100644
> index 000000000000..e824ce43b05a
> --- /dev/null
> +++ b/tools/testing/kunit/configs/arch_uml.config
> @@ -0,0 +1,5 @@
> +# Config options which are added to UML builds by default
> +
> +# Enable virtio/pci, as a lot of tests require it.
> +CONFIG_VIRTIO_UML=y
> +CONFIG_UML_PCI_OVER_VIRTIO=y
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 3539efaf99ba..05e7b1e188d7 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -26,6 +26,7 @@ KUNITCONFIG_PATH = '.kunitconfig'
> OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig'
> DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config'
> BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> +UML_KCONFIG_PATH = 'tools/testing/kunit/configs/arch_uml.config'
> OUTFILE_PATH = 'test.log'
> ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> @@ -53,7 +54,7 @@ class LinuxSourceTreeOperations:
> except subprocess.CalledProcessError as e:
> raise ConfigError(e.output.decode())
>
> - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> pass
>
> def make_allyesconfig(self, build_dir: str, make_options) -> None:
> @@ -109,7 +110,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> 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, base_kunitconfig: kunit_config.Kconfig) -> None:
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> kconfig = kunit_config.parse_from_string(self._kconfig)
> base_kunitconfig.merge_in_entries(kconfig)
>
> @@ -137,6 +138,10 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> def __init__(self, cross_compile=None):
> super().__init__(linux_arch='um', cross_compile=cross_compile)
>
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> + kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
> + base_kunitconfig.merge_in_entries(kconfig)
> +
> def make_allyesconfig(self, build_dir: str, make_options) -> None:
> kunit_parser.print_with_timestamp(
> 'Enabling all CONFIGs for UML...')
> @@ -313,7 +318,7 @@ class LinuxSourceTree:
> return self.build_config(build_dir, make_options)
>
> existing_kconfig = kunit_config.parse_file(kconfig_path)
> - self._ops.make_arch_qemuconfig(self._kconfig)
> + self._ops.make_arch_config(self._kconfig)
> if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
> return True
> print('Regenerating .config ...')
> --
> 2.37.0.rc0.104.g0611611a94-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-23 19:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-22 3:53 [RFC PATCH] kunit: tool: Enable virtio/PCI by default on UML David Gow
2022-06-22 15:59 ` Daniel Latypov
2022-06-23 19:30 ` Brendan Higgins
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).