[kvm-unit-tests PATCH v5 0/2] run_tests: support concurrent test execution

[kvm-unit-tests PATCH v5 0/2] run_tests: support concurrent test execution

[Peter Xu]

v5:
- add "/" at start/end of line where proper [Drew]
- remove useless newline in Makefile [Drew]
- don't check "mv" since it won't fail [Drew]
- avoid using '"s in (( )) [Drew]
- comment fix [Drew]

v4:
- add .gitignore for logs/ [Drew]
- instead of create globals.bash, renaming function.bash into
  common.bash, put globals inside [Drew]
- instead of removing logs/ directly when start run_tests, move it
  into logs.old so we at least have the last time result cached [Drew]
- s/ut_/unittest_/ through the whole series [Drew]
- remove unittest_log_summary var [Drew]
- remove radim's s-b in patch 2 since it does not suite [Drew]
- tiny fix on the usage lines [Drew]
- use bash arithmetic where proper [Drew]
- remove ut_in_parallel since not used [Drew]

v3:
- better handling for ctrl-c during run_tests.sh [Radim]

v2:
- patch 1: do per-test logging in all cases
- patch 2: throw away task.bash, instead, take Radim's suggestion to
  use jobs

run_tests.sh is getting slower. Maybe it's time to let it run faster.
An obvious issue is that, we were running the tests sequentially in
the past.

This series provides another new "-j" parameter. "-j 8" means we run
the tests on 8 task queues. That'll fasten the script a lot. A very
quick test of mine shows 3x speed boost with 8 task queues.

Please review, thanks.

Peter Xu (2):
  run_tests: put logs into per-test file
  run_tests: allow run tests in parallel

 .gitignore                              |  3 ++-
 Makefile                                |  5 ++---
 run_tests.sh                            | 30 +++++++++++++++++++++---------
 scripts/{functions.bash => common.bash} | 27 +++++++++++++++++++++++++--
 scripts/mkstandalone.sh                 |  2 +-
 5 files changed, 51 insertions(+), 16 deletions(-)
 rename scripts/{functions.bash => common.bash} (63%)

-- 
2.7.4

[Qemu-devel] [kvm-unit-tests PATCH v5 0/2] run_tests: support concurrent test execution

[Peter Xu]

v5:
- add "/" at start/end of line where proper [Drew]
- remove useless newline in Makefile [Drew]
- don't check "mv" since it won't fail [Drew]
- avoid using '"s in (( )) [Drew]
- comment fix [Drew]

v4:
- add .gitignore for logs/ [Drew]
- instead of create globals.bash, renaming function.bash into
  common.bash, put globals inside [Drew]
- instead of removing logs/ directly when start run_tests, move it
  into logs.old so we at least have the last time result cached [Drew]
- s/ut_/unittest_/ through the whole series [Drew]
- remove unittest_log_summary var [Drew]
- remove radim's s-b in patch 2 since it does not suite [Drew]
- tiny fix on the usage lines [Drew]
- use bash arithmetic where proper [Drew]
- remove ut_in_parallel since not used [Drew]

v3:
- better handling for ctrl-c during run_tests.sh [Radim]

v2:
- patch 1: do per-test logging in all cases
- patch 2: throw away task.bash, instead, take Radim's suggestion to
  use jobs

run_tests.sh is getting slower. Maybe it's time to let it run faster.
An obvious issue is that, we were running the tests sequentially in
the past.

This series provides another new "-j" parameter. "-j 8" means we run
the tests on 8 task queues. That'll fasten the script a lot. A very
quick test of mine shows 3x speed boost with 8 task queues.

Please review, thanks.

Peter Xu (2):
  run_tests: put logs into per-test file
  run_tests: allow run tests in parallel

 .gitignore                              |  3 ++-
 Makefile                                |  5 ++---
 run_tests.sh                            | 30 +++++++++++++++++++++---------
 scripts/{functions.bash => common.bash} | 27 +++++++++++++++++++++++++--
 scripts/mkstandalone.sh                 |  2 +-
 5 files changed, 51 insertions(+), 16 deletions(-)
 rename scripts/{functions.bash => common.bash} (63%)

-- 
2.7.4

[kvm-unit-tests PATCH v5 1/2] run_tests: put logs into per-test file

[Peter Xu]

We were using test.log before to keep all the test logs. This patch
creates one log file per test case under logs/ directory with name
"TESTNAME.log". Meanwhile, we will keep the last time log into
logs.old/.

Renaming scripts/functions.bash into scripts/common.bash to store some
more global variables.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 .gitignore                              |  3 ++-
 Makefile                                |  5 ++---
 run_tests.sh                            | 18 +++++++++++-------
 scripts/{functions.bash => common.bash} | 13 +++++++++++--
 scripts/mkstandalone.sh                 |  2 +-
 5 files changed, 27 insertions(+), 14 deletions(-)
 rename scripts/{functions.bash => common.bash} (75%)

