[kvm-unit-tests PATCH v3 3/6] arch-run: reduce return code ambiguity

[Andrew Jones <drjones@redhat.com>]

qemu/unittest exit codes are convoluted, causing codes 0 and 1
to be ambiguous. Here are the possible meanings

 .-----------------------------------------------------------------.
 |                | 0                                 | 1          |
 |-----------------------------------------------------------------|
 | QEMU           | did something successfully,       | FAILURE    |
 |                | but probably didn't run the       |            |
 |                | unittest, OR caught SIGINT,       |            |
 |                | SIGHUP, or SIGTERM                |            |
 |-----------------------------------------------------------------|
 | unittest       | for some reason exited using      | SUCCESS    |
 |                | ACPI/PSCI, not with debug-exit    |            |
 .-----------------------------------------------------------------.

As we can see above, an exit code of zero is even ambiguous for each
row, i.e. QEMU could exit with zero because it successfully completed,
or because it caught a signal. unittest could exit with zero because
it successfully powered-off, or because for some odd reason it powered-
off instead of calling debug-exit.

And, the most fun is that exit-code == 1 means QEMU failed, but the
unittest succeeded.

This patch attempts to reduce that ambiguity, by also looking at stderr.

Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

---
v3: now also powerpc/run (which is a bit different due to already having
    one exit code fixup in place)

 arm/run                 |  6 ++---
 powerpc/run             | 23 +++++++++++++++----
 scripts/arch-run.bash   | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
 scripts/mkstandalone.sh |  2 +-
 scripts/runtime.bash    |  2 +-
 x86/README              | 10 ++++----
 x86/run                 |  7 +++---
 7 files changed, 92 insertions(+), 19 deletions(-)
 create mode 100644 scripts/arch-run.bash

diff --git a/arm/run b/arm/run
index dc0a33204c20b..c9463de6dd241 100755
--- a/arm/run
+++ b/arm/run
@@ -6,6 +6,7 @@ if [ -z "$STANDALONE" ]; then
 		exit 2
 	fi
 	source config.mak
+	source scripts/arch-run.bash
 fi
 processor="$PROCESSOR"
 
@@ -71,7 +72,4 @@ command="$qemu $M -cpu $processor $chr_testdev"
 command+=" -display none -serial stdio -kernel"
 echo $command "$@"
 
-$command "$@"
-ret=$?
-echo Return value from qemu: $ret
-exit $ret
+exit_fixup $command "$@"
diff --git a/powerpc/run b/powerpc/run
index 45492a1cb8afc..8ac684cc7c12f 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -6,6 +6,7 @@ if [ -z "$STANDALONE" ]; then
 		exit 2
 	fi
 	source config.mak
+	source scripts/arch-run.bash
 fi
 
 if [ -c /dev/kvm ]; then
@@ -46,10 +47,22 @@ command="$qemu $M -bios $FIRMWARE"
 command+=" -display none -serial stdio -kernel"
 echo $command "$@"
 
-#FIXME: rtas-poweroff always exits with zero, so we have to parse
-#       the true exit code from the output.
-lines=$($command "$@")
+# powerpc tests currently exit with rtas-poweroff, which exits with 0.
+# exit_fixup treats that as a failure exit and returns 1, so we need
+# to fixup the fixup below by parsing the true exit code from the output.
+# The second fixup is also a FIXME, because once we add chr-testdev
+# support for powerpc, we won't need the second fixup.
+lines=$(exit_fixup $command "$@")
+ret=$?
 echo "$lines"
