[kvm-unit-tests PATCH 0/6] run_tests fixes and new functionality prep

[kvm-unit-tests PATCH 0/6] run_tests fixes and new functionality prep

[Andrew Jones]

It's time to revive up my "run_tests.sh changes"[*] series from a month ago.
Thanks to Radim's rework of mkstandalone it should be cleaner and easier this
time around. We need to fixup a couple things from that rework and start
preparing to share functions among arch run scripts. That's what this series
does.

Thanks,
drew

[*] https://www.mail-archive.com/kvm@vger.kernel.org/msg124375.html



Andrew Jones (6):
  run_tests.sh: a few rewrite fixups
  mkstandalone: fix generation for arm
  arch-run: DRYRUN is no longer needed, kill it
  arm/run: allow tests to run in AArch32 mode
  runtime: move getopts to run_tests.sh
  runtime: make the MAX_SMP loop a function

 arm/run                 | 42 ++++++++++++++--------------------
 run_tests.sh            | 43 +++++++++++++++++++++++++++++++++--
 scripts/mkstandalone.sh | 12 +++++++++-
 scripts/runtime.bash    | 60 +++++++++++++++----------------------------------
 x86/run                 | 10 ++++-----
 5 files changed, 91 insertions(+), 76 deletions(-)

-- 
2.4.3

[kvm-unit-tests PATCH 1/6] run_tests.sh: a few rewrite fixups

[Andrew Jones]

1) don't create a test.log file in every CWD that a standalone
   test gets run from.
2) keep '>> test.log' from interfering with the MAX_SMP loop.
3) replace ./$TEST_DIR-run with $RUNTIME_arch_run in the MAX_SMP
   loop.

Also fixes the MAX_SMP loop for later QEMU. The 'exceeds max...'
string needed updating to have capital CPU. We change that, but
also just add '-i' to grep to keep it working on older QEMU. The
comment for the MAX_SMP loop is also updated to better explain
it and hint at its eventual demise.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 run_tests.sh         |  5 +++--
 scripts/runtime.bash | 15 ++++++++++-----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index b488949d219b0..2312e031482a4 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -7,9 +7,10 @@ fi
 source config.mak
 source scripts/functions.bash
 
-RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
+RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
 
+RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
 config=$TEST_DIR/unittests.cfg
-
+echo > test.log
 for_each_unittest $config run
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 180746a70a8d9..fc878f99c977f 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -74,7 +74,6 @@ specify the appropriate qemu binary for ARCH-run.
 EOF
 }
 
-echo > test.log
 while getopts "g:hv" opt; do
     case $opt in
         g)
@@ -93,11 +92,17 @@ while getopts "g:hv" opt; do
     esac
 done
 
+MAX_SMP=$(getconf _NPROCESSORS_CONF)
 #
-# Probe for MAX_SMP
+# Probe for MAX_SMP, in case it's less than the number of host cpus.
 #
-MAX_SMP=$(getconf _NPROCESSORS_CONF)
-while ./$TEST_DIR-run _NO_FILE_4Uhere_ -smp $MAX_SMP \
-		|& grep -q 'exceeds max cpus'; do
+# This probing currently only works for ARM, as x86 bails on another
+# error first. Also, this probing isn't necessary for any ARM hosts
+# running kernels later than v4.3, i.e. those including ef748917b52
+# "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some
+# point when maintaining the while loop gets too tiresome, we can
+# just remove it...
+while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
+		|& grep -qi 'exceeds max CPUs'; do
 	((--MAX_SMP))
 done
-- 
2.4.3

[kvm-unit-tests PATCH 2/6] mkstandalone: fix generation for arm

[Andrew Jones]

The mkstandalone rewrite now packages the architecture run scripts.
This brings many benefits, but at the cost of no longer being able
to source additional files from the run script, at least not while
in standalone mode. We can live with that, but arm/run was sourcing
config.mak, so we need to stop that in standalone mode, as well as
to provide the standalone arm/run script what it needs.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/run                 | 10 ++++++----
 scripts/mkstandalone.sh | 11 ++++++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arm/run b/arm/run
index 4a648697d7fb5..0158ca5649a9f 100755
--- a/arm/run
+++ b/arm/run
@@ -1,10 +1,12 @@
 #!/bin/bash
 
-if [ ! -f config.mak ]; then
-	echo run ./configure first. See ./configure -h
-	exit 2
+if [ -z "$STANDALONE" ]; then
+	if [ ! -f config.mak ]; then
+		echo "run ./configure && make first. See ./configure -h"
+		exit 2
+	fi
+	source config.mak
 fi
-source config.mak
 processor="$PROCESSOR"
 
 if [ -c /dev/kvm ]; then
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 58910059a3559..b0f1e7c098afb 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -29,12 +29,21 @@ temp_file ()
 	echo "chmod +x \$$var"
 }
 
+config_export ()
+{
+	echo "export $(grep ^${1}= config.mak)"
+}
+
 generate_test ()
 {
 	local args=( $(escape "${@}") )
 
 	echo "#!/bin/bash"
-	grep '^ARCH=' config.mak
+	echo "export STANDALONE=yes"
+	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/')"
+	config_export ARCH
+	config_export ARCH_NAME
+	config_export PROCESSOR
 
 	if [ ! -f $kernel ]; then
 		echo 'echo "skip '"$testname"' (test kernel not present)"'
-- 
2.4.3

[kvm-unit-tests PATCH 3/6] arch-run: DRYRUN is no longer needed, kill it

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/run | 31 +++++++++----------------------
 x86/run | 10 ++++------
 2 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/arm/run b/arm/run
index 0158ca5649a9f..654a17c8f69c2 100755
--- a/arm/run
+++ b/arm/run
@@ -17,32 +17,21 @@ if [ -c /dev/kvm ]; then
 	fi
 fi
 
-if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ] &&
-		[ "$DRYRUN" != "yes" ]; then
-	printf "skip $TESTNAME (kvm only)\n\n"
+if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
+	echo "skip $TESTNAME (kvm only)"
 	exit 2
 fi
 
 if [ -z "$ACCEL" ]; then
-	if [ "$DRYRUN" = "yes" ]; then
-		# Output kvm with tcg fallback for dryrun (when both are
-		# allowed), since the command line we output may get used
-		# elsewhere.
-		ACCEL="kvm:tcg"
-	elif [ "$kvm_available" = "yes" ]; then
+	if [ "$kvm_available" = "yes" ]; then
 		ACCEL="kvm"
 	else
 		ACCEL="tcg"
 	fi
 fi
 
-if [ "$ARCH" = "arm64" ]; then
-	if [[ $ACCEL =~ kvm ]]; then
-		# arm64 must use '-cpu host' with kvm, and we can't use
-		# '-cpu host' with tcg, so we force kvm-only (no fallback)
-		ACCEL="kvm"
-		processor="host"
-	fi
+if [ "$ARCH" = "arm64" ] && [ "$ACCEL" = "kvm" ]; then
+	processor="host"
 fi
 
 qemu="${QEMU:-qemu-system-$ARCH_NAME}"
@@ -79,9 +68,7 @@ command="$qemu $M -cpu $processor $chr_testdev"
 command+=" -display none -serial stdio -kernel"
 echo $command "$@"
 
-if [ "$DRYRUN" != "yes" ]; then
-	$command "$@"
-	ret=$?
-	echo Return value from qemu: $ret
-	exit $ret
-fi
+$command "$@"
+ret=$?
+echo Return value from qemu: $ret
+exit $ret
diff --git a/x86/run b/x86/run
index 39a7cb92115cb..bdd7c7e394e45 100755
--- a/x86/run
+++ b/x86/run
@@ -45,9 +45,7 @@ fi
 command="${qemu} -enable-kvm $pc_testdev -vnc none -serial stdio $pci_testdev $hyperv_testdev -kernel"
 echo ${command} "$@"
 
-if [ "$DRYRUN" != "yes" ]; then
-	${command} "$@"
-	ret=$?
-	echo Return value from qemu: $ret
-	exit $ret
-fi
+${command} "$@"
+ret=$?
+echo Return value from qemu: $ret
+exit $ret
-- 
2.4.3

[kvm-unit-tests PATCH 4/6] arm/run: allow tests to run in AArch32 mode

[Andrew Jones]

With this and the "mkstandalone: fix generation for arm" patch, we
can now send an arm standalone test to an aarch64 host and run it.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/run | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arm/run b/arm/run
index 654a17c8f69c2..dc0a33204c20b 100755
--- a/arm/run
+++ b/arm/run
@@ -30,8 +30,11 @@ if [ -z "$ACCEL" ]; then
 	fi
 fi
 
