Re: [kvm-unit-tests PATCH v2 07/18] scripts: Introduce kvmtool_opts

[Andrew Jones <andrew.jones@linux.dev>]

On Mon, Jan 20, 2025 at 04:43:05PM +0000, Alexandru Elisei wrote:
> In preparation for supporting kvmtool, create and pass the variable
> 'kvmtool_opts' to the arch run script $RUNTIME_arch_run.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/common.bash  |  6 ++++--
>  scripts/runtime.bash | 14 +++++++++++---
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/common.bash b/scripts/common.bash
> index a40c28121b6a..1b5e0d667841 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -37,6 +37,7 @@ function for_each_unittest()
>  	local check
>  	local accel
>  	local timeout
> +	local kvmtool_opts
>  	local rematch
>  
>  	exec {fd}<"$unittests"
> @@ -45,7 +46,7 @@ function for_each_unittest()
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>  			rematch=${BASH_REMATCH[1]}
>  			if [ -n "${testname}" ]; then
> -				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> +				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"

It looks odd to have both qemu_opts and kvmtool_opts at the same time.

>  			fi
>  			testname=$rematch
>  			smp=1
> @@ -57,6 +58,7 @@ function for_each_unittest()
>  			check=""
>  			accel=""
>  			timeout=""
> +			kvmtool_opts=""
>  		elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
>  			kernel=$TEST_DIR/${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> @@ -80,7 +82,7 @@ function for_each_unittest()
>  		fi
>  	done
>  	if [ -n "${testname}" ]; then
> -		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> +		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
>  	fi
>  	exec {fd}<&-
>  }
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index a89f2d10ab78..451b5585f010 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -35,7 +35,7 @@ get_cmdline()
>  {
>      local kernel=$1
>  
> -    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $qemu_opts"
> +    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $opts"
>  }
>  
>  skip_nodefault()
> @@ -87,8 +87,16 @@ function run()
>      local check="${CHECK:-$8}"
>      local accel="$9"
>      local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
> -
> -    qemu_opts="-smp $smp $qemu_opts"
> +    local kvmtool_opts="${11}"
> +
> +    case "$TARGET" in
> +    qemu)
> +        opts="-smp $smp $qemu_opts"
> +        ;;
> +    kvmtool)
> +        opts="--cpus $smp $kvmtool_opts"
> +        ;;
> +    esac

This is similar to what I was proposing with the associative array, but
we'll only need to set a $vmm variable once with the array. If parsing
command lines is different between the vmms then we can even add
functions to the table

vmm_opts[qemu,func]=qemu_func
vmm_opts[kvmtool,func]=kvmtool_func

eval ${vmm_opts[$vmm,func]} ...

Thanks,
drew

>  
>      if [ "${CONFIG_EFI}" == "y" ]; then
>          kernel=${kernel/%.flat/.efi}
> -- 
> 2.47.1
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv