[PATCH kvm-unit-tests 00/12] scripts/mkstandalone: rewrite

[PATCH kvm-unit-tests 00/12] scripts/mkstandalone: rewrite

[Radim Krčmář]

Drew and I have shelved series' where mkstandalone was duplicating the
effort.  This patchset should make future changes much simpler by
reusing code from other files in standalone tests.


Radim Krčmář (12):
  scripts/mkstandalone: simplify redirection
  scripts/mkstandalone: fix ARCH
  scripts/mkstandalone: refactor temp file creation
  scripts/mkstandalone: use bash in standalone test
  scripts/mkstandalone: reuse $TEST_DIR/run
  run_tests: move reusable code to scripts/runtime
  scripts/runtime: change callsite of $TEST_DIR/run
  scripts/mkstandalone: refactor check for test kernel
  scripts/mkstandalone: use scripts/runtime
  scripts/mkstandalone: simplifiy redirection, again
  scripts/mkstandalone: sanitize one_kernel handling
  scripts/mkstandalone: return the exit value of a test

 run_tests.sh            | 101 +----------------------------
 scripts/mkstandalone.sh | 164 +++++++++++++++++++-----------------------------
 scripts/runtime.bash    | 103 ++++++++++++++++++++++++++++++
 3 files changed, 169 insertions(+), 199 deletions(-)
 create mode 100644 scripts/runtime.bash

-- 
2.7.0

[PATCH kvm-unit-tests 01/12] scripts/mkstandalone: simplify redirection

[Radim Krčmář]

Use exec to redirect stdout to a file, and start the cleanup.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 3ce244aff67b..5f9ef146208e 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -48,12 +48,13 @@ function mkstandalone()
 	qemu=$(cut -d' ' -f1 <<< "$cmdline")
 	cmdline=$(cut -d' ' -f2- <<< "$cmdline")
 
-	cat <<EOF > $standalone
-#!/bin/sh
+	exec {tmpfd}<&1
+	exec > $standalone
+
+	echo "#!/bin/sh"
 
-EOF
 if [ "$arch" ]; then