-if [ "$ARCH" = "arm64" ] && [ "$ACCEL" = "kvm" ]; then
+if [ "$HOST" = "aarch64" ] && [ "$ACCEL" = "kvm" ]; then
 	processor="host"
+	if [ "$ARCH" = "arm" ]; then
+		processor+=",aarch64=off"
+	fi
 fi
 
 qemu="${QEMU:-qemu-system-$ARCH_NAME}"
-- 
2.4.3

[kvm-unit-tests PATCH 5/6] runtime: move getopts to run_tests.sh

[Andrew Jones]

We don't need to check options in standalone tests, kick the
loop and usage() function out to run_tests.sh

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 run_tests.sh         | 36 ++++++++++++++++++++++++++++++++++++
 scripts/runtime.bash | 37 +------------------------------------
 2 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index 2312e031482a4..558b8e7431d8e 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+verbose="no"
+
 if [ ! -f config.mak ]; then
     echo "run ./configure && make first. See ./configure -h"
     exit
@@ -7,9 +9,43 @@ fi
 source config.mak
 source scripts/functions.bash
 
+function usage()
+{
+cat <<EOF
+
+Usage: $0 [-g group] [-h] [-v]
+
+    -g: Only execute tests in the given group
+    -h: Output this help text
+    -v: Enables verbose mode
+
+Set the environment variable QEMU=/path/to/qemu-system-ARCH to
+specify the appropriate qemu binary for ARCH-run.
+
+EOF
+}
+
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
 
+while getopts "g:hv" opt; do
+    case $opt in
+        g)
+            only_group=$OPTARG
+            ;;
+        h)
+            usage
+            exit
+            ;;
+        v)
+            verbose="yes"
+            ;;
+        *)
+            exit
+            ;;
+    esac
+done
+
 RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
 config=$TEST_DIR/unittests.cfg
 echo > test.log
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index fc878f99c977f..63d1b9653007b 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -1,7 +1,6 @@
 : "${RUNTIME_arch_run?}"
 
 qemu=${QEMU:-qemu-system-$ARCH}
-verbose=0
 
 function run()
 {
@@ -40,7 +39,7 @@ function run()
     done
 
     cmdline="TESTNAME=$testname ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
-    if [ $verbose != 0 ]; then
+    if [ "$verbose" = "yes" ]; then
         echo $cmdline
     fi
 
@@ -58,40 +57,6 @@ function run()
     return $ret
 }
 
-function usage()
-{
-cat <<EOF
-
-Usage: $0 [-g group] [-h] [-v]
-
-    -g: Only execute tests in the given group
-    -h: Output this help text
-    -v: Enables verbose mode
-
-Set the environment variable QEMU=/path/to/qemu-system-ARCH to
-specify the appropriate qemu binary for ARCH-run.
-
-EOF
-}
-
-while getopts "g:hv" opt; do
-    case $opt in
-        g)
-            only_group=$OPTARG
-            ;;
-        h)
-            usage
-            exit
-            ;;
-        v)
-            verbose=1
-            ;;
-        *)
-            exit
-            ;;
-    esac
-done
-
 MAX_SMP=$(getconf _NPROCESSORS_CONF)
 #
 # Probe for MAX_SMP, in case it's less than the number of host cpus.
-- 
2.4.3

[kvm-unit-tests PATCH 6/6] runtime: make the MAX_SMP loop a function

[Andrew Jones]

We can now source scripts/runtime.bash multiple times without
always running the MAX_SMP loop. runtime.bash is now ready to
be sourced by arch run scripts (when not running in standalone
mode). Patches coming soon will add a couple of arch-neutral
functions to the arch run scripts.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 run_tests.sh            |  2 ++
 scripts/mkstandalone.sh |  1 +
 scripts/runtime.bash    | 12 +++++++++---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index 558b8e7431d8e..c0c5a72b1ceee 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -28,6 +28,8 @@ EOF
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
 
+probe_max_smp
+
 while getopts "g:hv" opt; do
     case $opt in
         g)
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index b0f1e7c098afb..c31a5f10519a1 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -60,6 +60,7 @@ generate_test ()
 
 	cat scripts/runtime.bash
 
