Re: [kvm-unit-tests PATCH v3 07/16] scripts: Use an associative array for qemu argument names

Andrew Jones
2025-05-07T16:17:56Z

On Wed, May 07, 2025 at 04:12:47PM +0100, Alexandru Elisei wrote:
> Move away from hardcoded qemu arguments and use instead an associative
> array to get the needed arguments. This paves the way for adding kvmtool
> support to the scripts, which has a different syntax for the same VM
> configuration parameters.
>
> Suggested-by: Andrew Jones <andrew.jones@linux.dev>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> scripts/common.bash | 10 +++++++---
> scripts/runtime.bash | 7 +------
> scripts/vmm.bash | 7 +++++++
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 9deb87d4050d..649f1c737617 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -1,4 +1,5 @@
> source config.mak
> +source scripts/vmm.bash
>
> function for_each_unittest()
> {
> @@ -26,8 +27,11 @@ function for_each_unittest()
> $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$test_args" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> fi
> testname=$rematch
> - smp=1
> + smp="${vmm_opts[$TARGET:nr_cpus]} 1"

I think the wrapper functions you suggested in the cover letter might be
nice just to keep Bash from hurting people's brains too much. At least to
me,

smp="$(vmm_optname_nr_cpus) 1"

would read better. Also note the use of 'optname' in the name to try and
help self-document that this array is holding option names, not option
values.

> kernel=""
> + # Intentionally don't use -append if test_args is empty
> + # because qemu interprets the first argument after
> + # -append as a kernel parameter.
> test_args=""
> opts=""
> groups=""
> @@ -39,9 +43,9 @@ function for_each_unittest()
> elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
> kernel=$TEST_DIR/${BASH_REMATCH[1]}
> elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> - smp=${BASH_REMATCH[1]}
> + smp="${vmm_opts[$TARGET:nr_cpus]} ${BASH_REMATCH[1]}"
> elif [[ $line =~ ^test_args\ *=\ *(.*)$ ]]; then
> - test_args=${BASH_REMATCH[1]}
> + test_args="${vmm_opts[$TARGET:args]} ${BASH_REMATCH[1]}"
> elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *'"""'(.*)$ ]]; then
> opts=${BASH_REMATCH[2]}$'\n'
> while read -r -u $fd; do
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 06cc58e79b69..86d8a2cd8528 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -34,7 +34,7 @@ premature_failure()
> get_cmdline()
> {
> local kernel=$1
> - echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> + echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $smp $test_args $opts"
> }
>
> skip_nodefault()
> @@ -88,11 +88,6 @@ function run()
> local accel="${10}"
> local timeout="${11:-$TIMEOUT}" # unittests.cfg overrides the default
>
> - # If $test_args is empty, qemu will interpret the first option after -append
> - # as a kernel parameter instead of a qemu option, so make sure the -append
> - # option is used only if $test_args is not empy.
> - [ -n "$test_args" ] && opts="-append $test_args $opts"
> -
> if [ "${CONFIG_EFI}" == "y" ]; then
> kernel=${kernel/%.flat/.efi}
> fi
> diff --git a/scripts/vmm.bash b/scripts/vmm.bash
> index 39325858c6b3..b02055a5c0b6 100644
> --- a/scripts/vmm.bash
> +++ b/scripts/vmm.bash
> @@ -1,5 +1,12 @@
> source config.mak
>
> +declare -A vmm_opts=(
> + [qemu:nr_cpus]='-smp'
> + [qemu:kernel]='-kernel'
> + [qemu:args]='-append'
> + [qemu:initrd]='-initrd'
> +)
> +
> function check_vmm_supported()
> {
> case "$TARGET" in
> --
> 2.49.0

Otherwise,

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

Thanks,
drew