linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Jones <andrew.jones@linux.dev>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: eric.auger@redhat.com, lvivier@redhat.com, thuth@redhat.com,
	 frankja@linux.ibm.com, imbrenda@linux.ibm.com,
	nrb@linux.ibm.com, david@redhat.com,  pbonzini@redhat.com,
	kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	 linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org,
	linux-s390@vger.kernel.org,  will@kernel.org,
	julien.thierry.kdev@gmail.com, maz@kernel.org,
	 oliver.upton@linux.dev, suzuki.poulose@arm.com,
	yuzenghui@huawei.com, joey.gouly@arm.com,
	 andre.przywara@arm.com
Subject: Re: [kvm-unit-tests PATCH v3 10/16] scripts: Add default arguments for kvmtool
Date: Wed, 7 May 2025 18:43:02 +0200	[thread overview]
Message-ID: <20250507-87fb6e08b8aef50d174a0437@orel> (raw)
In-Reply-To: <20250507151256.167769-11-alexandru.elisei@arm.com>

On Wed, May 07, 2025 at 04:12:50PM +0100, Alexandru Elisei wrote:
> kvmtool, unless told otherwise, will do its best to make sure that a kernel
> successfully boots in a virtual machine. It does things like automatically
> creating a rootfs and adding extra parameters to the kernel command line.
> This is actively harmful to kvm-unit-tests, because some tests parse the
> kernel command line and they will fail if they encounter the options added
> by kvmtool.
> 
> Fortunately for us, kvmtool commit 5613ae26b998 ("Add --nodefaults command
> line argument") addded the --nodefaults kvmtool parameter which disables
> all the implicit virtual machine configuration that cannot be disabled by
> using other parameters, like modifying the kernel command line. So always
> use --nodefaults to allow a test to run.
> 
> kvmtool can also be too verbose when running a virtual machine, and this is
> controlled by several parameters. Add those to the default kvmtool command
> line to reduce this verbosity to a minimum.
> 
> Before:
> 
> $ vm run arm/selftest.flat --cpus 2 --mem 256 --params "setup smp=2 mem=256"
> Info: # lkvm run -k arm/selftest.flat -m 256 -c 2 --name guest-5035
> Unknown subtest
> 
> EXIT: STATUS=127
> Warning: KVM compatibility warning.
>     virtio-9p device was not detected.
>     While you have requested a virtio-9p device, the guest kernel did not initialize it.
>     Please make sure that the guest kernel was compiled with CONFIG_NET_9P_VIRTIO=y enabled in .config.
> Warning: KVM compatibility warning.
>     virtio-net device was not detected.
>     While you have requested a virtio-net device, the guest kernel did not initialize it.
>     Please make sure that the guest kernel was compiled with CONFIG_VIRTIO_NET=y enabled in .config.
> Info: KVM session ended normally.
> 
> After:
> 
> $ vm run arm/selftest.flat --nodefaults --network mode=none --loglevel=warning --cpus 2 --mem 256 --params "setup smp=2 mem=256"
> PASS: selftest: setup: smp: number of CPUs matches expectation
> INFO: selftest: setup: smp: found 2 CPUs
> PASS: selftest: setup: mem: memory size matches expectation
> INFO: selftest: setup: mem: found 256 MB
> SUMMARY: 2 tests
> 
> EXIT: STATUS=1
> 
> Note that KVMTOOL_DEFAULT_OPTS can be overwritten by an environment
> variable with the same name, but it's not documented in the help string for
> run_tests.sh. This has been done on purpose, since overwritting
> KVMTOOL_DEFAULT_OPTS should only be necessary for debugging or development
> purposes.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/common.bash | 10 +++++-----
>  scripts/vmm.bash    | 13 +++++++++++++
>  2 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 0645235d8baa..ee0ae71948c2 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -56,7 +56,7 @@ function for_each_unittest()
>  			# because qemu interprets the first argument after
>  			# -append as a kernel parameter.
>  			test_args=""
> -			opts=""
> +			opts="${vmm_opts[$TARGET:default_opts]}"
>  			groups=""
>  			arch=""
>  			machine=""
> @@ -70,13 +70,13 @@ function for_each_unittest()
>  		elif [[ $line =~ ^test_args\ *=\ *(.*)$ ]]; then
>  			test_args="${vmm_opts[$TARGET:args]} ${BASH_REMATCH[1]}"
>  		elif [[ $TARGET = "qemu" ]] && [[ $line =~ ^(extra_params|qemu_params)\ *=\ *'"""'(.*)$ ]]; then
> -			opts=$(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)
> +			opts="${vmm_opts[$TARGET:default_opts]} $(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)"
>  		elif [[ $TARGET  = "qemu" ]] && [[ $line =~ ^(extra_params|qemu_params)\ *=\ *(.*)$ ]]; then
> -			opts=${BASH_REMATCH[2]}
> +			opts="${vmm_opts[$TARGET:default_opts]} ${BASH_REMATCH[2]}"
>  		elif [[ $TARGET = "kvmtool" ]] && [[ $line =~ ^kvmtool_params\ *=\ *'"""'(.*)$ ]]; then
> -			opts=$(parse_opts ${BASH_REMATCH[1]}$'\n' $fd)
> +			opts="${vmm_opts[$TARGET:default_opts]} $(parse_opts ${BASH_REMATCH[1]}$'\n' $fd)"
>  		elif [[ $TARGET = "kvmtool" ]] && [[ $line =~ ^kvmtool_params\ *=\ *(.*)$ ]]; then
> -			opts=${BASH_REMATCH[1]}
> +			opts="${vmm_opts[$TARGET:default_opts]} ${BASH_REMATCH[1]}"
>  		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
>  			groups=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> diff --git a/scripts/vmm.bash b/scripts/vmm.bash
> index 20968f2e6b10..d24a4c4b8713 100644
> --- a/scripts/vmm.bash
> +++ b/scripts/vmm.bash
> @@ -1,5 +1,16 @@
>  source config.mak
>  
> +# The following parameters are enabled by default when running a test with
> +# kvmtool:
> +# --nodefaults: suppress VM configuration that cannot be disabled otherwise
> +#               (like modifying the supplied kernel command line). Tests that
> +#               use the command line will fail without this parameter.
> +# --network mode=none: do not create a network device. kvmtool tries to help the
> +#               user by automatically create one, and then prints a warning
> +#               when the VM terminates if the device hasn't been initialized.
> +# --loglevel=warning: reduce verbosity
> +: "${KVMTOOL_DEFAULT_OPTS:="--nodefaults --network mode=none --loglevel=warning"}"
> +
>  ##############################################################################
>  # qemu_fixup_return_code translates the ambiguous exit status in Table1 to that
>  # in Table2.  Table3 simply documents the complete status table.
> @@ -87,12 +98,14 @@ declare -A vmm_opts=(
>  	[qemu:kernel]='-kernel'
>  	[qemu:args]='-append'
>  	[qemu:initrd]='-initrd'
> +	[qemu:default_opts]=''
>  	[qemu:fixup_return_code]=qemu_fixup_return_code
>  
>  	[kvmtool:nr_cpus]='--cpus'
>  	[kvmtool:kernel]='--kernel'
>  	[kvmtool:args]='--params'
>  	[kvmtool:initrd]='--initrd'
> +	[kvmtool:default_opts]="$KVMTOOL_DEFAULT_OPTS"
>  	[kvmtool:fixup_return_code]=kvmtool_fixup_return_code
>  )
>  
> -- 
> 2.49.0

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>


  reply	other threads:[~2025-05-07 16:43 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-07 15:12 [kvm-unit-tests PATCH v3 00/16] arm/arm64: Add kvmtool to the runner script Alexandru Elisei
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 01/16] scripts: unittests.cfg: Rename 'extra_params' to 'qemu_params' Alexandru Elisei
2025-05-07 15:40   ` Andrew Jones
2025-05-14  2:57   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 02/16] scripts: Add 'test_args' test definition parameter Alexandru Elisei
2025-05-07 15:58   ` Andrew Jones
2025-05-14  3:16   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 03/16] configure: Export TARGET unconditionally Alexandru Elisei
2025-05-07 16:02   ` Andrew Jones
2025-05-08  8:52     ` Alexandru Elisei
2025-05-08  9:39       ` Andrew Jones
2025-05-08 10:05         ` Alexandru Elisei
2025-05-08 10:17           ` Andrew Jones
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 04/16] run_tests.sh: Document --probe-maxsmp argument Alexandru Elisei
2025-05-14  3:29   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 05/16] scripts: Document environment variables Alexandru Elisei
2025-05-14  3:36   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 06/16] scripts: Refuse to run the tests if not configured for qemu Alexandru Elisei
2025-05-07 16:10   ` Andrew Jones
2025-05-07 16:14     ` Alexandru Elisei
2025-05-14  7:49   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 07/16] scripts: Use an associative array for qemu argument names Alexandru Elisei
2025-05-07 16:17   ` Andrew Jones
2025-05-14  8:02   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 08/16] scripts: Add 'kvmtool_params' to test definition Alexandru Elisei
2025-05-07 16:28   ` Andrew Jones
2025-05-08 15:54     ` Alexandru Elisei
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 09/16] scripts: Add support for kvmtool Alexandru Elisei
2025-05-07 16:38   ` Andrew Jones
2025-05-19  8:55   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 10/16] scripts: Add default arguments " Alexandru Elisei
2025-05-07 16:43   ` Andrew Jones [this message]
2025-05-21  3:21   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 11/16] scripts: Add KVMTOOL environment variable for kvmtool binary path Alexandru Elisei
2025-05-07 16:45   ` Andrew Jones
2025-05-19  8:13   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 12/16] scripts: Detect kvmtool failure in premature_failure() Alexandru Elisei
2025-05-07 16:47   ` Andrew Jones
2025-05-21  5:58   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 13/16] scripts: Do not probe for maximum number of VCPUs when using kvmtool Alexandru Elisei
2025-05-07 16:48   ` Andrew Jones
2025-05-21  6:02   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 14/16] scripts/mkstandalone: Export $TARGET Alexandru Elisei
2025-05-21  6:16   ` Shaoqin Huang
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 15/16] scripts: Add 'disabled_if' test definition parameter for kvmtool to use Alexandru Elisei
2025-05-07 16:56   ` Andrew Jones
2025-05-07 15:12 ` [kvm-unit-tests PATCH v3 16/16] scripts: Enable kvmtool Alexandru Elisei
2025-05-07 16:59   ` Andrew Jones
2025-05-21  6:20   ` Shaoqin Huang
2025-05-19  8:56 ` [kvm-unit-tests PATCH v3 00/16] arm/arm64: Add kvmtool to the runner script Shaoqin Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250507-87fb6e08b8aef50d174a0437@orel \
    --to=andrew.jones@linux.dev \
    --cc=alexandru.elisei@arm.com \
    --cc=andre.przywara@arm.com \
    --cc=david@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=joey.gouly@arm.com \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lvivier@redhat.com \
    --cc=maz@kernel.org \
    --cc=nrb@linux.ibm.com \
    --cc=oliver.upton@linux.dev \
    --cc=pbonzini@redhat.com \
    --cc=suzuki.poulose@arm.com \
    --cc=thuth@redhat.com \
    --cc=will@kernel.org \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).