+	echo probe_max_smp
 	echo "run ${args[@]}"
 }
 
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 63d1b9653007b..dd700f24cffa3 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -1,5 +1,9 @@
 : "${RUNTIME_arch_run?}"
 
+if [ -z "$MAX_SMP" ]; then
+    MAX_SMP=$(getconf _NPROCESSORS_CONF)
+fi
+
 qemu=${QEMU:-qemu-system-$ARCH}
 
 function run()
@@ -57,7 +61,6 @@ function run()
     return $ret
 }
 
-MAX_SMP=$(getconf _NPROCESSORS_CONF)
 #
 # Probe for MAX_SMP, in case it's less than the number of host cpus.
 #
@@ -67,7 +70,10 @@ MAX_SMP=$(getconf _NPROCESSORS_CONF)
 # "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some
 # point when maintaining the while loop gets too tiresome, we can
 # just remove it...
-while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
+function probe_max_smp()
+{
+    while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
 		|& grep -qi 'exceeds max CPUs'; do
 	((--MAX_SMP))
-done
+    done
+}
-- 
2.4.3

Re: [kvm-unit-tests PATCH 0/6] run_tests fixes and new functionality prep

[Radim Krčmář]

2016-01-22 15:58+0100, Andrew Jones:
> It's time to revive up my "run_tests.sh changes"[*] series from a month ago.
> Thanks to Radim's rework of mkstandalone it should be cleaner and easier this
> time around. We need to fixup a couple things from that rework and start
> preparing to share functions among arch run scripts. That's what this series
> does.

Great, thanks.  I see some petty improvements, but as I already added
made you do extra work ...

Reviwed-by: Radim Krčmář <rkrcmar@redhat.com>

(I swear I had most of [1/6] of in the original patch ...
 I'll pay attention to diff when splitting. :])

Re: [kvm-unit-tests PATCH 2/6] mkstandalone: fix generation for arm

[Paolo Bonzini]

On 22/01/2016 15:58, Andrew Jones wrote:
> The mkstandalone rewrite now packages the architecture run scripts.
> This brings many benefits, but at the cost of no longer being able
> to source additional files from the run script, at least not while
> in standalone mode. We can live with that, but arm/run was sourcing
> config.mak, so we need to stop that in standalone mode, as well as
> to provide the standalone arm/run script what it needs.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arm/run                 | 10 ++++++----
>  scripts/mkstandalone.sh | 11 ++++++++++-
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/arm/run b/arm/run
> index 4a648697d7fb5..0158ca5649a9f 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -1,10 +1,12 @@
>  #!/bin/bash
>  
> -if [ ! -f config.mak ]; then
> -	echo run ./configure first. See ./configure -h
> -	exit 2
> +if [ -z "$STANDALONE" ]; then

What do you think about checking:

    if [ "${ARCH:+set}" != set ]; then

instead?

Paolo

> +	if [ ! -f config.mak ]; then
> +		echo "run ./configure && make first. See ./configure -h"
> +		exit 2
> +	fi
> +	source config.mak
>  fi
> -source config.mak
>  processor="$PROCESSOR"
>  

Re: [kvm-unit-tests PATCH 5/6] runtime: move getopts to run_tests.sh

[Paolo Bonzini]

On 22/01/2016 15:58, Andrew Jones wrote:
> We don't need to check options in standalone tests, kick the
> loop and usage() function out to run_tests.sh
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  run_tests.sh         | 36 ++++++++++++++++++++++++++++++++++++
>  scripts/runtime.bash | 37 +------------------------------------
>  2 files changed, 37 insertions(+), 36 deletions(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 2312e031482a4..558b8e7431d8e 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -1,5 +1,7 @@
>  #!/bin/bash
>  
> +verbose="no"
> +
>  if [ ! -f config.mak ]; then
>      echo "run ./configure && make first. See ./configure -h"
>      exit
> @@ -7,9 +9,43 @@ fi
>  source config.mak
>  source scripts/functions.bash
>  
> +function usage()
> +{
> +cat <<EOF
> +
> +Usage: $0 [-g group] [-h] [-v]
> +
> +    -g: Only execute tests in the given group
> +    -h: Output this help text
> +    -v: Enables verbose mode
> +
> +Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> +specify the appropriate qemu binary for ARCH-run.
> +
> +EOF
> +}
> +
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
>  
> +while getopts "g:hv" opt; do
> +    case $opt in
> +        g)
> +            only_group=$OPTARG
> +            ;;
> +        h)
> +            usage
> +            exit
> +            ;;
> +        v)
> +            verbose="yes"
> +            ;;
> +        *)
> +            exit
> +            ;;
> +    esac
> +done
> +
>  RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
>  config=$TEST_DIR/unittests.cfg
>  echo > test.log
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index fc878f99c977f..63d1b9653007b 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -1,7 +1,6 @@
>  : "${RUNTIME_arch_run?}"
>  
>  qemu=${QEMU:-qemu-system-$ARCH}
> -verbose=0
>  
>  function run()
>  {
> @@ -40,7 +39,7 @@ function run()
>      done
>  
>      cmdline="TESTNAME=$testname ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> -    if [ $verbose != 0 ]; then
> +    if [ "$verbose" = "yes" ]; then
>          echo $cmdline
>      fi
>  
> @@ -58,40 +57,6 @@ function run()
>      return $ret
>  }
>  
> -function usage()
> -{
> -cat <<EOF
> -
> -Usage: $0 [-g group] [-h] [-v]
> -
> -    -g: Only execute tests in the given group
> -    -h: Output this help text
> -    -v: Enables verbose mode
> -
> -Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> -specify the appropriate qemu binary for ARCH-run.
> -
> -EOF
> -}
> -
> -while getopts "g:hv" opt; do
> -    case $opt in
> -        g)
> -            only_group=$OPTARG
> -            ;;
> -        h)
> -            usage
> -            exit
> -            ;;
> -        v)
> -            verbose=1
> -            ;;
> -        *)
> -            exit
> -            ;;
> -    esac
> -done
> -
>  MAX_SMP=$(getconf _NPROCESSORS_CONF)
>  #
>  # Probe for MAX_SMP, in case it's less than the number of host cpus.
> 