-ret=$(grep '^EXIT: ' <<<"$lines" | sed 's/.*STATUS=\([0-9][0-9]*\).*/\1/')
-echo Return value from qemu: $ret
+if [ $ret -eq 1 ]; then
+	testret=$(grep '^EXIT: ' <<<"$lines" | sed 's/.*STATUS=\([0-9][0-9]*\).*/\1/')
+	if [ "$testret" ]; then
+		if [ $testret -eq 1 ]; then
+			ret=0
+		else
+			ret=$testret
+		fi
+	fi
+fi
 exit $ret
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
new file mode 100644
index 0000000000000..3a63ed179a7de
--- /dev/null
+++ b/scripts/arch-run.bash
@@ -0,0 +1,61 @@
+##############################################################################
+# exit_fixup translates the ambiguous exit status in Table1 to that in Table2.
+# Table3 simply documents the complete status table.
+#
+# Table1: Before fixup
+# --------------------
+# 0      - Unexpected exit from QEMU (possible signal), or the unittest did
+#          not use debug-exit
+# 1      - most likely unittest succeeded, or QEMU failed
+#
+# Table2: After fixup
+# -------------------
+# 0      - Everything succeeded
+# 1      - most likely QEMU failed
+#
+# Table3: Complete table
+# ----------------------
+# 0      - SUCCESS
+# 1      - most likely QEMU failed
+# 2      - most likely a run script failed
+# 3      - most likely the unittest failed
+# 127    - most likely the unittest called abort()
+# 1..127 - FAILURE (could be QEMU, a run script, or the unittest)
+# >= 128 - Signal (signum = status - 128)
+##############################################################################
+exit_fixup ()
+{
+	local stdout errors ret sig
+
+	exec {stdout}>&1
+	errors=$("${@}" 2>&1 1>&${stdout} | tee >(cat - 1>&2); exit ${PIPESTATUS[0]})
+	ret=$?
+	exec {stdout}>&-
+
+	if [ "$errors" ]; then
+		sig=$(grep 'terminating on signal' <<<"$errors")
+		if [ "$sig" ]; then
+			sig=$(sed 's/.*terminating on signal \([0-9][0-9]*\).*/\1/' <<<"$sig")
+		fi
+	fi
+
+	if [ $ret -eq 0 ]; then
+		# Some signals result in a zero return status, but the
+		# error log tells the truth.
+		if [ "$sig" ]; then
+			((ret=sig+128))
+		else
+			# Exiting with zero (non-debugexit) is an error
+			ret=1
+		fi
+	elif [ $ret -eq 1 ]; then
+		# Even when ret==1 (unittest success) if we also got stderr
+		# logs, then we assume a QEMU failure. Otherwise we translate
+		# status of 1 to 0 (SUCCESS)
+		if [ -z "$errors" ]; then
+			ret=0
+		fi
+	fi
+
+	return $ret
+}
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 408bb05480b11..4c201aa9a4743 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -61,7 +61,7 @@ generate_test ()
 	temp_file bin "$kernel"
 	args[3]='$bin'
 
-	temp_file RUNTIME_arch_run "$TEST_DIR/run"
+	temp_file RUNTIME_arch_run <(echo "#!/bin/bash"; cat scripts/arch-run.bash "$TEST_DIR/run")
 
 	cat scripts/runtime.bash
 
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 04adb9d070f13..9c973f26f8de6 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -48,7 +48,7 @@ function run()
     eval $cmdline
     ret=$?
 
-    if [ $ret -le 1 ]; then
+    if [ $ret -eq 0 ]; then
         echo -e "\e[32mPASS\e[0m $1"
     else
         echo -e "\e[31mFAIL\e[0m $1"
diff --git a/x86/README b/x86/README
index 996ed33546d62..218fe1a1c6f1a 100644
--- a/x86/README
+++ b/x86/README
@@ -41,7 +41,9 @@ Tests in this directory and what they do:
  pcid:		basic functionality test of PCID/INVPCID feature
 
 Legacy notes:
- The exit status of the binary (and the script) is inconsistent: with
- qemu-system, after the unit-test is done, the exit status of qemu is 1,
- different from the 'old style' qemu-kvm, whose exit status in successful
- completion is 0.
+ The exit status of the binary is inconsistent; with qemu-system, after
+ the unit-test is done, the exit status of qemu is 1, different from the
+ 'old style' qemu-kvm, whose exit status in successful completion is 0.
+ The run script converts the qemu-system exit status to 0 (SUCCESS), and
+ treats the legacy exit status of 0 as an error, converting it to an exit
+ status of 1.
diff --git a/x86/run b/x86/run
index 22d7f2cad0d06..b07c815c5a6e1 100755
--- a/x86/run
+++ b/x86/run
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+[ -z "$STANDALONE" ] && source scripts/arch-run.bash
+
 qemubinarysearch="${QEMU:-qemu-kvm qemu-system-x86_64}"
 
 for qemucmd in ${qemubinarysearch}
@@ -43,7 +45,4 @@ fi
 command="${qemu} -enable-kvm $pc_testdev -vnc none -serial stdio $pci_testdev $hyperv_testdev -kernel"
 echo ${command} "$@"
 
-${command} "$@"
-ret=$?
-echo Return value from qemu: $ret
-exit $ret
+exit_fixup ${command} "$@"
-- 
2.4.3