diff mbox series

[kvm-unit-tests,v8,04/10] run_tests.sh: add --config option for alt test set

Message ID 20211118184650.661575-5-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series MTTCG sanity tests for ARM | expand

Commit Message

Alex Bennée Nov. 18, 2021, 6:46 p.m. UTC
The upcoming MTTCG tests don't need to be run for normal KVM unit
tests so lets add the facility to have a custom set of tests.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 run_tests.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Andrew Jones Nov. 24, 2021, 4:48 p.m. UTC | #1
On Thu, Nov 18, 2021 at 06:46:44PM +0000, Alex Bennée wrote:
> The upcoming MTTCG tests don't need to be run for normal KVM unit
> tests so lets add the facility to have a custom set of tests.

I think an environment variable override would be better than this command
line override, because then we could also get mkstandalone to work with
the new unittests.cfg files. Or, it may be better to just add them to
the main unittests.cfg with lines like these

groups = nodefault mttcg
accel = tcg

That'll "dirty" the logs with SKIP ... (test marked as manual run only)
for each one, but at least we won't easily forget about running them from
time to time.

Thanks,
drew


> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  run_tests.sh | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 9f233c5..b1088d2 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -15,7 +15,7 @@ function usage()
>  {
>  cat <<EOF
>  
> -Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t]
> +Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t] [-c CONFIG]
>  
>      -h, --help      Output this help text
>      -v, --verbose   Enables verbose mode
> @@ -24,6 +24,7 @@ Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t]
>      -g, --group     Only execute tests in the given group
>      -j, --parallel  Execute tests in parallel
>      -t, --tap13     Output test results in TAP format
> +    -c, --config    Override default unittests.cfg
>  
>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
>  specify the appropriate qemu binary for ARCH-run.
> @@ -42,7 +43,7 @@ if [ $? -ne 4 ]; then
>  fi
>  
>  only_tests=""
> -args=$(getopt -u -o ag:htj:v -l all,group:,help,tap13,parallel:,verbose -- $*)
> +args=$(getopt -u -o ag:htj:vc: -l all,group:,help,tap13,parallel:,verbose,config: -- $*)
>  [ $? -ne 0 ] && exit 2;
>  set -- $args;
>  while [ $# -gt 0 ]; do
> @@ -73,6 +74,10 @@ while [ $# -gt 0 ]; do
>          -t | --tap13)
>              tap_output="yes"
>              ;;
> +        -c | --config)
> +            shift
> +            config=$1
> +            ;;
>          --)
>              ;;
>          *)
> @@ -152,7 +157,7 @@ function run_task()
>  
>  : ${unittest_log_dir:=logs}
>  : ${unittest_run_queues:=1}
> -config=$TEST_DIR/unittests.cfg
> +: ${config:=$TEST_DIR/unittests.cfg}
>  
>  rm -rf $unittest_log_dir.old
>  [ -d $unittest_log_dir ] && mv $unittest_log_dir $unittest_log_dir.old
> -- 
> 2.30.2
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Alex Bennée Dec. 1, 2021, 4:20 p.m. UTC | #2
Andrew Jones <drjones@redhat.com> writes:

> On Thu, Nov 18, 2021 at 06:46:44PM +0000, Alex Bennée wrote:
>> The upcoming MTTCG tests don't need to be run for normal KVM unit
>> tests so lets add the facility to have a custom set of tests.
>
> I think an environment variable override would be better than this command
> line override, because then we could also get mkstandalone to work with
> the new unittests.cfg files. Or, it may be better to just add them to
> the main unittests.cfg with lines like these
>
> groups = nodefault mttcg
> accel = tcg
>
> That'll "dirty" the logs with SKIP ... (test marked as manual run only)
> for each one, but at least we won't easily forget about running them from
> time to time.

So what is the meaning of accel here? Is it:

  - this test only runs on accel FOO

or

  - this test defaults to running on accel FOO

because while the tests are for TCG I want to run them on KVM (so I can
validate the test on real HW). If I have accel=tcg then:

  env ACCEL=kvm QEMU=$HOME/lsrc/qemu.git/builds/all/qemu-system-aarch64 ./run_tests.sh -g mttcg
  SKIP tlbflush-code::all_other (tcg only, but ACCEL=kvm)
  SKIP tlbflush-code::page_other (tcg only, but ACCEL=kvm)
  SKIP tlbflush-code::all_self (tcg only, but ACCEL=kvm)
  ...

so I can either drop the accel line and rely on nodefault to ensure it
doesn't run normally or make the env ACCEL processing less anal about
preventing me running TCG tests under KVM. What do you think?
Andrew Jones Dec. 1, 2021, 4:41 p.m. UTC | #3
On Wed, Dec 01, 2021 at 04:20:02PM +0000, Alex Bennée wrote:
> 
> Andrew Jones <drjones@redhat.com> writes:
> 
> > On Thu, Nov 18, 2021 at 06:46:44PM +0000, Alex Bennée wrote:
> >> The upcoming MTTCG tests don't need to be run for normal KVM unit
> >> tests so lets add the facility to have a custom set of tests.
> >
> > I think an environment variable override would be better than this command
> > line override, because then we could also get mkstandalone to work with
> > the new unittests.cfg files. Or, it may be better to just add them to
> > the main unittests.cfg with lines like these
> >
> > groups = nodefault mttcg
> > accel = tcg
> >
> > That'll "dirty" the logs with SKIP ... (test marked as manual run only)
> > for each one, but at least we won't easily forget about running them from
> > time to time.
> 
> So what is the meaning of accel here? Is it:
> 
>   - this test only runs on accel FOO
> 
> or
> 
>   - this test defaults to running on accel FOO
> 
> because while the tests are for TCG I want to run them on KVM (so I can
> validate the test on real HW). If I have accel=tcg then:
> 
>   env ACCEL=kvm QEMU=$HOME/lsrc/qemu.git/builds/all/qemu-system-aarch64 ./run_tests.sh -g mttcg
>   SKIP tlbflush-code::all_other (tcg only, but ACCEL=kvm)
>   SKIP tlbflush-code::page_other (tcg only, but ACCEL=kvm)
>   SKIP tlbflush-code::all_self (tcg only, but ACCEL=kvm)
>   ...
> 
> so I can either drop the accel line and rely on nodefault to ensure it
> doesn't run normally or make the env ACCEL processing less anal about
> preventing me running TCG tests under KVM. What do you think?