Should standalone tests always set verbose=1?

Paolo

Re: [kvm-unit-tests PATCH 6/6] runtime: make the MAX_SMP loop a function

[Paolo Bonzini]

On 22/01/2016 15:58, Andrew Jones wrote:
> We can now source scripts/runtime.bash multiple times without
> always running the MAX_SMP loop. runtime.bash is now ready to
> be sourced by arch run scripts (when not running in standalone
> mode). Patches coming soon will add a couple of arch-neutral
> functions to the arch run scripts.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  run_tests.sh            |  2 ++
>  scripts/mkstandalone.sh |  1 +
>  scripts/runtime.bash    | 12 +++++++++---
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 558b8e7431d8e..c0c5a72b1ceee 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -28,6 +28,8 @@ EOF
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
>  
> +probe_max_smp
> +
>  while getopts "g:hv" opt; do
>      case $opt in
>          g)
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index b0f1e7c098afb..c31a5f10519a1 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -60,6 +60,7 @@ generate_test ()
>  
>  	cat scripts/runtime.bash
>  
> +	echo probe_max_smp
>  	echo "run ${args[@]}"
>  }
>  
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 63d1b9653007b..dd700f24cffa3 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -1,5 +1,9 @@
>  : "${RUNTIME_arch_run?}"
>  
> +if [ -z "$MAX_SMP" ]; then
> +    MAX_SMP=$(getconf _NPROCESSORS_CONF)
> +fi
> +
>  qemu=${QEMU:-qemu-system-$ARCH}
>  
>  function run()
> @@ -57,7 +61,6 @@ function run()
>      return $ret
>  }
>  
> -MAX_SMP=$(getconf _NPROCESSORS_CONF)
>  #
>  # Probe for MAX_SMP, in case it's less than the number of host cpus.
>  #
> @@ -67,7 +70,10 @@ MAX_SMP=$(getconf _NPROCESSORS_CONF)
>  # "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some
>  # point when maintaining the while loop gets too tiresome, we can
>  # just remove it...
> -while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
> +function probe_max_smp()
> +{
> +    while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
>  		|& grep -qi 'exceeds max CPUs'; do
>  	((--MAX_SMP))
> -done
> +    done
> +}
> 

This kinda goes against the idea of removing duplication between
runtests and mkstandalone...

I wonder if you should instead remove the

    : "${RUNTIME_arch_run?}"

and conditionalize the MAX_SMP test on the existence of
RUNTIME_arch_run.  Is it set during execution of the arch run scripts?

