| Message ID | 20240504122841.1177683-8-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | powerpc improvements | expand |
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
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@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 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. Thanks, Nick
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@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 > > 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
On 08/05/2024 14.55, Thomas Huth wrote: > 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@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 >> >> 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... I guess you need to adjust arch_cmd_s390x in scripts/s390x/func.bash to be aware of the new parameter, too? Thomas
On 08/05/2024 14.58, Thomas Huth wrote: > On 08/05/2024 14.55, Thomas Huth wrote: >> 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@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 >>> >>> 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... > > I guess you need to adjust arch_cmd_s390x in scripts/s390x/func.bash to be > aware of the new parameter, too? This seems to fix the problem: diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash index fa47d019..6b817727 100644 --- a/scripts/s390x/func.bash +++ b/scripts/s390x/func.bash @@ -13,12 +13,13 @@ function arch_cmd_s390x() local kernel=$5 local opts=$6 local arch=$7 - local check=$8 - local accel=$9 - local timeout=${10} + local machine=$8 + local check=$9 + local accel=${10} + local timeout=${11} # run the normal test case - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" + "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout" # run PV test case if [ "$accel" = 'tcg' ] || grep -q "migration" <<< "$groups"; then If you don't like to respin, I can add it to the patch while picking it up? Thomas
On Wed May 8, 2024 at 11:36 PM AEST, Thomas Huth wrote: > On 08/05/2024 14.58, Thomas Huth wrote: > > On 08/05/2024 14.55, Thomas Huth wrote: > >> 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@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 > >>> > >>> 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... > > > > I guess you need to adjust arch_cmd_s390x in scripts/s390x/func.bash to be > > aware of the new parameter, too? > > This seems to fix the problem: Thanks, that looks good. > diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash > index fa47d019..6b817727 100644 > --- a/scripts/s390x/func.bash > +++ b/scripts/s390x/func.bash > @@ -13,12 +13,13 @@ function arch_cmd_s390x() > local kernel=$5 > local opts=$6 > local arch=$7 > - local check=$8 > - local accel=$9 > - local timeout=${10} > + local machine=$8 > + local check=$9 > + local accel=${10} > + local timeout=${11} > > # run the normal test case > - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" > + "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout" > > # run PV test case > if [ "$accel" = 'tcg' ] || grep -q "migration" <<< "$groups"; then > > If you don't like to respin, I can add it to the patch while picking it up? Yeah I shouldn't resend the full series just for this. If you're happy to squash it in that would be good. Thanks, Nick
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