Just drop the 'accel = tcg' line. I only suggested it because I didn't
know you also wanted to run the MTTCG "specific" tests under KVM. Now,
that I do, I wonder why we wouldn't run them all the time, i.e. no
nodefault group? Do the tests not exercise enough hypervisor code to
be worth the energy used to run them?

Thanks,
drew
Alex Bennée Dec. 1, 2021, 5:07 p.m. UTC | #4
Andrew Jones <drjones@redhat.com> writes:

> On Wed, Dec 01, 2021 at 04:20:02PM +0000, Alex Bennée wrote:
>> 
>> Andrew Jones <drjones@redhat.com> writes:
>> 
>> > On Thu, Nov 18, 2021 at 06:46:44PM +0000, Alex Bennée wrote:
>> >> The upcoming MTTCG tests don't need to be run for normal KVM unit
>> >> tests so lets add the facility to have a custom set of tests.
>> >
>> > I think an environment variable override would be better than this command
>> > line override, because then we could also get mkstandalone to work with
>> > the new unittests.cfg files. Or, it may be better to just add them to
>> > the main unittests.cfg with lines like these
>> >
>> > groups = nodefault mttcg
>> > accel = tcg
>> >
>> > That'll "dirty" the logs with SKIP ... (test marked as manual run only)
>> > for each one, but at least we won't easily forget about running them from
>> > time to time.
>> 
>> So what is the meaning of accel here? Is it:
>> 
>>   - this test only runs on accel FOO
>> 
>> or
>> 
>>   - this test defaults to running on accel FOO
>> 
>> because while the tests are for TCG I want to run them on KVM (so I can
>> validate the test on real HW). If I have accel=tcg then:
>> 
>>   env ACCEL=kvm QEMU=$HOME/lsrc/qemu.git/builds/all/qemu-system-aarch64 ./run_tests.sh -g mttcg
>>   SKIP tlbflush-code::all_other (tcg only, but ACCEL=kvm)
>>   SKIP tlbflush-code::page_other (tcg only, but ACCEL=kvm)
>>   SKIP tlbflush-code::all_self (tcg only, but ACCEL=kvm)
>>   ...
>> 
>> so I can either drop the accel line and rely on nodefault to ensure it
>> doesn't run normally or make the env ACCEL processing less anal about
>> preventing me running TCG tests under KVM. What do you think?
>
> Just drop the 'accel = tcg' line. I only suggested it because I didn't
> know you also wanted to run the MTTCG "specific" tests under KVM. Now,
> that I do, I wonder why we wouldn't run them all the time, i.e. no
> nodefault group? Do the tests not exercise enough hypervisor code to
> be worth the energy used to run them?

I think in most cases if they fail under KVM it wouldn't be due to the
hypervisor being broken but the silicon not meeting it's architectural
specification. I'm fine with them being nodefault for that.

I'm not sure how much the tlbflush code exercises on the host. There is
a WIP tlbflush-data which might make a case for being run more
regularly on KVM.

>
> Thanks,
> drew
diff mbox series

Patch

diff --git a/run_tests.sh b/run_tests.sh
index 9f233c5..b1088d2 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -15,7 +15,7 @@  function usage()
 {
 cat <<EOF
 
-Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t]
+Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t] [-c CONFIG]
 
     -h, --help      Output this help text
     -v, --verbose   Enables verbose mode
@@ -24,6 +24,7 @@  Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t]
     -g, --group     Only execute tests in the given group
     -j, --parallel  Execute tests in parallel
     -t, --tap13     Output test results in TAP format
+    -c, --config    Override default unittests.cfg
 
 Set the environment variable QEMU=/path/to/qemu-system-ARCH to
 specify the appropriate qemu binary for ARCH-run.
@@ -42,7 +43,7 @@  if [ $? -ne 4 ]; then
 fi
 
 only_tests=""
-args=$(getopt -u -o ag:htj:v -l all,group:,help,tap13,parallel:,verbose -- $*)
+args=$(getopt -u -o ag:htj:vc: -l all,group:,help,tap13,parallel:,verbose,config: -- $*)
 [ $? -ne 0 ] && exit 2;
 set -- $args;
 while [ $# -gt 0 ]; do
@@ -73,6 +74,10 @@  while [ $# -gt 0 ]; do
         -t | --tap13)
             tap_output="yes"
             ;;
+        -c | --config)
+            shift
+            config=$1
+            ;;
         --)
             ;;
         *)
@@ -152,7 +157,7 @@  function run_task()
 
 : ${unittest_log_dir:=logs}
 : ${unittest_run_queues:=1}
-config=$TEST_DIR/unittests.cfg
+: ${config:=$TEST_DIR/unittests.cfg}
 
 rm -rf $unittest_log_dir.old
 [ -d $unittest_log_dir ] && mv $unittest_log_dir $unittest_log_dir.old