Paolo

Re: [kvm-unit-tests PATCH 0/6] run_tests fixes and new functionality prep

[Paolo Bonzini]

On 22/01/2016 16:29, Radim Krčmář wrote:
> 2016-01-22 15:58+0100, Andrew Jones:
>> It's time to revive up my "run_tests.sh changes"[*] series from a month ago.
>> Thanks to Radim's rework of mkstandalone it should be cleaner and easier this
>> time around. We need to fixup a couple things from that rework and start
>> preparing to share functions among arch run scripts. That's what this series
>> does.
> 
> Great, thanks.  I see some petty improvements, but as I already added
> made you do extra work ...
> 
> Reviwed-by: Radim Krčmář <rkrcmar@redhat.com>

Thanks.  For now, I'm applying 1-4.

Paolo

> (I swear I had most of [1/6] of in the original patch ...
>  I'll pay attention to diff when splitting. :])
> 

Re: [kvm-unit-tests PATCH 2/6] mkstandalone: fix generation for arm

[Andrew Jones]

On Tue, Jan 26, 2016 at 12:01:17PM +0100, Paolo Bonzini wrote:
> 
> 
> On 22/01/2016 15:58, Andrew Jones wrote:
> > The mkstandalone rewrite now packages the architecture run scripts.
> > This brings many benefits, but at the cost of no longer being able
> > to source additional files from the run script, at least not while
> > in standalone mode. We can live with that, but arm/run was sourcing
> > config.mak, so we need to stop that in standalone mode, as well as
> > to provide the standalone arm/run script what it needs.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  arm/run                 | 10 ++++++----
> >  scripts/mkstandalone.sh | 11 ++++++++++-
> >  2 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arm/run b/arm/run
> > index 4a648697d7fb5..0158ca5649a9f 100755
> > --- a/arm/run
> > +++ b/arm/run
> > @@ -1,10 +1,12 @@
> >  #!/bin/bash
> >  
> > -if [ ! -f config.mak ]; then
> > -	echo run ./configure first. See ./configure -h
> > -	exit 2
> > +if [ -z "$STANDALONE" ]; then
> 
> What do you think about checking:
> 
>     if [ "${ARCH:+set}" != set ]; then
> 
> instead?

That's a nice trick, but I sort of prefer the explicit STANDALONE
variable, and I see you already merged this version.

Thanks,
drew

Re: [kvm-unit-tests PATCH 5/6] runtime: move getopts to run_tests.sh

[Andrew Jones]

On Tue, Jan 26, 2016 at 12:06:46PM +0100, Paolo Bonzini wrote:
> 
> 
> On 22/01/2016 15:58, Andrew Jones wrote:
> > We don't need to check options in standalone tests, kick the
> > loop and usage() function out to run_tests.sh
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  run_tests.sh         | 36 ++++++++++++++++++++++++++++++++++++
> >  scripts/runtime.bash | 37 +------------------------------------
> >  2 files changed, 37 insertions(+), 36 deletions(-)
> > 
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 2312e031482a4..558b8e7431d8e 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -1,5 +1,7 @@
> >  #!/bin/bash
> >  
> > +verbose="no"
> > +
> >  if [ ! -f config.mak ]; then
> >      echo "run ./configure && make first. See ./configure -h"
> >      exit
> > @@ -7,9 +9,43 @@ fi
> >  source config.mak
> >  source scripts/functions.bash
> >  
> > +function usage()
> > +{
> > +cat <<EOF
> > +
> > +Usage: $0 [-g group] [-h] [-v]
> > +
> > +    -g: Only execute tests in the given group
> > +    -h: Output this help text
> > +    -v: Enables verbose mode
> > +
> > +Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> > +specify the appropriate qemu binary for ARCH-run.
> > +
> > +EOF
> > +}
> > +
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > +while getopts "g:hv" opt; do
> > +    case $opt in
> > +        g)
> > +            only_group=$OPTARG
> > +            ;;
> > +        h)
> > +            usage
> > +            exit
> > +            ;;
> > +        v)
> > +            verbose="yes"
> > +            ;;
> > +        *)
> > +            exit
> > +            ;;
> > +    esac
> > +done
> > +
> >  RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> >  config=$TEST_DIR/unittests.cfg
> >  echo > test.log
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index fc878f99c977f..63d1b9653007b 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -1,7 +1,6 @@
> >  : "${RUNTIME_arch_run?}"
> >  
> >  qemu=${QEMU:-qemu-system-$ARCH}
> > -verbose=0
> >  
> >  function run()
> >  {
> > @@ -40,7 +39,7 @@ function run()
> >      done
> >  
> >      cmdline="TESTNAME=$testname ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> > -    if [ $verbose != 0 ]; then
> > +    if [ "$verbose" = "yes" ]; then
> >          echo $cmdline
> >      fi
> >  
> > @@ -58,40 +57,6 @@ function run()
> >      return $ret
> >  }
> >  
> > -function usage()
> > -{
> > -cat <<EOF
> > -
> > -Usage: $0 [-g group] [-h] [-v]
> > -
> > -    -g: Only execute tests in the given group
> > -    -h: Output this help text
> > -    -v: Enables verbose mode
> > -
> > -Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> > -specify the appropriate qemu binary for ARCH-run.
> > -
> > -EOF
> > -}
> > -
> > -while getopts "g:hv" opt; do
> > -    case $opt in
> > -        g)
> > -            only_group=$OPTARG
> > -            ;;
> > -        h)
> > -            usage
> > -            exit
> > -            ;;
> > -        v)
> > -            verbose=1
> > -            ;;
> > -        *)
> > -            exit
> > -            ;;
> > -    esac
> > -done
> > -
> >  MAX_SMP=$(getconf _NPROCESSORS_CONF)
> >  #
> >  # Probe for MAX_SMP, in case it's less than the number of host cpus.
> > 
> 
> Should standalone tests always set verbose=1?

