Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

[Thomas Huth <thuth@redhat.com>]

On 04/05/2024 14.28, Nicholas Piggin wrote:
> This allows different machines with different requirements to be
> supported by run_tests.sh, similarly to how different accelerators
> are handled.
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> Acked-by: Andrew Jones <andrew.jones@linux.dev>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   docs/unittests.txt   |  7 +++++++
>   scripts/common.bash  |  8 ++++++--
>   scripts/runtime.bash | 16 ++++++++++++----
>   3 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/unittests.txt b/docs/unittests.txt
> index 7cf2c55ad..6449efd78 100644
> --- a/docs/unittests.txt
> +++ b/docs/unittests.txt
> @@ -42,6 +42,13 @@ For <arch>/ directories that support multiple architectures, this restricts
>   the test to the specified arch. By default, the test will run on any
>   architecture.
>   
> +machine
> +-------
> +For those architectures that support multiple machine types, this restricts
> +the test to the specified machine. By default, the test will run on
> +any machine type. (Note, the machine can be specified with the MACHINE=
> +environment variable, and defaults to the architecture's default.)
> +
>   smp
>   ---
>   smp = <number>
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 5e9ad53e2..3aa557c8c 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -10,6 +10,7 @@ function for_each_unittest()
>   	local opts
>   	local groups
>   	local arch
> +	local machine
>   	local check
>   	local accel
>   	local timeout
> @@ -21,7 +22,7 @@ function for_each_unittest()
>   		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>   			rematch=${BASH_REMATCH[1]}
>   			if [ -n "${testname}" ]; then
> -				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
>   			fi
>   			testname=$rematch
>   			smp=1
> @@ -29,6 +30,7 @@ function for_each_unittest()
>   			opts=""
>   			groups=""
>   			arch=""
> +			machine=""
>   			check=""
>   			accel=""
>   			timeout=""
> @@ -58,6 +60,8 @@ function for_each_unittest()
>   			groups=${BASH_REMATCH[1]}
>   		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
>   			arch=${BASH_REMATCH[1]}
> +		elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
> +			machine=${BASH_REMATCH[1]}
>   		elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
>   			check=${BASH_REMATCH[1]}
>   		elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
> @@ -67,7 +71,7 @@ function for_each_unittest()
>   		fi
>   	done
>   	if [ -n "${testname}" ]; then
> -		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
>   	fi
>   	exec {fd}<&-
>   }
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 177b62166..0c96d6ea2 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -32,7 +32,7 @@ premature_failure()
>   get_cmdline()
>   {
>       local kernel=$1
> -    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> +    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
>   }
>   
>   skip_nodefault()
> @@ -80,9 +80,10 @@ function run()
>       local kernel="$4"
>       local opts="$5"
>       local arch="$6"
> -    local check="${CHECK:-$7}"
> -    local accel="$8"
> -    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
> +    local machine="$7"
> +    local check="${CHECK:-$8}"
> +    local accel="$9"
> +    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
>   
>       if [ "${CONFIG_EFI}" == "y" ]; then
>           kernel=${kernel/%.flat/.efi}
> @@ -116,6 +117,13 @@ function run()
>           return 2
>       fi
>   
> +    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != "$MACHINE" ]; then
> +        print_result "SKIP" $testname "" "$machine only"
> +        return 2
> +    elif [ -n "$MACHINE" ]; then
> +        machine="$MACHINE"
> +    fi
> +
>       if [ -n "$accel" ] && [ -n "$ACCEL" ] && [ "$accel" != "$ACCEL" ]; then
>           print_result "SKIP" $testname "" "$accel only, but ACCEL=$ACCEL"
>           return 2

For some reasons that I don't quite understand yet, this patch causes the 
"sieve" test to always timeout on the s390x runner, see e.g.:

  https://gitlab.com/thuth/kvm-unit-tests/-/jobs/6798954987

Everything is fine in the previous patches (I pushed now the previous 5 
patches to the repo):

  https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/pipelines/1281919104

Could it be that he TIMEOUT gets messed up in certain cases?

  Thomas