-	cat <<EOF >> $standalone
+	cat <<EOF
 ARCH=\`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'\`
 [ "\$ARCH" = "aarch64" ] && ARCH="arm64"
 if [ "\$ARCH" != "$arch" ]; then
@@ -64,7 +65,7 @@ fi
 EOF
 fi
 if [ "$check" ]; then
-	cat <<EOF >> $standalone
+	cat <<EOF
 for param in $check; do
 	path=\`echo \$param | cut -d= -f1\`
 	value=\`echo \$param | cut -d= -f2\`
@@ -77,18 +78,18 @@ done
 EOF
 fi
 if [ ! -f $kernel ]; then
-	cat <<EOF >> $standalone
+	cat <<EOF
 echo "skip $testname (test kernel not present)" 1>&2
 exit 1
 EOF
 else
-	cat <<EOF >> $standalone
+	cat <<EOF
 trap 'rm -f \$bin; exit 1' HUP INT TERM
 bin=\`mktemp\`
 base64 -d << 'BIN_EOF' | zcat > \$bin &&
 EOF
-gzip - < $kernel | base64 >> $standalone
-	cat <<EOF >> $standalone
+gzip - < $kernel | base64
+	cat <<EOF
 BIN_EOF
 
 qemu="$qemu"
@@ -122,8 +123,10 @@ rm -f \$bin
 exit 0
 EOF
 fi
-chmod +x $standalone
-return 0
+	exec 1<&$tmpfd {tmpfd}<&-
+	chmod +x $standalone
+
+	return 0
 }
 
 trap 'rm -f $cfg; exit 1' HUP INT TERM
-- 
2.7.0

[PATCH kvm-unit-tests 02/12] scripts/mkstandalone: fix ARCH

[Radim Krčmář]

ARCH depends on the built image, not on target's arch.
Use the value from config.mak.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 5f9ef146208e..47d6e279d212 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -53,17 +53,8 @@ function mkstandalone()
 
 	echo "#!/bin/sh"
 
-if [ "$arch" ]; then
-	cat <<EOF
-ARCH=\`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'\`
-[ "\$ARCH" = "aarch64" ] && ARCH="arm64"
-if [ "\$ARCH" != "$arch" ]; then
-	echo "skip $testname ($arch only)" 1>&2
-	exit 1
-fi
+	cat config.mak | grep '^ARCH='
 
-EOF
-fi
 if [ "$check" ]; then
 	cat <<EOF
 for param in $check; do
-- 
2.7.0

Re: [PATCH kvm-unit-tests 02/12] scripts/mkstandalone: fix ARCH

[Paolo Bonzini]

On 19/01/2016 14:24, Radim Krčmář wrote:
> ARCH depends on the built image, not on target's arch.
> Use the value from config.mak.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  scripts/mkstandalone.sh | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 5f9ef146208e..47d6e279d212 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -53,17 +53,8 @@ function mkstandalone()
>  
>  	echo "#!/bin/sh"
>  
> -if [ "$arch" ]; then
> -	cat <<EOF
> -ARCH=\`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'\`
> -[ "\$ARCH" = "aarch64" ] && ARCH="arm64"
> -if [ "\$ARCH" != "$arch" ]; then
> -	echo "skip $testname ($arch only)" 1>&2
> -	exit 1
> -fi
> +	cat config.mak | grep '^ARCH='

Useless use of cat... so far nothing that I cannot fix myself.

Paolo

>  
> -EOF
> -fi
>  if [ "$check" ]; then
>  	cat <<EOF
>  for param in $check; do
> 

[PATCH kvm-unit-tests 03/12] scripts/mkstandalone: refactor temp file creation

[Radim Krčmář]

Using the cleanup variable is ugly, but I couldn't come up with a better
idea.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 47d6e279d212..a187adc856b7 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -20,6 +20,21 @@ fi
 unittests=$TEST_DIR/unittests.cfg
 mkdir -p tests
 
+temp_file ()
+{
+	local var="$1"
+	local file="$2"
+
+	echo "cleanup=\"\$$var \$cleanup\""
+	echo "$var=\`mktemp\`"
+	echo "base64 -d << 'BIN_EOF' | zcat > \$$var || exit 1"
+
+	gzip - < $file | base64
+
+	echo "BIN_EOF"
+	echo "chmod +x \$$var"
+}
+
 function mkstandalone()
 {
 	local testname="$1"
@@ -74,14 +89,11 @@ echo "skip $testname (test kernel not present)" 1>&2
 exit 1
 EOF
 else
+	echo "trap 'rm -f \$cleanup; exit 1' HUP INT TERM"
+
+	temp_file bin "$kernel"
+
 	cat <<EOF
-trap 'rm -f \$bin; exit 1' HUP INT TERM
-bin=\`mktemp\`
-base64 -d << 'BIN_EOF' | zcat > \$bin &&
-EOF
-gzip - < $kernel | base64
-	cat <<EOF
-BIN_EOF
 
 qemu="$qemu"
 if [ "\$QEMU" ]; then
-- 
2.7.0

Re: [PATCH kvm-unit-tests 03/12] scripts/mkstandalone: refactor temp file creation

[Paolo Bonzini]

On 19/01/2016 14:24, Radim Krčmář wrote:
> +
> +	echo "cleanup=\"\$$var \$cleanup\""
> +	echo "$var=\`mktemp\`"

$var is not yet defined when you update cleanup.

Paolo

Re: [PATCH kvm-unit-tests 03/12] scripts/mkstandalone: refactor temp file creation

[Radim Krčmář]

2016-01-19 15:46+0100, Paolo Bonzini:
> On 19/01/2016 14:24, Radim Krčmář wrote:
>> +
>> +	echo "cleanup=\"\$$var \$cleanup\""
>> +	echo "$var=\`mktemp\`"
> 
> $var is not yet defined when you update cleanup.

Ah, I missed that bug.
I'll add `unset var`, so we don't accidentaly delete something.

[PATCH kvm-unit-tests 04/12] scripts/mkstandalone: use bash in standalone test

[Radim Krčmář]

A New Hope.

Limiting ourselves to POSIX shell is what made mkstandalone very messy.

Bash knows EXIT trap, which always happens.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index a187adc856b7..9ab4b1920e50 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -66,7 +66,7 @@ function mkstandalone()
 	exec {tmpfd}<&1
 	exec > $standalone
 
-	echo "#!/bin/sh"
+	echo "#!/bin/bash"
 
 	cat config.mak | grep '^ARCH='
 
@@ -89,7 +89,7 @@ echo "skip $testname (test kernel not present)" 1>&2
 exit 1
 EOF
 else
-	echo "trap 'rm -f \$cleanup; exit 1' HUP INT TERM"
+	echo "trap 'rm -f \$cleanup' EXIT"
 
 	temp_file bin "$kernel"
 
@@ -122,7 +122,6 @@ if [ \$ret -le 1 ]; then
 else
 	echo FAIL $testname 1>&2
 fi
-rm -f \$bin
 exit 0
 EOF
 fi
-- 
2.7.0

[PATCH kvm-unit-tests 05/12] scripts/mkstandalone: reuse $TEST_DIR/run

[Radim Krčmář]

We can now use bash, so there is no need to use DRYRUN when we can use
the functionality directly.  This also gets rid of minor bugs, like not
checking if QEMU on the host is actually supported.

Another pleasant side-effect is instant `make standalone`.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 9ab4b1920e50..b2bc2f7d6c2e 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -55,13 +55,6 @@ function mkstandalone()
 	fi
 
 	standalone=tests/$testname
-	cmdline=$(DRYRUN=yes ACCEL=$accel ./$TEST_DIR-run $kernel)
-	if [ $? -ne 0 ]; then
-		echo $cmdline
-		exit 1
-	fi
-	qemu=$(cut -d' ' -f1 <<< "$cmdline")
-	cmdline=$(cut -d' ' -f2- <<< "$cmdline")
 
 	exec {tmpfd}<&1
 	exec > $standalone
@@ -92,31 +85,24 @@ else
 	echo "trap 'rm -f \$cleanup' EXIT"
 
 	temp_file bin "$kernel"
+	temp_file RUNTIME_arch_run "$TEST_DIR/run"
 
 	cat <<EOF
 
-qemu="$qemu"
-if [ "\$QEMU" ]; then
-	qemu="\$QEMU"
-fi
-
 MAX_SMP="MAX_SMP"
-echo \$qemu $cmdline -smp $smp $opts
+echo \$RUNTIME_arch_run \$bin -smp $smp $opts
 
-cmdline="\`echo '$cmdline' | sed s%$kernel%_NO_FILE_4Uhere_%\`"
-if \$qemu \$cmdline 2>&1 | grep 'No accelerator found'; then
+if \$RUNTIME_arch_run _NO_FILE_4Uhere_ 2>&1 | grep 'No accelerator found'; then
 	ret=2
 else
 	MAX_SMP=\`getconf _NPROCESSORS_CONF\`
-	while \$qemu \$cmdline -smp \$MAX_SMP 2>&1 | grep 'exceeds max cpus' > /dev/null; do
+	while \$RUNTIME_arch_run \$bin -smp \$MAX_SMP 2>&1 | grep 'exceeds max cpus' > /dev/null; do
 		MAX_SMP=\`expr \$MAX_SMP - 1\`
 	done
 
-	cmdline="\`echo '$cmdline' | sed s%$kernel%\$bin%\`"
-	\$qemu \$cmdline -smp $smp $opts
+	\$RUNTIME_arch_run \$bin -smp $smp $opts
 	ret=\$?
 fi
-echo Return value from qemu: \$ret
 if [ \$ret -le 1 ]; then
 	echo PASS $testname 1>&2
 else
-- 
2.7.0

[PATCH kvm-unit-tests 06/12] run_tests: move reusable code to scripts/runtime

[Radim Krčmář]

Standalone tests will use it.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 run_tests.sh         | 99 +---------------------------------------------------
 scripts/runtime.bash | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 98 deletions(-)
 create mode 100644 scripts/runtime.bash

diff --git a/run_tests.sh b/run_tests.sh
index fad22a935b00..2549679e653b 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -6,105 +6,8 @@ if [ ! -f config.mak ]; then
 fi
 source config.mak
 source scripts/functions.bash
+source scripts/runtime.bash
 
 config=$TEST_DIR/unittests.cfg
-qemu=${QEMU:-qemu-system-$ARCH}
-verbose=0
-
-function run()
-{
-    local testname="$1"
-    local groups="$2"
-    local smp="$3"
-    local kernel="$4"
-    local opts="$5"
-    local arch="$6"
-    local check="$7"
-    local accel="$8"
-
-    if [ -z "$testname" ]; then
-        return
-    fi
-
-    if [ -n "$only_group" ] && ! grep -q "$only_group" <<<$groups; then
-        return
-    fi
-
-    if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
-        echo "skip $1 ($arch only)"
-        return
-    fi
-
-    # check a file for a particular value before running a test
-    # the check line can contain multiple files to check separated by a space
-    # but each check parameter needs to be of the form <path>=<value>
-    for check_param in ${check[@]}; do
-        path=${check_param%%=*}
-        value=${check_param#*=}
-        if [ "$path" ] && [ "$(cat $path)" != "$value" ]; then
-            echo "skip $1 ($path not equal to $value)"
-            return
-        fi
-    done
-
-    cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp $smp $opts"
-    if [ $verbose != 0 ]; then
-        echo $cmdline
-    fi
-
-    # extra_params in the config file may contain backticks that need to be
-    # expanded, so use eval to start qemu
-    eval $cmdline >> test.log
-
-    if [ $? -le 1 ]; then
-        echo -e "\e[32mPASS\e[0m $1"
-    else
-        echo -e "\e[31mFAIL\e[0m $1"
-    fi
-}
-
-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
-}
-
-echo > test.log
-while getopts "g:hv" opt; do
-    case $opt in
-        g)
-            only_group=$OPTARG
-            ;;
-        h)
-            usage
-            exit
-            ;;
-        v)
-            verbose=1
-            ;;
-        *)
-            exit
-            ;;
-    esac
-done
-
-#
-# Probe for MAX_SMP
-#
-MAX_SMP=$(getconf _NPROCESSORS_CONF)
-while ./$TEST_DIR-run _NO_FILE_4Uhere_ -smp $MAX_SMP \
-		|& grep -q 'exceeds max cpus'; do
-	((--MAX_SMP))
-done
 
 for_each_unittest $config run
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
new file mode 100644
index 000000000000..8813b9ddbf1e
--- /dev/null
+++ b/scripts/runtime.bash
@@ -0,0 +1,98 @@
+qemu=${QEMU:-qemu-system-$ARCH}
+verbose=0
+
+function run()
+{
+    local testname="$1"
+    local groups="$2"
+    local smp="$3"
+    local kernel="$4"
+    local opts="$5"
+    local arch="$6"
+    local check="$7"
+    local accel="$8"
+
+    if [ -z "$testname" ]; then
+        return
+    fi
+
+    if [ -n "$only_group" ] && ! grep -q "$only_group" <<<$groups; then
+        return
+    fi
+
+    if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
+        echo "skip $1 ($arch only)"
+        return
+    fi
+
+    # check a file for a particular value before running a test
+    # the check line can contain multiple files to check separated by a space
+    # but each check parameter needs to be of the form <path>=<value>
+    for check_param in ${check[@]}; do
+        path=${check_param%%=*}
+        value=${check_param#*=}
+        if [ "$path" ] && [ "$(cat $path)" != "$value" ]; then
+            echo "skip $1 ($path not equal to $value)"
+            return
+        fi
+    done
+
+    cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp $smp $opts"
+    if [ $verbose != 0 ]; then
+        echo $cmdline
+    fi
+
+    # extra_params in the config file may contain backticks that need to be
+    # expanded, so use eval to start qemu
+    eval $cmdline >> test.log
+
+    if [ $? -le 1 ]; then
+        echo -e "\e[32mPASS\e[0m $1"
+    else
+        echo -e "\e[31mFAIL\e[0m $1"
+    fi
+}
+
+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
+}
+
+echo > test.log
+while getopts "g:hv" opt; do
+    case $opt in
+        g)
+            only_group=$OPTARG
+            ;;
+        h)
+            usage
+            exit
+            ;;
+        v)
+            verbose=1
+            ;;
+        *)
+            exit
+            ;;
+    esac
+done
+
+#
+# Probe for MAX_SMP
+#
+MAX_SMP=$(getconf _NPROCESSORS_CONF)
+while ./$TEST_DIR-run _NO_FILE_4Uhere_ -smp $MAX_SMP \
+		|& grep -q 'exceeds max cpus'; do
+	((--MAX_SMP))
+done
-- 
2.7.0

[PATCH kvm-unit-tests 07/12] scripts/runtime: change callsite of $TEST_DIR/run

[Radim Krčmář]

$TEST_DIR/run doesn't exist for standalone tests, they use
$RUNTIME_arch_run in its place.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 run_tests.sh         | 2 ++
 scripts/runtime.bash | 6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index 2549679e653b..b488949d219b 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -6,6 +6,8 @@ if [ ! -f config.mak ]; then
 fi
 source config.mak
 source scripts/functions.bash
+
+RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
 source scripts/runtime.bash
 
 config=$TEST_DIR/unittests.cfg
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 8813b9ddbf1e..defbef31377d 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -1,3 +1,5 @@
+[ "${RUNTIME_arch_run?}" ]
+
 qemu=${QEMU:-qemu-system-$ARCH}
 verbose=0
 
@@ -37,14 +39,14 @@ function run()
         fi
     done
 
-    cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp $smp $opts"
+    cmdline="TESTNAME=$testname ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
     if [ $verbose != 0 ]; then
         echo $cmdline
     fi
 
     # extra_params in the config file may contain backticks that need to be
     # expanded, so use eval to start qemu
-    eval $cmdline >> test.log
+    eval $cmdline
 
     if [ $? -le 1 ]; then
         echo -e "\e[32mPASS\e[0m $1"
-- 
2.7.0

Re: [PATCH kvm-unit-tests 07/12] scripts/runtime: change callsite of $TEST_DIR/run

[Paolo Bonzini]

On 19/01/2016 14:24, Radim Krčmář wrote:
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 8813b9ddbf1e..defbef31377d 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -1,3 +1,5 @@
> +[ "${RUNTIME_arch_run?}" ]

If you want magic, you might as well say

: "${RUNTIME_arch_run?}"

Paolo

Re: [PATCH kvm-unit-tests 07/12] scripts/runtime: change callsite of $TEST_DIR/run

[Radim Krčmář]

2016-01-19 15:38+0100, Paolo Bonzini:
> On 19/01/2016 14:24, Radim Krčmář wrote:
>> +++ b/scripts/runtime.bash
>> @@ -1,3 +1,5 @@
>> +[ "${RUNTIME_arch_run?}" ]
> 
> If you want magic, you might as well say
> 
> : "${RUNTIME_arch_run?}"

Much better, thanks.

[PATCH kvm-unit-tests 08/12] scripts/mkstandalone: refactor check for test kernel

[Radim Krčmář]

We can return early and it was practically a failure, so non-zero code
would be better. (We probably shouldn't generate a useless test, but
that's for later.)

Indentation will be fixed later.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index b2bc2f7d6c2e..70efe64af243 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -76,12 +76,12 @@ done
 
 EOF
 fi
-if [ ! -f $kernel ]; then
-	cat <<EOF
-echo "skip $testname (test kernel not present)" 1>&2
-exit 1
-EOF
-else
+	if [ ! -f $kernel ]; then
+		echo 'echo "skip '"$testname"' (test kernel not present)" 1>&2'
+		echo 'exit 1'
+	else
+	# XXX: bad indentation
+
 	echo "trap 'rm -f \$cleanup' EXIT"
 
 	temp_file bin "$kernel"
@@ -110,7 +110,8 @@ else
 fi
 exit 0
 EOF
-fi
+	fi
+
 	exec 1<&$tmpfd {tmpfd}<&-
 	chmod +x $standalone
 
-- 
2.7.0

[PATCH kvm-unit-tests 09/12] scripts/mkstandalone: use scripts/runtime

[Radim Krčmář]

Patch is based around a call to run with required parameters.

This brings a lot of changes to the behavior.  The only change that
could be considered as negative is that we don't print summary messages
to stderr.  The main benefit is that the final scripts/mkstandalone.sh
is fairly orthogonal, so we don't need to do every future change twice.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 57 ++++++++++++-------------------------------------
 1 file changed, 14 insertions(+), 43 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 70efe64af243..d9cfded36d5e 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -20,6 +20,13 @@ fi
 unittests=$TEST_DIR/unittests.cfg
 mkdir -p tests
 
+escape ()
+{
+	for arg in "${@}"; do
+		printf "%q " "$arg"; # XXX: trailing whitespace
+	done
+}
+
 temp_file ()
 {
 	local var="$1"
@@ -38,13 +45,7 @@ temp_file ()
 function mkstandalone()
 {
 	local testname="$1"
-	local groups="$2"
-	local smp="$3"
-	local kernel="$4"
-	local opts="$5"
-	local arch="$6"
-	local check="$7"
-	local accel="$8"
+	local args=( $(escape "${@}") )
 
 	if [ -z "$testname" ]; then
 		return 1
@@ -63,21 +64,8 @@ function mkstandalone()
 
 	cat config.mak | grep '^ARCH='
 
-if [ "$check" ]; then
-	cat <<EOF
-for param in $check; do
-	path=\`echo \$param | cut -d= -f1\`
-	value=\`echo \$param | cut -d= -f2\`
-	if [ -f "\$path" ] && [ "\`cat \$path\`" != "\$value" ]; then
-		echo "skip $testname (\$path not equal to \$value)" 1>&2
-		exit 1
-	fi
-done
-
-EOF
-fi
 	if [ ! -f $kernel ]; then
-		echo 'echo "skip '"$testname"' (test kernel not present)" 1>&2'
+		echo 'echo "skip '"$testname"' (test kernel not present)"'
 		echo 'exit 1'
 	else
 	# XXX: bad indentation
@@ -85,31 +73,14 @@ fi
 	echo "trap 'rm -f \$cleanup' EXIT"
 
 	temp_file bin "$kernel"
+	args[3]='$bin'
+
 	temp_file RUNTIME_arch_run "$TEST_DIR/run"
 
-	cat <<EOF
+	cat scripts/runtime.bash
 
-MAX_SMP="MAX_SMP"
-echo \$RUNTIME_arch_run \$bin -smp $smp $opts
-
-if \$RUNTIME_arch_run _NO_FILE_4Uhere_ 2>&1 | grep 'No accelerator found'; then
-	ret=2
-else
-	MAX_SMP=\`getconf _NPROCESSORS_CONF\`
-	while \$RUNTIME_arch_run \$bin -smp \$MAX_SMP 2>&1 | grep 'exceeds max cpus' > /dev/null; do
-		MAX_SMP=\`expr \$MAX_SMP - 1\`
-	done
-
-	\$RUNTIME_arch_run \$bin -smp $smp $opts
-	ret=\$?
-fi
-if [ \$ret -le 1 ]; then
-	echo PASS $testname 1>&2
-else
-	echo FAIL $testname 1>&2
-fi
-exit 0
-EOF
+	echo "run ${args[@]}"
+	echo "exit 0"
 	fi
 
 	exec 1<&$tmpfd {tmpfd}<&-
-- 
2.7.0

[PATCH kvm-unit-tests 10/12] scripts/mkstandalone: simplifiy redirection, again

[Radim Krčmář]

We don't have variables all over the place now, so it's nicer to move
script generator into a separate function and redirect its output.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index d9cfded36d5e..6ced8e5970ef 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -42,24 +42,10 @@ temp_file ()
 	echo "chmod +x \$$var"
 }
 
-function mkstandalone()
+generate_test ()
 {
-	local testname="$1"
 	local args=( $(escape "${@}") )
 
-	if [ -z "$testname" ]; then
-		return 1
-	fi
-
-	if [ -n "$one_testname" ] && [ "$testname" != "$one_testname" ]; then
-		return 1
-	fi
-
-	standalone=tests/$testname
-
-	exec {tmpfd}<&1
-	exec > $standalone
-
 	echo "#!/bin/bash"
 
 	cat config.mak | grep '^ARCH='
@@ -67,8 +53,8 @@ function mkstandalone()
 	if [ ! -f $kernel ]; then
 		echo 'echo "skip '"$testname"' (test kernel not present)"'
 		echo 'exit 1'
-	else
-	# XXX: bad indentation
+		return 1
+	fi
 
 	echo "trap 'rm -f \$cleanup' EXIT"
 
@@ -81,9 +67,24 @@ function mkstandalone()
 
 	echo "run ${args[@]}"
 	echo "exit 0"
+}
+
+function mkstandalone()
+{
+	local testname="$1"
+
+	if [ -z "$testname" ]; then
+		return 1
 	fi
 
-	exec 1<&$tmpfd {tmpfd}<&-
+	if [ -n "$one_testname" ] && [ "$testname" != "$one_testname" ]; then
+		return 1
+	fi
+
+	standalone=tests/$testname
+
+	generate_test "$@" > $standalone
+
 	chmod +x $standalone
 
 	return 0
-- 
2.7.0

[PATCH kvm-unit-tests 11/12] scripts/mkstandalone: sanitize one_kernel handling

[Radim Krčmář]

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 6ced8e5970ef..764c42d12e3b 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -7,19 +7,6 @@ fi
 source config.mak
 source scripts/functions.bash
 
-one_kernel="$1"
-[ "$one_kernel" ] && one_kernel_base=$(basename $one_kernel)
-one_testname="$2"
-if [ -n "$one_kernel" ] && [ ! -f $one_kernel ]; then
-	echo "$one_kernel doesn't exist"
-	exit 1
-elif [ -n "$one_kernel" ] && [ -z "$one_testname" ]; then
-	one_testname="${one_kernel_base%.*}"
-fi
-
-unittests=$TEST_DIR/unittests.cfg
-mkdir -p tests
-
 escape ()
 {
 	for arg in "${@}"; do
@@ -90,11 +77,21 @@ function mkstandalone()
 	return 0
 }
 
-trap 'rm -f $cfg; exit 1' HUP INT TERM
 trap 'rm -f $cfg' EXIT
 cfg=$(mktemp)
 
-if [ -n "$one_testname" ]; then
+unittests=$TEST_DIR/unittests.cfg
+one_kernel="$1"
+
+if [ "$one_kernel" ]; then
+	[ ! -f $one_kernel ] && {
+		echo "$one_kernel doesn't exist"
+		exit 1
+	}
+
+	one_kernel_base=$(basename $one_kernel)
+	one_testname="${2:-${one_kernel_base%.*}}"
+
 	if grep -q "\[$one_testname\]" $unittests; then
 		sed -n "/\\[$one_testname\\]/,/^\\[/p" $unittests \
 			| awk '!/^\[/ || NR == 1' > $cfg
@@ -106,4 +103,6 @@ else
 	cp -f $unittests $cfg
 fi
 
+mkdir -p tests
+
 for_each_unittest $cfg mkstandalone
-- 
2.7.0

[PATCH kvm-unit-tests 12/12] scripts/mkstandalone: return the exit value of a test

[Radim Krčmář]

This brings scripts/mkstandalone in line with $TEST_DIR/run.
The exit value doesn't make much sense and should be fixed.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/mkstandalone.sh | 1 -
 scripts/runtime.bash    | 5 ++++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 764c42d12e3b..05d86d281f68 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -53,7 +53,6 @@ generate_test ()
 	cat scripts/runtime.bash
 
 	echo "run ${args[@]}"
-	echo "exit 0"
 }
 
 function mkstandalone()
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index defbef31377d..15e3534cb52c 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -47,12 +47,15 @@ function run()
     # extra_params in the config file may contain backticks that need to be
     # expanded, so use eval to start qemu
     eval $cmdline
+    ret=$?
 
-    if [ $? -le 1 ]; then
+    if [ $ret -le 1 ]; then
         echo -e "\e[32mPASS\e[0m $1"
     else
         echo -e "\e[31mFAIL\e[0m $1"
     fi
+
+    return $ret
 }
 
 function usage()
-- 
2.7.0

Re: [PATCH kvm-unit-tests 00/12] scripts/mkstandalone: rewrite

[Paolo Bonzini]

On 19/01/2016 14:23, Radim Krčmář wrote:
> Drew and I have shelved series' where mkstandalone was duplicating the
> effort.  This patchset should make future changes much simpler by
> reusing code from other files in standalone tests.

Nice!  Tacked on the following at the end, and pushed.

commit c8104a66356d03d5dbb264422023297e8ec34652
Author: Paolo Bonzini <pbonzini@redhat.com>
Date:   Tue Jan 19 15:47:48 2016 +0100

    scripts/mkstandalone: document what it writes to

    Makes it a bit less puzzling. :)

    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 1e0da8e..5891005 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -71,6 +71,7 @@ function mkstandalone()
 	generate_test "$@" > $standalone

 	chmod +x $standalone
+	echo Written $standalone.

 	return 0
 }


Paolo

Re: [PATCH kvm-unit-tests 00/12] scripts/mkstandalone: rewrite

[Radim Krčmář]

2016-01-19 15:49+0100, Paolo Bonzini:
> On 19/01/2016 14:23, Radim Krčmář wrote:
>> Drew and I have shelved series' where mkstandalone was duplicating the
>> effort.  This patchset should make future changes much simpler by
>> reusing code from other files in standalone tests.
> 
> Nice!  Tacked on the following at the end, and pushed.

Thanks!  (Seems like I won't be doing a v2, then :])

> commit c8104a66356d03d5dbb264422023297e8ec34652
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date:   Tue Jan 19 15:47:48 2016 +0100
> 
>     scripts/mkstandalone: document what it writes to
> 
>     Makes it a bit less puzzling. :)

Definitely.

>     Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 1e0da8e..5891005 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -71,6 +71,7 @@ function mkstandalone()
>  	generate_test "$@" > $standalone
> 
>  	chmod +x $standalone
> +	echo Written $standalone.
> 
>  	return 0
>  }
> 
> 
> Paolo

Re: [PATCH kvm-unit-tests 00/12] scripts/mkstandalone: rewrite

[Andrew Jones]

On Tue, Jan 19, 2016 at 02:23:59PM +0100, Radim Krčmář wrote:
> Drew and I have shelved series' where mkstandalone was duplicating the
> effort.  This patchset should make future changes much simpler by
> reusing code from other files in standalone tests.

Thanks Radim! This definitely makes mkstandalone much, much more
maintainable. Long-live Bash :-)

I'll rework my return code and timeout support patches on top of this,
and post them soon.

Paolo,

I noticed you wrote 'applied', but I guess you haven't yet pushed.

Thanks,
drew

> 
> 
> Radim Krčmář (12):
>   scripts/mkstandalone: simplify redirection
>   scripts/mkstandalone: fix ARCH
>   scripts/mkstandalone: refactor temp file creation
>   scripts/mkstandalone: use bash in standalone test
>   scripts/mkstandalone: reuse $TEST_DIR/run
>   run_tests: move reusable code to scripts/runtime
>   scripts/runtime: change callsite of $TEST_DIR/run
>   scripts/mkstandalone: refactor check for test kernel
>   scripts/mkstandalone: use scripts/runtime
>   scripts/mkstandalone: simplifiy redirection, again
>   scripts/mkstandalone: sanitize one_kernel handling
>   scripts/mkstandalone: return the exit value of a test
> 
>  run_tests.sh            | 101 +----------------------------
>  scripts/mkstandalone.sh | 164 +++++++++++++++++++-----------------------------
>  scripts/runtime.bash    | 103 ++++++++++++++++++++++++++++++
>  3 files changed, 169 insertions(+), 199 deletions(-)
>  create mode 100644 scripts/runtime.bash
> 
> -- 
> 2.7.0
> 
> --
> 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