atm, no. standalone tests already always echo the command line, which
is currently the only thing verbose does. Actually, turning it on would
result in a redundant echo. So, if we ever expand the scope of verbose,
then we may need to revisit this some day, but I don't see much need now.

Thanks,
drew

Re: [kvm-unit-tests PATCH 6/6] runtime: make the MAX_SMP loop a function

[Andrew Jones]

On Tue, Jan 26, 2016 at 12:08:54PM +0100, Paolo Bonzini wrote:
> 
> 
> On 22/01/2016 15:58, Andrew Jones wrote:
> > We can now source scripts/runtime.bash multiple times without
> > always running the MAX_SMP loop. runtime.bash is now ready to
> > be sourced by arch run scripts (when not running in standalone
> > mode). Patches coming soon will add a couple of arch-neutral
> > functions to the arch run scripts.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  run_tests.sh            |  2 ++
> >  scripts/mkstandalone.sh |  1 +
> >  scripts/runtime.bash    | 12 +++++++++---
> >  3 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 558b8e7431d8e..c0c5a72b1ceee 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -28,6 +28,8 @@ EOF
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > +probe_max_smp
> > +
> >  while getopts "g:hv" opt; do
> >      case $opt in
> >          g)
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index b0f1e7c098afb..c31a5f10519a1 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -60,6 +60,7 @@ generate_test ()
> >  
> >  	cat scripts/runtime.bash
> >  
> > +	echo probe_max_smp
> >  	echo "run ${args[@]}"
> >  }
> >  
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index 63d1b9653007b..dd700f24cffa3 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -1,5 +1,9 @@
> >  : "${RUNTIME_arch_run?}"
> >  
> > +if [ -z "$MAX_SMP" ]; then
> > +    MAX_SMP=$(getconf _NPROCESSORS_CONF)
> > +fi
> > +
> >  qemu=${QEMU:-qemu-system-$ARCH}
> >  
> >  function run()
> > @@ -57,7 +61,6 @@ function run()
> >      return $ret
> >  }
> >  
> > -MAX_SMP=$(getconf _NPROCESSORS_CONF)
> >  #
> >  # Probe for MAX_SMP, in case it's less than the number of host cpus.
> >  #
> > @@ -67,7 +70,10 @@ MAX_SMP=$(getconf _NPROCESSORS_CONF)
> >  # "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some
> >  # point when maintaining the while loop gets too tiresome, we can
> >  # just remove it...
> > -while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
> > +function probe_max_smp()
> > +{
> > +    while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
> >  		|& grep -qi 'exceeds max CPUs'; do
> >  	((--MAX_SMP))
> > -done
> > +    done
> > +}
> > 
> 
> This kinda goes against the idea of removing duplication between
> runtests and mkstandalone...
> 
> I wonder if you should instead remove the
> 
>     : "${RUNTIME_arch_run?}"
> 
> and conditionalize the MAX_SMP test on the existence of
> RUNTIME_arch_run.  Is it set during execution of the arch run scripts?

