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

[Thomas Huth]

On 08/05/2024 14.27, Nicholas Piggin wrote:
> On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:
>> 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 at redhat.com>
>>> Acked-by: Andrew Jones <andrew.jones at linux.dev>
>>> Signed-off-by: Nicholas Piggin <npiggin at 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
> 
> How do you use the s390x runner?
> 
>>
>> 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?
> 
> Hmm not sure yet. At least it got timeout right for the duration=90s
> message.

That seems to be wrong, the test is declared like this in s390x/unittests.cfg :

[sieve]
file = sieve.elf
groups = selftest
# can take fairly long when KVM is nested inside z/VM
timeout = 600

And indeed, it takes way longer than 90 seconds on that CI machine, so the 
timeout after 90 seconds should not occur here...

  Thomas