| Message ID | 20210923144505.60776-1-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Run kvm-unit-tests with --kernel | expand |
On Thu, 23 Sep 2021 15:44:55 +0100, Alexandru Elisei wrote: > What prompted this series (which I really hoped will turn out smaller than > it did) is my attempt to add support for kvmtool to kvm-unit-tests > automated test runner [1]. When working through the review comments for > that series, I realized that kvmtool must be able to load an initrd when > running a test to get all the features that tests rely on. > > kvm-unit-tests uses the initrd, which is expected to be a text file in the > format key=value, to pass parameters to a test. The initrd is by default > generated by the runner script, but the location of a custom initrd file > can also be set using the environment variable KVM_UNIT_TEST_ENV (many > thanks to Andrew Jones for explaining that). Contained in the automatically > generated initrd is information about the presence of certain commits in > the host kernel. These commits are important because they fix serious bugs > in KVM, and running tests which are designed to exercise the fix on systems > where it isn't present can cause the host kernel to crash. kvm-unit-tests > calls these bug fixing commits erratas, and their presence is signalled by > an entry ERRATA_<commit_id>=y in the initrd. > > [...] Applied patches 1-7, 9 and 10 to kvmtool (master), thanks! [01/10] builtin-run: Treat specifying both --kernel and --firmware as an error https://git.kernel.org/will/kvmtool/c/6810e75ce9e0 [02/10] builtin-run: Warn when ignoring initrd because --firmware was specified https://git.kernel.org/will/kvmtool/c/6cbec43ef88d [03/10] builtin-run: Do not attempt to find vmlinux if --firmware https://git.kernel.org/will/kvmtool/c/638630c9f7a3 [04/10] builtin-run: Abstract argument validation into a separate function https://git.kernel.org/will/kvmtool/c/cce9616484bd [05/10] Use kvm->nr_disks instead of kvm->cfg.image_count https://git.kernel.org/will/kvmtool/c/39ab3a0b380c [06/10] builtin-run: Move kernel command line generation to a separate function https://git.kernel.org/will/kvmtool/c/a5253f7cc810 [07/10] Add --nodefaults command line argument https://git.kernel.org/will/kvmtool/c/5613ae26b998 [09/10] arm64: Use the default offset when the kernel image magic is not found https://git.kernel.org/will/kvmtool/c/5303f0964ffd [10/10] arm64: Be more permissive when parsing the kernel header https://git.kernel.org/will/kvmtool/c/dc6646192057 Cheers,
What prompted this series (which I really hoped will turn out smaller than it did) is my attempt to add support for kvmtool to kvm-unit-tests automated test runner [1]. When working through the review comments for that series, I realized that kvmtool must be able to load an initrd when running a test to get all the features that tests rely on. kvm-unit-tests uses the initrd, which is expected to be a text file in the format key=value, to pass parameters to a test. The initrd is by default generated by the runner script, but the location of a custom initrd file can also be set using the environment variable KVM_UNIT_TEST_ENV (many thanks to Andrew Jones for explaining that). Contained in the automatically generated initrd is information about the presence of certain commits in the host kernel. These commits are important because they fix serious bugs in KVM, and running tests which are designed to exercise the fix on systems where it isn't present can cause the host kernel to crash. kvm-unit-tests calls these bug fixing commits erratas, and their presence is signalled by an entry ERRATA_<commit_id>=y in the initrd. Using --kernel to run a test is not possible for two reasons: 1. Commit fd0a05bd27dd ("arm64: Obtain text offset from kernel image") made it such that the kernel load offset is read from the binary file even if the kernel header is not detected. kvmtool will try to load a test at a random address read from the text section of the binary, which most of the time is well above the upper limit for the VM's RAM in a normal VM configuration. 2. kvm-unit-tests uses the kernel command line as the source for various test parameters. kvmtool modifies the kernel command line that the user specified to try to make it as painless as possible to boot a kernel, but for a test this means that parsing the kernel command line for the required parameters fails because of those unexpected, and in this case, unwanted additions. Currently, running any kvm-unit-tests test with kvmtool can only be done with --firmware, which does not touch the command line, but it has the downside of not being able to load an initrd. I decided to add a new kvmtool command line option, --nodefaults, which disables all the automated stuff that kvmtool does without being explicitly told so by the user (which includes modifying the kernel command line). I believe a --nodefaults option has merit even outside enabling support for automating kvm-unit-tests runs. There are legitimate reasons for a user to remove all the parameters that kvmtool adds to the kernel command line (like testing or trying to understand how something works), or the generated rootfs filesystem (for example, the initrd with the rootfs is included in the kernel). I think that giving the user as much control as possible is very useful for a program like kvmtool, which lends itself very well to quick testing and prototyping. The --nocompat option was added because compat warnings, in certain situations, can be more confusing than useful on arm64, which has virtio in defconfig. Of course, this is under the assumption that a user who removes virtio from the kernel config knows what he's doing, but the compat warnings are still shown by default just in case. Also, with the --firmware option, the assumption that every guest is a Linux kernel and has working virtio is not really true any more. A quick summary of the patches: * Patches #1 through #4 are for making kvmtool's command line more informative when options are ignored because --kernel and --firmware are handled differently. * Patch #5 removes kvm->cfg.image_count, which is really the same thing as kvm->nr_disks. I found this very confusing when trying to understand the function kvm_cmd_run_init(), but the patch is not necessary for what this series is trying to achieve. * Patch #6 is a preparatory patch and #7 adds the --nodefaults option. * Patch #8 adds the --nocompat option. * Patch #9 and #10 is my attempt at making kvmtool slightly more lenient when loading something other than a kernel with --kernel. Patch #9 is squarely aimed at loading kvm-unit-tests with --kernel (which was possible before kernel header parsing, but not anymore). Patch #10 is my attempt at hiding a warning which is harmless in the context of loading a kvm-unit-tests test. [1] https://lists.cs.columbia.edu/pipermail/kvmarm/2021-July/047747.html Alexandru Elisei (10): builtin-run: Treat specifying both --kernel and --firmware as an error builtin-run: Warn when ignoring initrd because --firmware was specified builtin-run: Do not attempt to find vmlinux if --firmware builtin-run: Abstract argument validation into a separate function Use kvm->nr_disks instead of kvm->cfg.image_count builtin-run: Move kernel command line generation to a separate function Add --nodefaults command line argument Add --nocompat option to disable compat warnings arm64: Use the default offset when the kernel image magic is not found arm64: Be more permissive when parsing the kernel header arm/aarch64/kvm.c | 18 ++--- arm/fdt.c | 3 +- builtin-run.c | 144 ++++++++++++++++++++++++--------------- disk/core.c | 18 ++--- guest_compat.c | 1 + include/kvm/kvm-config.h | 3 +- mips/kvm.c | 3 +- 7 files changed, 114 insertions(+), 76 deletions(-)