Let's drop this patch. Originally I planned on adding more functions to
scripts/runtime.bash, in my next series (coming soon), which would require
the arch-run scripts to source scripts/runtime.bash. Now, I think it's
cleaner to just create a new file to source, scripts/arch-run.bash.

Thanks,
drew

Re: [kvm-unit-tests PATCH 6/6] runtime: make the MAX_SMP loop a function

[Paolo Bonzini]

 26/01/2016 13:28, Andrew Jones wrote:
> On Tue, Jan 26, 2016 at 12:08:54PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 22/01/2016 15:58, Andrew Jones wrote:
>>> We can now source scripts/runtime.bash multiple times without
>>> always running the MAX_SMP loop. runtime.bash is now ready to
>>> be sourced by arch run scripts (when not running in standalone
>>> mode). Patches coming soon will add a couple of arch-neutral
>>> functions to the arch run scripts.
>>>
>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>> ---
>>>  run_tests.sh            |  2 ++
>>>  scripts/mkstandalone.sh |  1 +
>>>  scripts/runtime.bash    | 12 +++++++++---
>>>  3 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/run_tests.sh b/run_tests.sh
>>> index 558b8e7431d8e..c0c5a72b1ceee 100755
>>> --- a/run_tests.sh
>>> +++ b/run_tests.sh
>>> @@ -28,6 +28,8 @@ EOF
>>>  RUNTIME_arch_run="./$TEST_DIR/run"
>>>  source scripts/runtime.bash
>>>  
>>> +probe_max_smp
>>> +
>>>  while getopts "g:hv" opt; do
>>>      case $opt in
>>>          g)
>>> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
>>> index b0f1e7c098afb..c31a5f10519a1 100755
>>> --- a/scripts/mkstandalone.sh
>>> +++ b/scripts/mkstandalone.sh
>>> @@ -60,6 +60,7 @@ generate_test ()
>>>  
>>>  	cat scripts/runtime.bash
>>>  
>>> +	echo probe_max_smp
>>>  	echo "run ${args[@]}"
>>>  }
>>>  
>>> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
>>> index 63d1b9653007b..dd700f24cffa3 100644
>>> --- a/scripts/runtime.bash
>>> +++ b/scripts/runtime.bash
>>> @@ -1,5 +1,9 @@
>>>  : "${RUNTIME_arch_run?}"
>>>  
>>> +if [ -z "$MAX_SMP" ]; then
>>> +    MAX_SMP=$(getconf _NPROCESSORS_CONF)
>>> +fi
>>> +
>>>  qemu=${QEMU:-qemu-system-$ARCH}
>>>  
>>>  function run()
>>> @@ -57,7 +61,6 @@ function run()
>>>      return $ret
>>>  }
>>>  
>>> -MAX_SMP=$(getconf _NPROCESSORS_CONF)
>>>  #
>>>  # Probe for MAX_SMP, in case it's less than the number of host cpus.
>>>  #
>>> @@ -67,7 +70,10 @@ MAX_SMP=$(getconf _NPROCESSORS_CONF)
>>>  # "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some
>>>  # point when maintaining the while loop gets too tiresome, we can
>>>  # just remove it...
>>> -while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
>>> +function probe_max_smp()
>>> +{
>>> +    while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
>>>  		|& grep -qi 'exceeds max CPUs'; do
>>>  	((--MAX_SMP))
>>> -done
>>> +    done
>>> +}
>>>
>>
>> This kinda goes against the idea of removing duplication between
>> runtests and mkstandalone...
>>
>> I wonder if you should instead remove the
>>
>>     : "${RUNTIME_arch_run?}"
>>
>> and conditionalize the MAX_SMP test on the existence of
>> RUNTIME_arch_run.  Is it set during execution of the arch run scripts?
> 
> Let's drop this patch. Originally I planned on adding more functions to
> scripts/runtime.bash, in my next series (coming soon), which would require
> the arch-run scripts to source scripts/runtime.bash. Now, I think it's
> cleaner to just create a new file to source, scripts/arch-run.bash.

Good.  Applying 5 then.

Paolo