diff --git a/.gitignore b/.gitignore
index 3155418..2213b9b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,7 +12,8 @@ cscope.*
 /lib/asm
 /config.mak
 /*-run
-/test.log
 /msr.out
 /tests
 /build-head
+/logs/
+/logs.old/
diff --git a/Makefile b/Makefile
index a32333b..844bacc 100644
--- a/Makefile
+++ b/Makefile
@@ -94,9 +94,8 @@ libfdt_clean:
 	$(LIBFDT_objdir)/.*.d
 
 distclean: clean libfdt_clean
-	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
-	      build-head
-	$(RM) -r tests
+	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
+	$(RM) -r tests logs logs.old
 
 cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
 cscope:
diff --git a/run_tests.sh b/run_tests.sh
index 254129d..b6a1059 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
     exit 1
 fi
 source config.mak
-source scripts/functions.bash
+source scripts/common.bash
 
 function usage()
 {
@@ -46,17 +46,21 @@ while getopts "g:hv" opt; do
     esac
 done
 
-RUNTIME_log_stderr () { cat >> test.log; }
+# RUNTIME_log_file will be configured later
+RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
 RUNTIME_log_stdout () {
     if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
-        ./scripts/pretty_print_stacks.py $1 >> test.log
+        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
     else
-        cat >> test.log
+        cat >> $RUNTIME_log_file
     fi
 }
 
-
 config=$TEST_DIR/unittests.cfg
-rm -f test.log
-printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
+
+rm -rf $unittest_log_dir.old
+mv $unittest_log_dir $unittest_log_dir.old
+mkdir $unittest_log_dir
+echo "BUILD_HEAD=$(cat build-head)" > $unittest_log_dir/SUMMARY
+
 for_each_unittest $config run
diff --git a/scripts/functions.bash b/scripts/common.bash
similarity index 75%
rename from scripts/functions.bash
rename to scripts/common.bash
index ee9143c..2dd7360 100644
--- a/scripts/functions.bash
+++ b/scripts/common.bash
@@ -1,3 +1,12 @@
+: ${unittest_log_dir:=logs}
+
+function run_task()
+{
+	local testname="$2"
+
+	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
+	"$@"
+}
 
 function for_each_unittest()
 {
@@ -17,7 +26,7 @@ function for_each_unittest()
 
 	while read -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
-			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+			run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 			testname=${BASH_REMATCH[1]}
 			smp=1
 			kernel=""
@@ -45,6 +54,6 @@ function for_each_unittest()
 			timeout=${BASH_REMATCH[1]}
 		fi
 	done
-	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 	exec {fd}<&-
 }
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index d2bae19..3c1938e 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -5,7 +5,7 @@ if [ ! -f config.mak ]; then
 	exit 1
 fi
 source config.mak
-source scripts/functions.bash
+source scripts/common.bash
 
 escape ()
 {
-- 
2.7.4

[Qemu-devel] [kvm-unit-tests PATCH v5 1/2] run_tests: put logs into per-test file

[Peter Xu]

We were using test.log before to keep all the test logs. This patch
creates one log file per test case under logs/ directory with name
"TESTNAME.log". Meanwhile, we will keep the last time log into
logs.old/.

Renaming scripts/functions.bash into scripts/common.bash to store some
more global variables.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 .gitignore                              |  3 ++-
 Makefile                                |  5 ++---
 run_tests.sh                            | 18 +++++++++++-------
 scripts/{functions.bash => common.bash} | 13 +++++++++++--
 scripts/mkstandalone.sh                 |  2 +-
 5 files changed, 27 insertions(+), 14 deletions(-)
 rename scripts/{functions.bash => common.bash} (75%)

diff --git a/.gitignore b/.gitignore
index 3155418..2213b9b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,7 +12,8 @@ cscope.*
 /lib/asm
 /config.mak
 /*-run
-/test.log
 /msr.out
 /tests
 /build-head
+/logs/
+/logs.old/
diff --git a/Makefile b/Makefile
index a32333b..844bacc 100644
--- a/Makefile
+++ b/Makefile
@@ -94,9 +94,8 @@ libfdt_clean:
 	$(LIBFDT_objdir)/.*.d
 
 distclean: clean libfdt_clean
-	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
-	      build-head
-	$(RM) -r tests
+	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
+	$(RM) -r tests logs logs.old
 
 cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
 cscope:
diff --git a/run_tests.sh b/run_tests.sh
index 254129d..b6a1059 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
     exit 1
 fi
 source config.mak
-source scripts/functions.bash
+source scripts/common.bash
 
 function usage()
 {
@@ -46,17 +46,21 @@ while getopts "g:hv" opt; do
     esac
 done
 
-RUNTIME_log_stderr () { cat >> test.log; }
+# RUNTIME_log_file will be configured later
+RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
 RUNTIME_log_stdout () {
     if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
-        ./scripts/pretty_print_stacks.py $1 >> test.log
+        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
     else
-        cat >> test.log
+        cat >> $RUNTIME_log_file
     fi
 }
 
-
 config=$TEST_DIR/unittests.cfg
-rm -f test.log
-printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
+
+rm -rf $unittest_log_dir.old
+mv $unittest_log_dir $unittest_log_dir.old
+mkdir $unittest_log_dir
+echo "BUILD_HEAD=$(cat build-head)" > $unittest_log_dir/SUMMARY
+
 for_each_unittest $config run
diff --git a/scripts/functions.bash b/scripts/common.bash
similarity index 75%
rename from scripts/functions.bash
rename to scripts/common.bash
index ee9143c..2dd7360 100644
--- a/scripts/functions.bash
+++ b/scripts/common.bash
@@ -1,3 +1,12 @@
+: ${unittest_log_dir:=logs}
+
+function run_task()
+{
+	local testname="$2"
+
+	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
+	"$@"
+}
 
 function for_each_unittest()
 {
@@ -17,7 +26,7 @@ function for_each_unittest()
 
 	while read -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
-			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+			run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 			testname=${BASH_REMATCH[1]}
 			smp=1
 			kernel=""
@@ -45,6 +54,6 @@ function for_each_unittest()
 			timeout=${BASH_REMATCH[1]}
 		fi
 	done
-	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 	exec {fd}<&-
 }
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index d2bae19..3c1938e 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -5,7 +5,7 @@ if [ ! -f config.mak ]; then
 	exit 1
 fi
 source config.mak
-source scripts/functions.bash
+source scripts/common.bash
 
 escape ()
 {
-- 
2.7.4

[kvm-unit-tests PATCH v5 2/2] run_tests: allow run tests in parallel

[Peter Xu]

run_task.sh is getting slow. This patch is trying to make it faster by
running the tests concurrently.

We provide a new parameter "-j" for the run_tests.sh, which can be used
to specify how many run queues we want for the tests. Default queue
length is 1, which is the old behavior.

Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:

   |-----------------+-----------|
   | command         | time used |
   |-----------------+-----------|
   | run_test.sh     | 75s       |
   | run_test.sh -j8 | 27s       |
   |-----------------+-----------|

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 run_tests.sh        | 12 ++++++++++--
 scripts/common.bash | 16 +++++++++++++++-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index b6a1059..477d4fb 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -13,10 +13,11 @@ function usage()
 {
 cat <<EOF
 
-Usage: $0 [-g group] [-h] [-v]
+Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
 
     -g: Only execute tests in the given group
     -h: Output this help text
+    -j: Execute tests in parallel
     -v: Enables verbose mode
 
 Set the environment variable QEMU=/path/to/qemu-system-ARCH to
@@ -28,7 +29,7 @@ EOF
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
 
-while getopts "g:hv" opt; do
+while getopts "g:hj:v" opt; do
     case $opt in
         g)
             only_group=$OPTARG
@@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
             usage
             exit
             ;;
+        j)
+            unittest_run_queues=$OPTARG
+            if (( $unittest_run_queues <= 0 )); then
+                echo "Invalid -j option: $unittest_run_queues"
+                exit 1
+            fi
+            ;;
         v)
             verbose="yes"
             ;;
diff --git a/scripts/common.bash b/scripts/common.bash
index 2dd7360..ef103ee 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -1,11 +1,19 @@
 : ${unittest_log_dir:=logs}
+: ${unittest_run_queues:=1}
 
 function run_task()
 {
 	local testname="$2"
 
+	while (( $(jobs | wc -l) == $unittest_run_queues )); do
+		# wait for any background test to finish
+		wait -n
+	done
+
 	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
-	"$@"
+
+	# start the testcase in the background
+	"$@" &
 }
 
 function for_each_unittest()
@@ -22,6 +30,8 @@ function for_each_unittest()
 	local accel
 	local timeout
 
+	trap "wait; exit 130" SIGINT
+
 	exec {fd}<"$unittests"
 
 	while read -u $fd line; do
@@ -55,5 +65,9 @@ function for_each_unittest()
 		fi
 	done
 	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+
+	# wait until all task finish
+	wait
+
 	exec {fd}<&-
 }
-- 
2.7.4

[Qemu-devel] [kvm-unit-tests PATCH v5 2/2] run_tests: allow run tests in parallel

[Peter Xu]

run_task.sh is getting slow. This patch is trying to make it faster by
running the tests concurrently.

We provide a new parameter "-j" for the run_tests.sh, which can be used
to specify how many run queues we want for the tests. Default queue
length is 1, which is the old behavior.

Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:

   |-----------------+-----------|
   | command         | time used |
   |-----------------+-----------|
   | run_test.sh     | 75s       |
   | run_test.sh -j8 | 27s       |
   |-----------------+-----------|

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 run_tests.sh        | 12 ++++++++++--
 scripts/common.bash | 16 +++++++++++++++-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index b6a1059..477d4fb 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -13,10 +13,11 @@ function usage()
 {
 cat <<EOF
 
-Usage: $0 [-g group] [-h] [-v]
+Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
 
     -g: Only execute tests in the given group
     -h: Output this help text
+    -j: Execute tests in parallel
     -v: Enables verbose mode
 
 Set the environment variable QEMU=/path/to/qemu-system-ARCH to
@@ -28,7 +29,7 @@ EOF
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
 
-while getopts "g:hv" opt; do
+while getopts "g:hj:v" opt; do
     case $opt in
         g)
             only_group=$OPTARG
@@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
             usage
             exit
             ;;
+        j)
+            unittest_run_queues=$OPTARG
+            if (( $unittest_run_queues <= 0 )); then
+                echo "Invalid -j option: $unittest_run_queues"
+                exit 1
+            fi
+            ;;
         v)
             verbose="yes"
             ;;
diff --git a/scripts/common.bash b/scripts/common.bash
index 2dd7360..ef103ee 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -1,11 +1,19 @@
 : ${unittest_log_dir:=logs}
+: ${unittest_run_queues:=1}
 
 function run_task()
 {
 	local testname="$2"
 
+	while (( $(jobs | wc -l) == $unittest_run_queues )); do
+		# wait for any background test to finish
+		wait -n
+	done
+
 	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
-	"$@"
+
+	# start the testcase in the background
+	"$@" &
 }
 
 function for_each_unittest()
@@ -22,6 +30,8 @@ function for_each_unittest()
 	local accel
 	local timeout
 
+	trap "wait; exit 130" SIGINT
+
 	exec {fd}<"$unittests"
 
 	while read -u $fd line; do
@@ -55,5 +65,9 @@ function for_each_unittest()
 		fi
 	done
 	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+
+	# wait until all task finish
+	wait
+
 	exec {fd}<&-
 }
-- 
2.7.4

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 1/2] run_tests: put logs into per-test file

[Andrew Jones]

On Wed, Jan 11, 2017 at 01:29:34PM +0800, Peter Xu wrote:
> We were using test.log before to keep all the test logs. This patch
> creates one log file per test case under logs/ directory with name
> "TESTNAME.log". Meanwhile, we will keep the last time log into
> logs.old/.
> 
> Renaming scripts/functions.bash into scripts/common.bash to store some
> more global variables.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  .gitignore                              |  3 ++-
>  Makefile                                |  5 ++---
>  run_tests.sh                            | 18 +++++++++++-------
>  scripts/{functions.bash => common.bash} | 13 +++++++++++--
>  scripts/mkstandalone.sh                 |  2 +-
>  5 files changed, 27 insertions(+), 14 deletions(-)
>  rename scripts/{functions.bash => common.bash} (75%)
> 
> diff --git a/.gitignore b/.gitignore
> index 3155418..2213b9b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -12,7 +12,8 @@ cscope.*
>  /lib/asm
>  /config.mak
>  /*-run
> -/test.log
>  /msr.out
>  /tests
>  /build-head
> +/logs/
> +/logs.old/

We should send a cleanup patch fixing the other dirs too someday,
patches, /lib/asm, and /tests

> diff --git a/Makefile b/Makefile
> index a32333b..844bacc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -94,9 +94,8 @@ libfdt_clean:
>  	$(LIBFDT_objdir)/.*.d
>  
>  distclean: clean libfdt_clean
> -	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> -	      build-head
> -	$(RM) -r tests
> +	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> +	$(RM) -r tests logs logs.old
>  
>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
>  cscope:
> diff --git a/run_tests.sh b/run_tests.sh
> index 254129d..b6a1059 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
>      exit 1
>  fi
>  source config.mak
> -source scripts/functions.bash
> +source scripts/common.bash
>  
>  function usage()
>  {
> @@ -46,17 +46,21 @@ while getopts "g:hv" opt; do
>      esac
>  done
>  
> -RUNTIME_log_stderr () { cat >> test.log; }
> +# RUNTIME_log_file will be configured later
> +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
>  RUNTIME_log_stdout () {
>      if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> -        ./scripts/pretty_print_stacks.py $1 >> test.log
> +        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
>      else
> -        cat >> test.log
> +        cat >> $RUNTIME_log_file
>      fi
>  }
>  
> -
>  config=$TEST_DIR/unittests.cfg
> -rm -f test.log
> -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> +
> +rm -rf $unittest_log_dir.old
> +mv $unittest_log_dir $unittest_log_dir.old
> +mkdir $unittest_log_dir
> +echo "BUILD_HEAD=$(cat build-head)" > $unittest_log_dir/SUMMARY
> +
>  for_each_unittest $config run
> diff --git a/scripts/functions.bash b/scripts/common.bash
> similarity index 75%
> rename from scripts/functions.bash
> rename to scripts/common.bash
> index ee9143c..2dd7360 100644
> --- a/scripts/functions.bash
> +++ b/scripts/common.bash
> @@ -1,3 +1,12 @@
> +: ${unittest_log_dir:=logs}
> +
> +function run_task()
> +{
> +	local testname="$2"
> +
> +	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> +	"$@"
> +}
>  
>  function for_each_unittest()
>  {
> @@ -17,7 +26,7 @@ function for_each_unittest()
>  
>  	while read -u $fd line; do
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> -			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +			run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>  			testname=${BASH_REMATCH[1]}
>  			smp=1
>  			kernel=""
> @@ -45,6 +54,6 @@ function for_each_unittest()
>  			timeout=${BASH_REMATCH[1]}
>  		fi
>  	done
> -	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>  	exec {fd}<&-
>  }
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index d2bae19..3c1938e 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -5,7 +5,7 @@ if [ ! -f config.mak ]; then
>  	exit 1
>  fi
>  source config.mak
> -source scripts/functions.bash
> +source scripts/common.bash
>  
>  escape ()
>  {
> -- 
> 2.7.4
> 
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 1/2] run_tests: put logs into per-test file

[Andrew Jones]

On Wed, Jan 11, 2017 at 10:06:07AM +0100, Andrew Jones wrote:
> On Wed, Jan 11, 2017 at 01:29:34PM +0800, Peter Xu wrote:
> > We were using test.log before to keep all the test logs. This patch
> > creates one log file per test case under logs/ directory with name
> > "TESTNAME.log". Meanwhile, we will keep the last time log into
> > logs.old/.
> > 
> > Renaming scripts/functions.bash into scripts/common.bash to store some
> > more global variables.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  .gitignore                              |  3 ++-
> >  Makefile                                |  5 ++---
> >  run_tests.sh                            | 18 +++++++++++-------
> >  scripts/{functions.bash => common.bash} | 13 +++++++++++--
> >  scripts/mkstandalone.sh                 |  2 +-
> >  5 files changed, 27 insertions(+), 14 deletions(-)
> >  rename scripts/{functions.bash => common.bash} (75%)
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 3155418..2213b9b 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -12,7 +12,8 @@ cscope.*
> >  /lib/asm
> >  /config.mak
> >  /*-run
> > -/test.log
> >  /msr.out
> >  /tests
> >  /build-head
> > +/logs/
> > +/logs.old/
> 
> We should send a cleanup patch fixing the other dirs too someday,
> patches, /lib/asm, and /tests
> 
> > diff --git a/Makefile b/Makefile
> > index a32333b..844bacc 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -94,9 +94,8 @@ libfdt_clean:
> >  	$(LIBFDT_objdir)/.*.d
> >  
> >  distclean: clean libfdt_clean
> > -	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> > -	      build-head
> > -	$(RM) -r tests
> > +	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> > +	$(RM) -r tests logs logs.old
> >  
> >  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
> >  cscope:
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 254129d..b6a1059 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
> >      exit 1
> >  fi
> >  source config.mak
> > -source scripts/functions.bash
> > +source scripts/common.bash
> >  
> >  function usage()
> >  {
> > @@ -46,17 +46,21 @@ while getopts "g:hv" opt; do
> >      esac
> >  done
> >  
> > -RUNTIME_log_stderr () { cat >> test.log; }
> > +# RUNTIME_log_file will be configured later
> > +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
> >  RUNTIME_log_stdout () {
> >      if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> > -        ./scripts/pretty_print_stacks.py $1 >> test.log
> > +        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
> >      else
> > -        cat >> test.log
> > +        cat >> $RUNTIME_log_file
> >      fi
> >  }
> >  
> > -
> >  config=$TEST_DIR/unittests.cfg
> > -rm -f test.log
> > -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> > +
> > +rm -rf $unittest_log_dir.old
> > +mv $unittest_log_dir $unittest_log_dir.old

Actually this mv can fail, but we definitely wouldn't want to error-out
like v4 did, we want to ignore it. The error is that /logs doesn't exist,
as it won't the first time run_tests.sh is run. So, we need to redirect
stderr to /dev/null on this mv to avoid the message
"mv: cannot stat 'logs': No such file or directory"
on that first run.

Thanks,
drew

> > +mkdir $unittest_log_dir
> > +echo "BUILD_HEAD=$(cat build-head)" > $unittest_log_dir/SUMMARY
> > +
> >  for_each_unittest $config run
> > diff --git a/scripts/functions.bash b/scripts/common.bash
> > similarity index 75%
> > rename from scripts/functions.bash
> > rename to scripts/common.bash
> > index ee9143c..2dd7360 100644
> > --- a/scripts/functions.bash
> > +++ b/scripts/common.bash
> > @@ -1,3 +1,12 @@
> > +: ${unittest_log_dir:=logs}
> > +
> > +function run_task()
> > +{
> > +	local testname="$2"
> > +
> > +	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> > +	"$@"
> > +}
> >  
> >  function for_each_unittest()
> >  {
> > @@ -17,7 +26,7 @@ function for_each_unittest()
> >  
> >  	while read -u $fd line; do
> >  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> > -			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > +			run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> >  			testname=${BASH_REMATCH[1]}
> >  			smp=1
> >  			kernel=""
> > @@ -45,6 +54,6 @@ function for_each_unittest()
> >  			timeout=${BASH_REMATCH[1]}
> >  		fi
> >  	done
> > -	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > +	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> >  	exec {fd}<&-
> >  }
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index d2bae19..3c1938e 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -5,7 +5,7 @@ if [ ! -f config.mak ]; then
> >  	exit 1
> >  fi
> >  source config.mak
> > -source scripts/functions.bash
> > +source scripts/common.bash
> >  
> >  escape ()
> >  {
> > -- 
> > 2.7.4
> > 
> >
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 1/2] run_tests: put logs into per-test file

[Peter Xu]

On Wed, Jan 11, 2017 at 10:51:12AM +0100, Andrew Jones wrote:
> On Wed, Jan 11, 2017 at 10:06:07AM +0100, Andrew Jones wrote:
> > On Wed, Jan 11, 2017 at 01:29:34PM +0800, Peter Xu wrote:
> > > We were using test.log before to keep all the test logs. This patch
> > > creates one log file per test case under logs/ directory with name
> > > "TESTNAME.log". Meanwhile, we will keep the last time log into
> > > logs.old/.
> > > 
> > > Renaming scripts/functions.bash into scripts/common.bash to store some
> > > more global variables.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > >  .gitignore                              |  3 ++-
> > >  Makefile                                |  5 ++---
> > >  run_tests.sh                            | 18 +++++++++++-------
> > >  scripts/{functions.bash => common.bash} | 13 +++++++++++--
> > >  scripts/mkstandalone.sh                 |  2 +-
> > >  5 files changed, 27 insertions(+), 14 deletions(-)
> > >  rename scripts/{functions.bash => common.bash} (75%)
> > > 
> > > diff --git a/.gitignore b/.gitignore
> > > index 3155418..2213b9b 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -12,7 +12,8 @@ cscope.*
> > >  /lib/asm
> > >  /config.mak
> > >  /*-run
> > > -/test.log
> > >  /msr.out
> > >  /tests
> > >  /build-head
> > > +/logs/
> > > +/logs.old/
> > 
> > We should send a cleanup patch fixing the other dirs too someday,
> > patches, /lib/asm, and /tests
> > 
> > > diff --git a/Makefile b/Makefile
> > > index a32333b..844bacc 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -94,9 +94,8 @@ libfdt_clean:
> > >  	$(LIBFDT_objdir)/.*.d
> > >  
> > >  distclean: clean libfdt_clean
> > > -	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> > > -	      build-head
> > > -	$(RM) -r tests
> > > +	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> > > +	$(RM) -r tests logs logs.old
> > >  
> > >  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
> > >  cscope:
> > > diff --git a/run_tests.sh b/run_tests.sh
> > > index 254129d..b6a1059 100755
> > > --- a/run_tests.sh
> > > +++ b/run_tests.sh
> > > @@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
> > >      exit 1
> > >  fi
> > >  source config.mak
> > > -source scripts/functions.bash
> > > +source scripts/common.bash
> > >  
> > >  function usage()
> > >  {
> > > @@ -46,17 +46,21 @@ while getopts "g:hv" opt; do
> > >      esac
> > >  done
> > >  
> > > -RUNTIME_log_stderr () { cat >> test.log; }
> > > +# RUNTIME_log_file will be configured later
> > > +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
> > >  RUNTIME_log_stdout () {
> > >      if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> > > -        ./scripts/pretty_print_stacks.py $1 >> test.log
> > > +        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
> > >      else
> > > -        cat >> test.log
> > > +        cat >> $RUNTIME_log_file
> > >      fi
> > >  }
> > >  
> > > -
> > >  config=$TEST_DIR/unittests.cfg
> > > -rm -f test.log
> > > -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> > > +
> > > +rm -rf $unittest_log_dir.old
> > > +mv $unittest_log_dir $unittest_log_dir.old
> 
> Actually this mv can fail, but we definitely wouldn't want to error-out
> like v4 did, we want to ignore it. The error is that /logs doesn't exist,
> as it won't the first time run_tests.sh is run. So, we need to redirect
> stderr to /dev/null on this mv to avoid the message
> "mv: cannot stat 'logs': No such file or directory"
> on that first run.

Hmm, yes.

After a second thought, I think the mv can still fail even if this is
not the first time we run the script - e.g. if the directory belongs
to user1 but user2 runs run_tests.sh under it (while user2 don't have
write permission on this directory). So I think we still need some way
to stop the script if we detected that we might encounter issue.

We can do explicit check before the mv, making sure that the directory
is there.

So, how about this:

    rm -rf $unittest_log_dir.old || err "Failed remove old logs"
    if [[ -d $unittest_log_dir ]]; then
        mv $unittest_log_dir $unittest_log_dir.old ||
            err "Failed backup logs"
    fi
    mkdir $unittest_log_dir || err "Failed to create log dir"

And define err() in common.bash:

    function err()
    {
        echo "$@"
        exit 1
    }

-- peterx

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 1/2] run_tests: put logs into per-test file

[Andrew Jones]

On Wed, Jan 11, 2017 at 06:12:44PM +0800, Peter Xu wrote:
> On Wed, Jan 11, 2017 at 10:51:12AM +0100, Andrew Jones wrote:
> > On Wed, Jan 11, 2017 at 10:06:07AM +0100, Andrew Jones wrote:
> > > On Wed, Jan 11, 2017 at 01:29:34PM +0800, Peter Xu wrote:
> > > > We were using test.log before to keep all the test logs. This patch
> > > > creates one log file per test case under logs/ directory with name
> > > > "TESTNAME.log". Meanwhile, we will keep the last time log into
> > > > logs.old/.
> > > > 
> > > > Renaming scripts/functions.bash into scripts/common.bash to store some
> > > > more global variables.
> > > > 
> > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > ---
> > > >  .gitignore                              |  3 ++-
> > > >  Makefile                                |  5 ++---
> > > >  run_tests.sh                            | 18 +++++++++++-------
> > > >  scripts/{functions.bash => common.bash} | 13 +++++++++++--
> > > >  scripts/mkstandalone.sh                 |  2 +-
> > > >  5 files changed, 27 insertions(+), 14 deletions(-)
> > > >  rename scripts/{functions.bash => common.bash} (75%)
> > > > 
> > > > diff --git a/.gitignore b/.gitignore
> > > > index 3155418..2213b9b 100644
> > > > --- a/.gitignore
> > > > +++ b/.gitignore
> > > > @@ -12,7 +12,8 @@ cscope.*
> > > >  /lib/asm
> > > >  /config.mak
> > > >  /*-run
> > > > -/test.log
> > > >  /msr.out
> > > >  /tests
> > > >  /build-head
> > > > +/logs/
> > > > +/logs.old/
> > > 
> > > We should send a cleanup patch fixing the other dirs too someday,
> > > patches, /lib/asm, and /tests
> > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index a32333b..844bacc 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -94,9 +94,8 @@ libfdt_clean:
> > > >  	$(LIBFDT_objdir)/.*.d
> > > >  
> > > >  distclean: clean libfdt_clean
> > > > -	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> > > > -	      build-head
> > > > -	$(RM) -r tests
> > > > +	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> > > > +	$(RM) -r tests logs logs.old
> > > >  
> > > >  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
> > > >  cscope:
> > > > diff --git a/run_tests.sh b/run_tests.sh
> > > > index 254129d..b6a1059 100755
> > > > --- a/run_tests.sh
> > > > +++ b/run_tests.sh
> > > > @@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
> > > >      exit 1
> > > >  fi
> > > >  source config.mak
> > > > -source scripts/functions.bash
> > > > +source scripts/common.bash
> > > >  
> > > >  function usage()
> > > >  {
> > > > @@ -46,17 +46,21 @@ while getopts "g:hv" opt; do
> > > >      esac
> > > >  done
> > > >  
> > > > -RUNTIME_log_stderr () { cat >> test.log; }
> > > > +# RUNTIME_log_file will be configured later
> > > > +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
> > > >  RUNTIME_log_stdout () {
> > > >      if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> > > > -        ./scripts/pretty_print_stacks.py $1 >> test.log
> > > > +        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
> > > >      else
> > > > -        cat >> test.log
> > > > +        cat >> $RUNTIME_log_file
> > > >      fi
> > > >  }
> > > >  
> > > > -
> > > >  config=$TEST_DIR/unittests.cfg
> > > > -rm -f test.log
> > > > -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> > > > +
> > > > +rm -rf $unittest_log_dir.old
> > > > +mv $unittest_log_dir $unittest_log_dir.old
> > 
> > Actually this mv can fail, but we definitely wouldn't want to error-out
> > like v4 did, we want to ignore it. The error is that /logs doesn't exist,
> > as it won't the first time run_tests.sh is run. So, we need to redirect
> > stderr to /dev/null on this mv to avoid the message
> > "mv: cannot stat 'logs': No such file or directory"
> > on that first run.
> 
> Hmm, yes.
> 
> After a second thought, I think the mv can still fail even if this is
> not the first time we run the script - e.g. if the directory belongs
> to user1 but user2 runs run_tests.sh under it (while user2 don't have
> write permission on this directory). So I think we still need some way
> to stop the script if we detected that we might encounter issue.

I'm not really concerned about the user1/user2 scenario. The general use
of kvm-unit-tests is to build and run, meaning the user must have write
access to the dir in order to compile. But, ignoring the errors, as I
suggested, probably isn't a great idea either...

> 
> We can do explicit check before the mv, making sure that the directory
> is there.
> 
> So, how about this:
> 
>     rm -rf $unittest_log_dir.old || err "Failed remove old logs"
>     if [[ -d $unittest_log_dir ]]; then

Only [ ... ] for tests like these

>         mv $unittest_log_dir $unittest_log_dir.old ||
>             err "Failed backup logs"
>     fi
>     mkdir $unittest_log_dir || err "Failed to create log dir"
> 
> And define err() in common.bash:
> 
>     function err()
>     {
>         echo "$@"
>         exit 1
>     }

The above is mostly just translating rm/mv/mkdir stderr messages to
new messages. We can do it much more simply like

 rm -rf logs.old
 [ -d logs ] && mv logs logs.old
 mkdir logs || exit 2

Thanks,
drew

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 2/2] run_tests: allow run tests in parallel

[Andrew Jones]

On Wed, Jan 11, 2017 at 01:29:35PM +0800, Peter Xu wrote:
> run_task.sh is getting slow. This patch is trying to make it faster by
> running the tests concurrently.
> 
> We provide a new parameter "-j" for the run_tests.sh, which can be used
> to specify how many run queues we want for the tests. Default queue
> length is 1, which is the old behavior.
> 
> Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:
> 
>    |-----------------+-----------|
>    | command         | time used |
>    |-----------------+-----------|
>    | run_test.sh     | 75s       |
>    | run_test.sh -j8 | 27s       |
>    |-----------------+-----------|
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  run_tests.sh        | 12 ++++++++++--
>  scripts/common.bash | 16 +++++++++++++++-
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index b6a1059..477d4fb 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,10 +13,11 @@ function usage()
>  {
>  cat <<EOF
>  
> -Usage: $0 [-g group] [-h] [-v]
> +Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
>  
>      -g: Only execute tests in the given group
>      -h: Output this help text
> +    -j: Execute tests in parallel
>      -v: Enables verbose mode
>  
>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> @@ -28,7 +29,7 @@ EOF
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
>  
> -while getopts "g:hv" opt; do
> +while getopts "g:hj:v" opt; do
>      case $opt in
>          g)
>              only_group=$OPTARG
> @@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
>              usage
>              exit
>              ;;
> +        j)
> +            unittest_run_queues=$OPTARG
> +            if (( $unittest_run_queues <= 0 )); then
> +                echo "Invalid -j option: $unittest_run_queues"
> +                exit 1

We should probably use 'exit 2' here, and below in the *) case.

> +            fi
> +            ;;
>          v)
>              verbose="yes"
>              ;;
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 2dd7360..ef103ee 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -1,11 +1,19 @@
>  : ${unittest_log_dir:=logs}
> +: ${unittest_run_queues:=1}
>  
>  function run_task()
>  {
>  	local testname="$2"
>  
> +	while (( $(jobs | wc -l) == $unittest_run_queues )); do
> +		# wait for any background test to finish
> +		wait -n
> +	done
> +
>  	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> -	"$@"
> +
> +	# start the testcase in the background
> +	"$@" &

If you check the logs before and after applying this patch series you'll
see a bunch of "stty: 'standard input': Inappropriate ioctl for device"
are now present. These messages come from the stty calls in run_qemu,
which we need to avoid the loss of terminal echo when QEMU aborts. We
can get rid of these new "inappropriate ioctl" messages by changing the
above line to

 "$@" <$(tty) &


>  }
>  
>  function for_each_unittest()
> @@ -22,6 +30,8 @@ function for_each_unittest()
>  	local accel
>  	local timeout
>  
> +	trap "wait; exit 130" SIGINT
> +
>  	exec {fd}<"$unittests"
>  
>  	while read -u $fd line; do
> @@ -55,5 +65,9 @@ function for_each_unittest()
>  		fi
>  	done
>  	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +
> +	# wait until all task finish

Still not quite right :-)  s/task/tasks/

> +	wait
> +
>  	exec {fd}<&-
>  }
> -- 
> 2.7.4
> 
> 

Thanks,
drew

Re: [kvm-unit-tests PATCH v5 2/2] run_tests: allow run tests in parallel

[Andrew Jones]

On Wed, Jan 11, 2017 at 12:00:23PM +0100, Andrew Jones wrote:
> On Wed, Jan 11, 2017 at 01:29:35PM +0800, Peter Xu wrote:
> > run_task.sh is getting slow. This patch is trying to make it faster by
> > running the tests concurrently.
> > 
> > We provide a new parameter "-j" for the run_tests.sh, which can be used
> > to specify how many run queues we want for the tests. Default queue
> > length is 1, which is the old behavior.
> > 
> > Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:
> > 
> >    |-----------------+-----------|
> >    | command         | time used |
> >    |-----------------+-----------|
> >    | run_test.sh     | 75s       |
> >    | run_test.sh -j8 | 27s       |
> >    |-----------------+-----------|
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  run_tests.sh        | 12 ++++++++++--
> >  scripts/common.bash | 16 +++++++++++++++-
> >  2 files changed, 25 insertions(+), 3 deletions(-)
> > 
> > diff --git a/run_tests.sh b/run_tests.sh
> > index b6a1059..477d4fb 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -13,10 +13,11 @@ function usage()
> >  {
> >  cat <<EOF
> >  
> > -Usage: $0 [-g group] [-h] [-v]
> > +Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
> >  
> >      -g: Only execute tests in the given group
> >      -h: Output this help text
> > +    -j: Execute tests in parallel
> >      -v: Enables verbose mode
> >  
> >  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> > @@ -28,7 +29,7 @@ EOF
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > -while getopts "g:hv" opt; do
> > +while getopts "g:hj:v" opt; do
> >      case $opt in
> >          g)
> >              only_group=$OPTARG
> > @@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
> >              usage
> >              exit
> >              ;;
> > +        j)
> > +            unittest_run_queues=$OPTARG
> > +            if (( $unittest_run_queues <= 0 )); then
> > +                echo "Invalid -j option: $unittest_run_queues"
> > +                exit 1
> 
> We should probably use 'exit 2' here, and below in the *) case.
> 
> > +            fi
> > +            ;;
> >          v)
> >              verbose="yes"
> >              ;;
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 2dd7360..ef103ee 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -1,11 +1,19 @@
> >  : ${unittest_log_dir:=logs}
> > +: ${unittest_run_queues:=1}
> >  
> >  function run_task()
> >  {
> >  	local testname="$2"
> >  
> > +	while (( $(jobs | wc -l) == $unittest_run_queues )); do
> > +		# wait for any background test to finish
> > +		wait -n
> > +	done
> > +
> >  	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> > -	"$@"
> > +
> > +	# start the testcase in the background
> > +	"$@" &
> 
> If you check the logs before and after applying this patch series you'll
> see a bunch of "stty: 'standard input': Inappropriate ioctl for device"
> are now present. These messages come from the stty calls in run_qemu,
> which we need to avoid the loss of terminal echo when QEMU aborts. We
> can get rid of these new "inappropriate ioctl" messages by changing the
> above line to
> 
>  "$@" <$(tty) &

Actually I just came up with a better solution for this. Since we don't
need input to our unit tests, i.e. we never attempt to read from the
serial port within them, then we can just use /dev/null for stdin. That
allows us to leave your code above alone, and also remove the stty stuff
in run_qemu. I'll send a patch.

drew

> 
> 
> >  }
> >  
> >  function for_each_unittest()
> > @@ -22,6 +30,8 @@ function for_each_unittest()
> >  	local accel
> >  	local timeout
> >  
> > +	trap "wait; exit 130" SIGINT
> > +
> >  	exec {fd}<"$unittests"
> >  
> >  	while read -u $fd line; do
> > @@ -55,5 +65,9 @@ function for_each_unittest()
> >  		fi
> >  	done
> >  	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > +
> > +	# wait until all task finish
> 
> Still not quite right :-)  s/task/tasks/
> 
> > +	wait
> > +
> >  	exec {fd}<&-
> >  }
> > -- 
> > 2.7.4
> > 
> > 
> 
> Thanks,
> drew
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 2/2] run_tests: allow run tests in parallel

[Andrew Jones]

On Wed, Jan 11, 2017 at 12:00:23PM +0100, Andrew Jones wrote:
> On Wed, Jan 11, 2017 at 01:29:35PM +0800, Peter Xu wrote:
> > run_task.sh is getting slow. This patch is trying to make it faster by
> > running the tests concurrently.
> > 
> > We provide a new parameter "-j" for the run_tests.sh, which can be used
> > to specify how many run queues we want for the tests. Default queue
> > length is 1, which is the old behavior.
> > 
> > Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:
> > 
> >    |-----------------+-----------|
> >    | command         | time used |
> >    |-----------------+-----------|
> >    | run_test.sh     | 75s       |
> >    | run_test.sh -j8 | 27s       |
> >    |-----------------+-----------|
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  run_tests.sh        | 12 ++++++++++--
> >  scripts/common.bash | 16 +++++++++++++++-
> >  2 files changed, 25 insertions(+), 3 deletions(-)
> > 
> > diff --git a/run_tests.sh b/run_tests.sh
> > index b6a1059..477d4fb 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -13,10 +13,11 @@ function usage()
> >  {
> >  cat <<EOF
> >  
> > -Usage: $0 [-g group] [-h] [-v]
> > +Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
> >  
> >      -g: Only execute tests in the given group
> >      -h: Output this help text
> > +    -j: Execute tests in parallel
> >      -v: Enables verbose mode
> >  
> >  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> > @@ -28,7 +29,7 @@ EOF
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > -while getopts "g:hv" opt; do
> > +while getopts "g:hj:v" opt; do
> >      case $opt in
> >          g)
> >              only_group=$OPTARG
> > @@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
> >              usage
> >              exit
> >              ;;
> > +        j)
> > +            unittest_run_queues=$OPTARG
> > +            if (( $unittest_run_queues <= 0 )); then
> > +                echo "Invalid -j option: $unittest_run_queues"
> > +                exit 1
> 
> We should probably use 'exit 2' here, and below in the *) case.
> 
> > +            fi
> > +            ;;
> >          v)
> >              verbose="yes"
> >              ;;
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 2dd7360..ef103ee 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -1,11 +1,19 @@
> >  : ${unittest_log_dir:=logs}
> > +: ${unittest_run_queues:=1}
> >  
> >  function run_task()
> >  {
> >  	local testname="$2"
> >  
> > +	while (( $(jobs | wc -l) == $unittest_run_queues )); do
> > +		# wait for any background test to finish
> > +		wait -n
> > +	done
> > +
> >  	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> > -	"$@"
> > +
> > +	# start the testcase in the background
> > +	"$@" &
> 
> If you check the logs before and after applying this patch series you'll
> see a bunch of "stty: 'standard input': Inappropriate ioctl for device"
> are now present. These messages come from the stty calls in run_qemu,
> which we need to avoid the loss of terminal echo when QEMU aborts. We
> can get rid of these new "inappropriate ioctl" messages by changing the
> above line to
> 
>  "$@" <$(tty) &

Actually I just came up with a better solution for this. Since we don't
need input to our unit tests, i.e. we never attempt to read from the
serial port within them, then we can just use /dev/null for stdin. That
allows us to leave your code above alone, and also remove the stty stuff
in run_qemu. I'll send a patch.

drew

> 
> 
> >  }
> >  
> >  function for_each_unittest()
> > @@ -22,6 +30,8 @@ function for_each_unittest()
> >  	local accel
> >  	local timeout
> >  
> > +	trap "wait; exit 130" SIGINT
> > +
> >  	exec {fd}<"$unittests"
> >  
> >  	while read -u $fd line; do
> > @@ -55,5 +65,9 @@ function for_each_unittest()
> >  		fi
> >  	done
> >  	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > +
> > +	# wait until all task finish
> 
> Still not quite right :-)  s/task/tasks/
> 
> > +	wait
> > +
> >  	exec {fd}<&-
> >  }
> > -- 
> > 2.7.4
> > 
> > 
> 
> Thanks,
> drew
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 1/2] run_tests: put logs into per-test file

[Peter Xu]

On Wed, Jan 11, 2017 at 11:46:38AM +0100, Andrew Jones wrote:

[...]

> > So, how about this:
> > 
> >     rm -rf $unittest_log_dir.old || err "Failed remove old logs"
> >     if [[ -d $unittest_log_dir ]]; then
> 
> Only [ ... ] for tests like these

I thought [[ ... ]] would be superior to [ ... ] if we are not
considering the POSIX compatibility issue?

Hmm, after a quick grep, I see that kvm-unit-tests repo is using [[
... ]] only if doing any kind of pattern/regex matching, right? If so,
I'll just follow. ;-)

> 
> >         mv $unittest_log_dir $unittest_log_dir.old ||
> >             err "Failed backup logs"
> >     fi
> >     mkdir $unittest_log_dir || err "Failed to create log dir"
> > 
> > And define err() in common.bash:
> > 
> >     function err()
> >     {
> >         echo "$@"
> >         exit 1
> >     }
> 
> The above is mostly just translating rm/mv/mkdir stderr messages to
> new messages. We can do it much more simply like
> 
>  rm -rf logs.old
>  [ -d logs ] && mv logs logs.old
>  mkdir logs || exit 2

Okay I'll use this. Thanks!

-- peterx

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 2/2] run_tests: allow run tests in parallel

[Peter Xu]

On Wed, Jan 11, 2017 at 02:09:34PM +0100, Andrew Jones wrote:
> On Wed, Jan 11, 2017 at 12:00:23PM +0100, Andrew Jones wrote:
> > On Wed, Jan 11, 2017 at 01:29:35PM +0800, Peter Xu wrote:
> > > run_task.sh is getting slow. This patch is trying to make it faster by
> > > running the tests concurrently.
> > > 
> > > We provide a new parameter "-j" for the run_tests.sh, which can be used
> > > to specify how many run queues we want for the tests. Default queue
> > > length is 1, which is the old behavior.
> > > 
> > > Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:
> > > 
> > >    |-----------------+-----------|
> > >    | command         | time used |
> > >    |-----------------+-----------|
> > >    | run_test.sh     | 75s       |
> > >    | run_test.sh -j8 | 27s       |
> > >    |-----------------+-----------|
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > >  run_tests.sh        | 12 ++++++++++--
> > >  scripts/common.bash | 16 +++++++++++++++-
> > >  2 files changed, 25 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/run_tests.sh b/run_tests.sh
> > > index b6a1059..477d4fb 100755
> > > --- a/run_tests.sh
> > > +++ b/run_tests.sh
> > > @@ -13,10 +13,11 @@ function usage()
> > >  {
> > >  cat <<EOF
> > >  
> > > -Usage: $0 [-g group] [-h] [-v]
> > > +Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
> > >  
> > >      -g: Only execute tests in the given group
> > >      -h: Output this help text
> > > +    -j: Execute tests in parallel
> > >      -v: Enables verbose mode
> > >  
> > >  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> > > @@ -28,7 +29,7 @@ EOF
> > >  RUNTIME_arch_run="./$TEST_DIR/run"
> > >  source scripts/runtime.bash
> > >  
> > > -while getopts "g:hv" opt; do
> > > +while getopts "g:hj:v" opt; do
> > >      case $opt in
> > >          g)
> > >              only_group=$OPTARG
> > > @@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
> > >              usage
> > >              exit
> > >              ;;
> > > +        j)
> > > +            unittest_run_queues=$OPTARG
> > > +            if (( $unittest_run_queues <= 0 )); then
> > > +                echo "Invalid -j option: $unittest_run_queues"
> > > +                exit 1
> > 
> > We should probably use 'exit 2' here, and below in the *) case.

Will fix this one first, with another one line patch for the below one
(we can either take that new one, or squash it into this patch).

> > 
> > > +            fi
> > > +            ;;
> > >          v)
> > >              verbose="yes"
> > >              ;;
> > > diff --git a/scripts/common.bash b/scripts/common.bash
> > > index 2dd7360..ef103ee 100644
> > > --- a/scripts/common.bash
> > > +++ b/scripts/common.bash
> > > @@ -1,11 +1,19 @@
> > >  : ${unittest_log_dir:=logs}
> > > +: ${unittest_run_queues:=1}
> > >  
> > >  function run_task()
> > >  {
> > >  	local testname="$2"
> > >  
> > > +	while (( $(jobs | wc -l) == $unittest_run_queues )); do
> > > +		# wait for any background test to finish
> > > +		wait -n
> > > +	done
> > > +
> > >  	RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> > > -	"$@"
> > > +
> > > +	# start the testcase in the background
> > > +	"$@" &
> > 
> > If you check the logs before and after applying this patch series you'll
> > see a bunch of "stty: 'standard input': Inappropriate ioctl for device"
> > are now present. These messages come from the stty calls in run_qemu,
> > which we need to avoid the loss of terminal echo when QEMU aborts. We
> > can get rid of these new "inappropriate ioctl" messages by changing the
> > above line to
> > 
> >  "$@" <$(tty) &
> 
> Actually I just came up with a better solution for this. Since we don't
> need input to our unit tests, i.e. we never attempt to read from the
> serial port within them, then we can just use /dev/null for stdin. That
> allows us to leave your code above alone, and also remove the stty stuff
> in run_qemu. I'll send a patch.

Thanks, then I'll keep it as it is.

> 
> drew
> 
> > 
> > 
> > >  }
> > >  
> > >  function for_each_unittest()
> > > @@ -22,6 +30,8 @@ function for_each_unittest()
> > >  	local accel
> > >  	local timeout
> > >  
> > > +	trap "wait; exit 130" SIGINT
> > > +
> > >  	exec {fd}<"$unittests"
> > >  
> > >  	while read -u $fd line; do
> > > @@ -55,5 +65,9 @@ function for_each_unittest()
> > >  		fi
> > >  	done
> > >  	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > > +
> > > +	# wait until all task finish
> > 
> > Still not quite right :-)  s/task/tasks/

Sorry! Fixing up.

-- peterx