All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication
@ 2017-06-28 20:08 Radim Krčmář
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 1/5] arch/run: simplify copy-paste of the command line Radim Krčmář
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Radim Krčmář @ 2017-06-28 20:08 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

The series isn't well split, so patches remove the code duplication and
fix bugs at the same time.  [3/5] is s390x+powerpc specific, but has
dependency on [4/5], which affects all architectures, just like [1,5/5].
[2/5] is unrelated, but was simple enough to include.


Radim Krčmář (5):
  arch/run: simplify copy-paste of the command line
  run_tests: add summary to powerpc and s390
  s390+powerpc/run: improve output handling
  scripts/arch-run: fix qemu binary search failure path
  arch/run: unify accelerator detection

 arm/run               | 28 +++------------------
 powerpc/run           | 41 ++++--------------------------
 s390x/run             | 38 +++-------------------------
 scripts/arch-run.bash | 69 +++++++++++++++++++++++++++++++++++++++++++++++----
 scripts/runtime.bash  |  2 +-
 x86/run               |  8 +++---
 6 files changed, 81 insertions(+), 105 deletions(-)

-- 
2.13.2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [kvm-unit-tests PATCH 1/5] arch/run: simplify copy-paste of the command line
  2017-06-28 20:08 [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Radim Krčmář
@ 2017-06-28 20:08 ` Radim Krčmář
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 2/5] run_tests: add summary to powerpc and s390 Radim Krčmář
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Radim Krčmář @ 2017-06-28 20:08 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

The major annoyance coming from errata was "-initrd `mktemp`" that had
to be removed before running the copy-pasted command line.
Move "-initrd ..." part at the end of the line and hide it behind "#" if
the initird file is a temporary one.

This patch also brings s390 to use the same code as other architectures
and eliminates code duplication.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
Adding the arch/run command line might also be worth it
(i.e. adding echo $0 $@ to arch/run).
---
 arm/run               |  5 +----
 powerpc/run           |  5 +----
 s390x/run             |  2 --
 scripts/arch-run.bash | 18 ++++++++++++++++--
 x86/run               |  5 +----
 5 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/arm/run b/arm/run
index 1b1602c74970..8d8918a461dc 100755
--- a/arm/run
+++ b/arm/run
@@ -71,12 +71,9 @@ if $qemu $M -device '?' 2>&1 | grep pci-testdev > /dev/null; then
 	pci_testdev="-device pci-testdev"
 fi
 
-initrd_create
-
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults $M -cpu $processor $chr_testdev $pci_testdev"
-command+=" -display none -serial stdio $INITRD -kernel"
+command+=" -display none -serial stdio -kernel"
 command="$(timeout_cmd) $command"
-echo $command "$@"
 
 run_qemu $command "$@"
diff --git a/powerpc/run b/powerpc/run
index 7ccf1a36ff49..3fd5a91aeffb 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -35,14 +35,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
 	exit 2
 fi
 
-initrd_create
-
 M='-machine pseries'
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults $M -bios $FIRMWARE"
-command+=" -display none -serial stdio $INITRD -kernel"
+command+=" -display none -serial stdio -kernel"
 command="$(migration_cmd) $(timeout_cmd) $command"
-echo $command "$@"
 
 # powerpc tests currently exit with rtas-poweroff, which exits with 0.
 # run_qemu treats that as a failure exit and returns 1, so we need
diff --git a/s390x/run b/s390x/run
index cf333de9ccce..89210f4fb6b6 100755
--- a/s390x/run
+++ b/s390x/run
@@ -34,10 +34,8 @@ M='-machine s390-ccw-virtio'
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults -nographic $M"
 command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
-[ -f "$ENV" ] && command+=" -initrd $ENV"
 command+=" -kernel"
 command="$(timeout_cmd) $command"
-echo $command "$@"
 
 # We return the exit code via stdout, not via the QEMU return code
 lines=$(run_qemu $command "$@")
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 7c6c79ad3a9d..8ad37e4ebf97 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -28,9 +28,14 @@ run_qemu ()
 {
 	local stdout errors ret sig
 
+	echo -n $@
+	initrd_create &&
+		echo -n " #"
+	echo " $INITRD"
+
 	# stdout to {stdout}, stderr to $errors and stderr
 	exec {stdout}>&1
-	errors=$("${@}" </dev/null 2> >(tee /dev/stderr) > /dev/fd/$stdout)
+	errors=$("${@}" $INITRD </dev/null 2> >(tee /dev/stderr) > /dev/fd/$stdout)
 	ret=$?
 	exec {stdout}>&-
 
@@ -158,14 +163,20 @@ search_qemu_binary ()
 
 initrd_create ()
 {
+	local ret
+
 	env_add_errata
+	ret=$?
+
 	unset INITRD
 	[ -f "$ENV" ] && INITRD="-initrd $ENV"
+
+	return $ret
 }
 
 env_add_errata ()
 {
-	local line errata
+	local line errata ret=1
 
 	if [ -f "$ENV" ] && grep -q '^ERRATA_' <(env); then
 		for line in $(grep '^ERRATA_' "$ENV"); do
@@ -183,7 +194,10 @@ env_add_errata ()
 		trap_exit_push 'rm -f $ENV; [ "$ENV_OLD" ] && export ENV="$ENV_OLD" || unset ENV; unset ENV_OLD'
 		[ -f "$ENV_OLD" ] && grep -v '^ERRATA_' "$ENV_OLD" > $ENV
 		grep '^ERRATA_' <(env) >> $ENV
+		ret=0
 	fi
+
+	return $ret
 }
 
 env_generate_errata ()
diff --git a/x86/run b/x86/run
index 88f5ff771140..a06a7c0f80c8 100755
--- a/x86/run
+++ b/x86/run
@@ -33,11 +33,8 @@ else
 	pc_testdev="-device testdev,chardev=testlog -chardev file,id=testlog,path=msr.out"
 fi
 
-initrd_create
-
 command="${qemu} -nodefaults -enable-kvm $pc_testdev -vnc none -serial stdio $pci_testdev $hyperv_testdev"
-command+=" $INITRD -kernel"
+command+=" -kernel"
 command="$(timeout_cmd) $command"
-echo ${command} "$@"
 
 run_qemu ${command} "$@"
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [kvm-unit-tests PATCH 2/5] run_tests: add summary to powerpc and s390
  2017-06-28 20:08 [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Radim Krčmář
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 1/5] arch/run: simplify copy-paste of the command line Radim Krčmář
@ 2017-06-28 20:08 ` Radim Krčmář
  2017-06-29  5:30   ` Thomas Huth
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 3/5] s390+powerpc/run: improve output handling Radim Krčmář
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Radim Krčmář @ 2017-06-28 20:08 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

Looking at the last line was not enough as these architectures have
information about the exit on the last two lines.  See the last 3 lines.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
Sample output on powerpc:
  PASS selftest-setup (2 tests)
  PASS spapr_hcall (9 tests, 1 expected failures)
  PASS rtas-get-time-of-day (10 tests)
  PASS rtas-get-time-of-day-base (10 tests)
  PASS rtas-set-time-of-day (5 tests)
  PASS emulator (4 tests)
  SKIP h_cede_tm (test marked as manual run only)
  PASS sprs (46 tests)

and s390x:
  FAIL selftest-setup (7 tests)
  FAIL intercept (3 tests, 2 unexpected failures)

s390 selftest-setup is failing even though all tests passed, but there
is also:
  ABORT: selftest: Unexpected program interrupt: 5 at 0x12b7c, ilen 4
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index e630279cdd37..ac09e0fd30ac 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -8,7 +8,7 @@ FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
 
 extract_summary()
 {
-    tail -1 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/$/)/'
+    tail -3 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/$/)/'
 }
 
 # We assume that QEMU is going to work if it tried to load the kernel
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [kvm-unit-tests PATCH 3/5] s390+powerpc/run: improve output handling
  2017-06-28 20:08 [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Radim Krčmář
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 1/5] arch/run: simplify copy-paste of the command line Radim Krčmář
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 2/5] run_tests: add summary to powerpc and s390 Radim Krčmář
@ 2017-06-28 20:08 ` Radim Krčmář
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 4/5] scripts/arch-run: fix qemu binary search failure path Radim Krčmář
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Radim Krčmář @ 2017-06-28 20:08 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

If the test is hanging, then using arch/run would produce no output,
because of caching in a temporary variable.
Use the `tee` trick to save the output while passing it through.

They were also using the very same code.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 powerpc/run           | 15 +--------------
 s390x/run             | 15 +--------------
 scripts/arch-run.bash | 23 +++++++++++++++++++++++
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/powerpc/run b/powerpc/run
index 3fd5a91aeffb..3f69386c642a 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -46,17 +46,4 @@ command="$(migration_cmd) $(timeout_cmd) $command"
 # 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=$(run_qemu $command "$@")
-ret=$?
-echo "$lines"
-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
+run_qemu_status $command "$@"
diff --git a/s390x/run b/s390x/run
index 89210f4fb6b6..6df348a93783 100755
--- a/s390x/run
+++ b/s390x/run
@@ -38,17 +38,4 @@ command+=" -kernel"
 command="$(timeout_cmd) $command"
 
 # We return the exit code via stdout, not via the QEMU return code
-lines=$(run_qemu $command "$@")
-ret=$?
-echo "$lines"
-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
+run_qemu_status $command "$@"
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 8ad37e4ebf97..6e429a8748f4 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -69,6 +69,29 @@ run_qemu ()
 	return $ret
 }
 
+run_qemu_status ()
+{
+	local stdout ret
+
+	exec {stdout}>&1
+	lines=$(run_qemu "$@" > >(tee /dev/fd/$stdout))
+	ret=$?
+	exec {stdout}>&-
+
+	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
+
+	return $ret
+}
+
 timeout_cmd ()
 {
 	if [ "$TIMEOUT" ] && [ "$TIMEOUT" != "0" ]; then
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [kvm-unit-tests PATCH 4/5] scripts/arch-run: fix qemu binary search failure path
  2017-06-28 20:08 [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Radim Krčmář
                   ` (2 preceding siblings ...)
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 3/5] s390+powerpc/run: improve output handling Radim Krčmář
@ 2017-06-28 20:08 ` Radim Krčmář
  2017-06-28 20:46   ` Paolo Bonzini
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection Radim Krčmář
  2017-06-29 11:41 ` [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Andrew Jones
  5 siblings, 1 reply; 18+ messages in thread
From: Radim Krčmář @ 2017-06-28 20:08 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

Doing exit from a subshell won't exit the main shell, so arch-run will
try to continue without a working QEMU if search_qemu_binary() failed.

Also fix error message from search_qemu_binary().

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
I was thinking about using `set -e` instead, so the "return 2" from
search_qemu_binary() would stop the script without the explicit exit.
It required just four changes to make the script work (resulting in
better diffstat), but would likely complicate future development.

The second alternative would be to call just "search_qemu_binary" and
set the qemu variable from it.  This would allow "exit 2" from
search_qemu_binary() to also exit arch/run, but side-effects on
variables seem worse than the first alternative ...
---
 arm/run               | 3 ++-
 powerpc/run           | 3 ++-
 s390x/run             | 3 ++-
 scripts/arch-run.bash | 6 +++---
 x86/run               | 3 ++-
 5 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arm/run b/arm/run
index 8d8918a461dc..d7d778fe1645 100755
--- a/arm/run
+++ b/arm/run
@@ -31,7 +31,8 @@ if [ -z "$ACCEL" ]; then
 	fi
 fi
 
-qemu=$(search_qemu_binary)
+qemu=$(search_qemu_binary) ||
+	exit 2
 
 if ! $qemu -machine '?' 2>&1 | grep 'ARM Virtual Machine' > /dev/null; then
 	echo "$qemu doesn't support mach-virt ('-machine virt'). Exiting."
diff --git a/powerpc/run b/powerpc/run
index 3f69386c642a..e43c9fd8063c 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -28,7 +28,8 @@ if [ -z "$ACCEL" ]; then
 	fi
 fi
 
-qemu=$(search_qemu_binary)
+qemu=$(search_qemu_binary) ||
+	exit 2
 
 if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
 	echo "$qemu doesn't support pSeries ('-machine pseries'). Exiting."
diff --git a/s390x/run b/s390x/run
index 6df348a93783..ce8d45c2c8fd 100755
--- a/s390x/run
+++ b/s390x/run
@@ -28,7 +28,8 @@ if [ -z "$ACCEL" ]; then
 	fi
 fi
 
-qemu=$(search_qemu_binary)
+qemu=$(search_qemu_binary) ||
+	exit 2
 
 M='-machine s390-ccw-virtio'
 M+=",accel=$ACCEL"
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 6e429a8748f4..994c1aa9c0cd 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -176,9 +176,9 @@ search_qemu_binary ()
 	done
 
 	if [ -z "$qemu" ]; then
-		echo "A QEMU binary was not found."
-		echo "You can set a custom location by using the QEMU=<path> environment variable."
-		exit 2
+		echo "A QEMU binary was not found." >&2
+		echo "You can set a custom location by using the QEMU=<path> environment variable." >&2
+		return 2
 	fi
 	command -v $qemu
 	export PATH=$save_path
diff --git a/x86/run b/x86/run
index a06a7c0f80c8..1099c5ccd609 100755
--- a/x86/run
+++ b/x86/run
@@ -9,7 +9,8 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-qemu=$(search_qemu_binary)
+qemu=$(search_qemu_binary) ||
+	exit 2
 
 if ! ${qemu} -device '?' 2>&1 | grep -F -e \"testdev\" -e \"pc-testdev\" > /dev/null;
 then
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection
  2017-06-28 20:08 [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Radim Krčmář
                   ` (3 preceding siblings ...)
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 4/5] scripts/arch-run: fix qemu binary search failure path Radim Krčmář
@ 2017-06-28 20:08 ` Radim Krčmář
  2017-06-28 20:48   ` Paolo Bonzini
                     ` (2 more replies)
  2017-06-29 11:41 ` [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Andrew Jones
  5 siblings, 3 replies; 18+ messages in thread
From: Radim Krčmář @ 2017-06-28 20:08 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

Use the same mechanism as search_qemu_binary().

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arm/run               | 20 +-------------------
 powerpc/run           | 18 +-----------------
 s390x/run             | 18 +-----------------
 scripts/arch-run.bash | 22 ++++++++++++++++++++++
 4 files changed, 25 insertions(+), 53 deletions(-)

diff --git a/arm/run b/arm/run
index d7d778fe1645..f2e71eca022d 100755
--- a/arm/run
+++ b/arm/run
@@ -10,26 +10,8 @@ if [ -z "$STANDALONE" ]; then
 fi
 processor="$PROCESSOR"
 
-if [ -c /dev/kvm ]; then
-	if [ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]; then
-		kvm_available=yes
-	elif [ "$HOST" = "aarch64" ]; then
-		kvm_available=yes
-	fi
-fi
-
-if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "KVM is needed, but not available on this host"
+ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
 	exit 2
-fi
-
-if [ -z "$ACCEL" ]; then
-	if [ "$kvm_available" = "yes" ]; then
-		ACCEL="kvm"
-	else
-		ACCEL="tcg"
-	fi
-fi
 
 qemu=$(search_qemu_binary) ||
 	exit 2
diff --git a/powerpc/run b/powerpc/run
index e43c9fd8063c..55b987c5efc5 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -9,24 +9,8 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-if [ -c /dev/kvm ]; then
-	if [ "$HOST" = "ppc64" ] && [ "$ARCH" = "ppc64" ]; then
-		kvm_available=yes
-	fi
-fi
-
-if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "KVM is needed, but not available on this host"
+ACCEL=$(get_accel '[ "$HOST" = "ppc64" ] && [ "$ARCH" = "ppc64" ]') ||
 	exit 2
-fi
-
-if [ -z "$ACCEL" ]; then
-	if [ "$kvm_available" = "yes" ]; then
-		ACCEL="kvm"
-	else
-		ACCEL="tcg"
-	fi
-fi
 
 qemu=$(search_qemu_binary) ||
 	exit 2
diff --git a/s390x/run b/s390x/run
index ce8d45c2c8fd..67d6e88ea7e4 100755
--- a/s390x/run
+++ b/s390x/run
@@ -9,24 +9,8 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-if [ -c /dev/kvm ]; then
-	if [ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]; then
-		kvm_available=yes
-	fi
-fi
-
-if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "KVM is needed, but not available on this host"
+ACCEL=$(get_accel '[ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]') ||
 	exit 2
-fi
-
-if [ -z "$ACCEL" ]; then
-	if [ "$kvm_available" = "yes" ]; then
-		ACCEL="kvm"
-	else
-		ACCEL="tcg"
-	fi
-fi
 
 qemu=$(search_qemu_binary) ||
 	exit 2
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 994c1aa9c0cd..a696652c61b1 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -272,3 +272,25 @@ trap_exit_push ()
 	local old_exit=$(trap -p EXIT | sed "s/^[^']*'//;s/'[^']*$//")
 	trap -- "$1; $old_exit" EXIT
 }
+
+get_accel ()
+{
+	local kvm_available
+
+	if [ -c /dev/kvm ] && eval "$@"; then
+		kvm_available=yes
+	fi
+
+	if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
+		echo "KVM is needed, but not available on this host" >&2
+		return 2
+	fi
+
+	if [ "$ACCEL" ]; then
+		echo $ACCEL
+	elif [ "$kvm_available" = "yes" ]; then
+		echo kvm
+	else
+		echo tcg
+	fi
+}
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 4/5] scripts/arch-run: fix qemu binary search failure path
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 4/5] scripts/arch-run: fix qemu binary search failure path Radim Krčmář
@ 2017-06-28 20:46   ` Paolo Bonzini
  2017-06-29 12:32     ` Radim Krčmář
  0 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2017-06-28 20:46 UTC (permalink / raw)
  To: Radim Krčmář, kvm
  Cc: Drew Jones, Laurent Vivier, Thomas Huth, David Hildenbrand



On 28/06/2017 22:08, Radim Krčmář wrote:
> Doing exit from a subshell won't exit the main shell, so arch-run will
> try to continue without a working QEMU if search_qemu_binary() failed.

Should it be "exit $?"?

Apart from this looks good.  I can do the change.

Paolo

> Also fix error message from search_qemu_binary().
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> I was thinking about using `set -e` instead, so the "return 2" from
> search_qemu_binary() would stop the script without the explicit exit.
> It required just four changes to make the script work (resulting in
> better diffstat), but would likely complicate future development.
> 
> The second alternative would be to call just "search_qemu_binary" and
> set the qemu variable from it.  This would allow "exit 2" from
> search_qemu_binary() to also exit arch/run, but side-effects on
> variables seem worse than the first alternative ...
> ---

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection Radim Krčmář
@ 2017-06-28 20:48   ` Paolo Bonzini
  2017-06-29 12:45     ` Radim Krčmář
  2017-06-29 14:28   ` [kvm-unit-tests PATCH v2 " Radim Krčmář
  2017-06-29 14:32   ` [kvm-unit-tests PATCH v2 6/5] x86/run: support accelerator controls Radim Krčmář
  2 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2017-06-28 20:48 UTC (permalink / raw)
  To: Radim Krčmář, kvm
  Cc: Drew Jones, Laurent Vivier, Thomas Huth, David Hildenbrand



On 28/06/2017 22:08, Radim Krčmář wrote:
> -if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
> -	echo "KVM is needed, but not available on this host"
> +ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
>  	exit 2

Should x86 do the same as ARM (follow-up patch)?

And maybe the test could move from an argument of get_accel to a
function "arch_kvm_available" that returns 0 if available and 1 if not?

Paolo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 2/5] run_tests: add summary to powerpc and s390
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 2/5] run_tests: add summary to powerpc and s390 Radim Krčmář
@ 2017-06-29  5:30   ` Thomas Huth
  2017-06-29 12:53     ` Radim Krčmář
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Huth @ 2017-06-29  5:30 UTC (permalink / raw)
  To: Radim Krčmář, kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, David Hildenbrand

On 28.06.2017 22:08, Radim Krčmář wrote:
> Looking at the last line was not enough as these architectures have
> information about the exit on the last two lines.  See the last 3 lines.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> Sample output on powerpc:
>   PASS selftest-setup (2 tests)
>   PASS spapr_hcall (9 tests, 1 expected failures)
>   PASS rtas-get-time-of-day (10 tests)
>   PASS rtas-get-time-of-day-base (10 tests)
>   PASS rtas-set-time-of-day (5 tests)
>   PASS emulator (4 tests)
>   SKIP h_cede_tm (test marked as manual run only)
>   PASS sprs (46 tests)
> 
> and s390x:
>   FAIL selftest-setup (7 tests)
>   FAIL intercept (3 tests, 2 unexpected failures)
> 
> s390 selftest-setup is failing even though all tests passed, but there
> is also:
>   ABORT: selftest: Unexpected program interrupt: 5 at 0x12b7c, ilen 4

Please use the latest and greatest QEMU from the master branch for s390x
... there have been quite a lot of fixes since v2.9 and some of them are
required to get the kvm-unit-tests running properly.

 Thomas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication
  2017-06-28 20:08 [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Radim Krčmář
                   ` (4 preceding siblings ...)
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection Radim Krčmář
@ 2017-06-29 11:41 ` Andrew Jones
  5 siblings, 0 replies; 18+ messages in thread
From: Andrew Jones @ 2017-06-29 11:41 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: kvm, Paolo Bonzini, Laurent Vivier, Thomas Huth, David Hildenbrand

On Wed, Jun 28, 2017 at 10:08:52PM +0200, Radim Krčmář wrote:
> The series isn't well split, so patches remove the code duplication and
> fix bugs at the same time.  [3/5] is s390x+powerpc specific, but has
> dependency on [4/5], which affects all architectures, just like [1,5/5].
> [2/5] is unrelated, but was simple enough to include.
> 
> 
> Radim Krčmář (5):
>   arch/run: simplify copy-paste of the command line
>   run_tests: add summary to powerpc and s390
>   s390+powerpc/run: improve output handling
>   scripts/arch-run: fix qemu binary search failure path
>   arch/run: unify accelerator detection
> 
>  arm/run               | 28 +++------------------
>  powerpc/run           | 41 ++++--------------------------
>  s390x/run             | 38 +++-------------------------
>  scripts/arch-run.bash | 69 +++++++++++++++++++++++++++++++++++++++++++++++----
>  scripts/runtime.bash  |  2 +-
>  x86/run               |  8 +++---
>  6 files changed, 81 insertions(+), 105 deletions(-)
> 
> -- 
> 2.13.2
>

With Paolo's fixups, the series looks good to me. For the series

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 4/5] scripts/arch-run: fix qemu binary search failure path
  2017-06-28 20:46   ` Paolo Bonzini
@ 2017-06-29 12:32     ` Radim Krčmář
  0 siblings, 0 replies; 18+ messages in thread
From: Radim Krčmář @ 2017-06-29 12:32 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, Drew Jones, Laurent Vivier, Thomas Huth, David Hildenbrand

2017-06-28 22:46+0200, Paolo Bonzini:
> On 28/06/2017 22:08, Radim Krčmář wrote:
> > Doing exit from a subshell won't exit the main shell, so arch-run will
> > try to continue without a working QEMU if search_qemu_binary() failed.
> 
> Should it be "exit $?"?

Makes sense.  I was thinking about 'return 1' from search_qemu_binary(),
but never actually went from it.

> Apart from this looks good.  I can do the change.

Please do, thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection
  2017-06-28 20:48   ` Paolo Bonzini
@ 2017-06-29 12:45     ` Radim Krčmář
  2017-06-29 13:46       ` Radim Krčmář
  0 siblings, 1 reply; 18+ messages in thread
From: Radim Krčmář @ 2017-06-29 12:45 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, Drew Jones, Laurent Vivier, Thomas Huth, David Hildenbrand

2017-06-28 22:48+0200, Paolo Bonzini:
> On 28/06/2017 22:08, Radim Krčmář wrote:
> > -if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
> > -	echo "KVM is needed, but not available on this host"
> > +ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
> >  	exit 2
> 
> Should x86 do the same as ARM (follow-up patch)?

Yeah, I'll prepare it when we've agreed on the format.

> And maybe the test could move from an argument of get_accel to a
> function "arch_kvm_available" that returns 0 if available and 1 if not?

Right, the arm condition needs to be fixed as

  ([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) ||
  ([ "$HOST" = "aarch64" ] && [ "$ARCH" = arm -o "$ARCH" = arm64 ])

and it would look even worse in the argument ...

Would it be ok like this?

diff --git a/arm/run b/arm/run
index f2e71eca022d..82d36d0e83f8 100755
--- a/arm/run
+++ b/arm/run
@@ -10,7 +10,12 @@ if [ -z "$STANDALONE" ]; then
 fi
 processor="$PROCESSOR"
 
-ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
+arch_kvm_available ()
+{
+	([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]
+}
+
+ACCEL=$(get_accel) ||
 	exit 2
 
 qemu=$(search_qemu_binary) ||
diff --git a/powerpc/run b/powerpc/run
index 55b987c5efc5..9ae21f333205 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -9,7 +9,12 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-ACCEL=$(get_accel '[ "$HOST" = "ppc64" ] && [ "$ARCH" = "ppc64" ]') ||
+arch_kvm_available ()
+{
+	[ "$HOST" = "ppc64" ] && [ "$ARCH" = "ppc64" ]
+}
+
+ACCEL=$(get_accel) ||
 	exit 2
 
 qemu=$(search_qemu_binary) ||
diff --git a/s390x/run b/s390x/run
index 67d6e88ea7e4..361693ee4b84 100755
--- a/s390x/run
+++ b/s390x/run
@@ -9,7 +9,12 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-ACCEL=$(get_accel '[ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]') ||
+arch_kvm_available ()
+{
+	[ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]
+}
+
+ACCEL=$(get_accel) ||
 	exit 2
 
 qemu=$(search_qemu_binary) ||
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index a696652c61b1..6c6abc6b36bc 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -277,7 +277,7 @@ get_accel ()
 {
 	local kvm_available
 
-	if [ -c /dev/kvm ] && eval "$@"; then
+	if [ -c /dev/kvm ] && arch_kvm_available; then
 		kvm_available=yes
 	fi
 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 2/5] run_tests: add summary to powerpc and s390
  2017-06-29  5:30   ` Thomas Huth
@ 2017-06-29 12:53     ` Radim Krčmář
  0 siblings, 0 replies; 18+ messages in thread
From: Radim Krčmář @ 2017-06-29 12:53 UTC (permalink / raw)
  To: Thomas Huth
  Cc: kvm, Paolo Bonzini, Drew Jones, Laurent Vivier, David Hildenbrand

2017-06-29 07:30+0200, Thomas Huth:
> On 28.06.2017 22:08, Radim Krčmář wrote:
> >   FAIL selftest-setup (7 tests)
> >   FAIL intercept (3 tests, 2 unexpected failures)
> > 
> > s390 selftest-setup is failing even though all tests passed, but there
> > is also:
> >   ABORT: selftest: Unexpected program interrupt: 5 at 0x12b7c, ilen 4
> 
> Please use the latest and greatest QEMU from the master branch for s390x
> ... there have been quite a lot of fixes since v2.9 and some of them are
> required to get the kvm-unit-tests running properly.

Will do, thanks.  In this case, I wanted to demonstrate that the summary
gets printed and didn't care what it was. :)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection
  2017-06-29 12:45     ` Radim Krčmář
@ 2017-06-29 13:46       ` Radim Krčmář
  2017-06-29 13:49         ` Paolo Bonzini
  0 siblings, 1 reply; 18+ messages in thread
From: Radim Krčmář @ 2017-06-29 13:46 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, Drew Jones, Laurent Vivier, Thomas Huth, David Hildenbrand

2017-06-29 14:45+0200, Radim Krčmář:
> 2017-06-28 22:48+0200, Paolo Bonzini:
> > On 28/06/2017 22:08, Radim Krčmář wrote:
> > > -if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
> > > -	echo "KVM is needed, but not available on this host"
> > > +ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
> > >  	exit 2
> > 
> > Should x86 do the same as ARM (follow-up patch)?
> 
> Yeah, I'll prepare it when we've agreed on the format.
> 
> > And maybe the test could move from an argument of get_accel to a
> > function "arch_kvm_available" that returns 0 if available and 1 if not?
> 
> Right, the arm condition needs to be fixed as
> 
>   ([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) ||
>   ([ "$HOST" = "aarch64" ] && [ "$ARCH" = arm -o "$ARCH" = arm64 ])
> 
> and it would look even worse in the argument ...
> 
> Would it be ok like this?

No, we can make a better use of the $HOST variable:

diff --git a/arm/run b/arm/run
index f2e71eca022d..1245d42edae7 100755
--- a/arm/run
+++ b/arm/run
@@ -10,7 +10,7 @@ if [ -z "$STANDALONE" ]; then
 fi
 processor="$PROCESSOR"
 
-ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
+ACCEL=$(get_accel) ||
 	exit 2
 
 qemu=$(search_qemu_binary) ||
diff --git a/powerpc/run b/powerpc/run
index 55b987c5efc5..58921e81eb8f 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -9,7 +9,7 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-ACCEL=$(get_accel '[ "$HOST" = "ppc64" ] && [ "$ARCH" = "ppc64" ]') ||
+ACCEL=$(get_accel) ||
 	exit 2
 
 qemu=$(search_qemu_binary) ||
diff --git a/s390x/run b/s390x/run
index 67d6e88ea7e4..2242ebb79fbc 100755
--- a/s390x/run
+++ b/s390x/run
@@ -9,7 +9,7 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-ACCEL=$(get_accel '[ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]') ||
+ACCEL=$(get_accel) ||
 	exit 2
 
 qemu=$(search_qemu_binary) ||
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index a696652c61b1..77071216ff99 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -273,22 +273,26 @@ trap_exit_push ()
 	trap -- "$1; $old_exit" EXIT
 }
 
+kvm_available ()
+{
+	[ -c /dev/kvm ] ||
+		return 1
+
+	[ "$HOST" = "$ARCH_NAME" ] ||
+		[ "$HOST" = aarch64 -a "$ARCH" = arm ] ||
+		[ "$HOST" = x86_64 -a "$ARCH" = i386 ]
+}
+
 get_accel ()
 {
-	local kvm_available
-
-	if [ -c /dev/kvm ] && eval "$@"; then
-		kvm_available=yes
-	fi
-
-	if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
+	if [ "$ACCEL" = "kvm" ] && ! kvm_available; then
 		echo "KVM is needed, but not available on this host" >&2
 		return 2
 	fi
 
 	if [ "$ACCEL" ]; then
 		echo $ACCEL
-	elif [ "$kvm_available" = "yes" ]; then
+	elif kvm_available; then
 		echo kvm
 	else
 		echo tcg

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection
  2017-06-29 13:46       ` Radim Krčmář
@ 2017-06-29 13:49         ` Paolo Bonzini
  0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2017-06-29 13:49 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: kvm, Drew Jones, Laurent Vivier, Thomas Huth, David Hildenbrand



On 29/06/2017 15:46, Radim Krčmář wrote:
> 2017-06-29 14:45+0200, Radim Krčmář:
>> 2017-06-28 22:48+0200, Paolo Bonzini:
>>> On 28/06/2017 22:08, Radim Krčmář wrote:
>>>> -if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
>>>> -	echo "KVM is needed, but not available on this host"
>>>> +ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
>>>>  	exit 2
>>>
>>> Should x86 do the same as ARM (follow-up patch)?
>>
>> Yeah, I'll prepare it when we've agreed on the format.
>>
>>> And maybe the test could move from an argument of get_accel to a
>>> function "arch_kvm_available" that returns 0 if available and 1 if not?
>>
>> Right, the arm condition needs to be fixed as
>>
>>   ([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) ||
>>   ([ "$HOST" = "aarch64" ] && [ "$ARCH" = arm -o "$ARCH" = arm64 ])
>>
>> and it would look even worse in the argument ...
>>
>> Would it be ok like this?
> 
> No, we can make a better use of the $HOST variable:

Either is okay.

Paolo

> diff --git a/arm/run b/arm/run
> index f2e71eca022d..1245d42edae7 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -10,7 +10,7 @@ if [ -z "$STANDALONE" ]; then
>  fi
>  processor="$PROCESSOR"
>  
> -ACCEL=$(get_accel '([ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]) || [ "$HOST" = "aarch64" ]') ||
> +ACCEL=$(get_accel) ||
>  	exit 2
>  
>  qemu=$(search_qemu_binary) ||
> diff --git a/powerpc/run b/powerpc/run
> index 55b987c5efc5..58921e81eb8f 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -9,7 +9,7 @@ if [ -z "$STANDALONE" ]; then
>  	source scripts/arch-run.bash
>  fi
>  
> -ACCEL=$(get_accel '[ "$HOST" = "ppc64" ] && [ "$ARCH" = "ppc64" ]') ||
> +ACCEL=$(get_accel) ||
>  	exit 2
>  
>  qemu=$(search_qemu_binary) ||
> diff --git a/s390x/run b/s390x/run
> index 67d6e88ea7e4..2242ebb79fbc 100755
> --- a/s390x/run
> +++ b/s390x/run
> @@ -9,7 +9,7 @@ if [ -z "$STANDALONE" ]; then
>  	source scripts/arch-run.bash
>  fi
>  
> -ACCEL=$(get_accel '[ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]') ||
> +ACCEL=$(get_accel) ||
>  	exit 2
>  
>  qemu=$(search_qemu_binary) ||
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index a696652c61b1..77071216ff99 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -273,22 +273,26 @@ trap_exit_push ()
>  	trap -- "$1; $old_exit" EXIT
>  }
>  
> +kvm_available ()
> +{
> +	[ -c /dev/kvm ] ||
> +		return 1
> +
> +	[ "$HOST" = "$ARCH_NAME" ] ||
> +		[ "$HOST" = aarch64 -a "$ARCH" = arm ] ||
> +		[ "$HOST" = x86_64 -a "$ARCH" = i386 ]
> +}
> +
>  get_accel ()
>  {
> -	local kvm_available
> -
> -	if [ -c /dev/kvm ] && eval "$@"; then
> -		kvm_available=yes
> -	fi
> -
> -	if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
> +	if [ "$ACCEL" = "kvm" ] && ! kvm_available; then
>  		echo "KVM is needed, but not available on this host" >&2
>  		return 2
>  	fi
>  
>  	if [ "$ACCEL" ]; then
>  		echo $ACCEL
> -	elif [ "$kvm_available" = "yes" ]; then
> +	elif kvm_available; then
>  		echo kvm
>  	else
>  		echo tcg
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [kvm-unit-tests PATCH v2 5/5] arch/run: unify accelerator detection
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection Radim Krčmář
  2017-06-28 20:48   ` Paolo Bonzini
@ 2017-06-29 14:28   ` Radim Krčmář
  2017-06-29 14:32   ` [kvm-unit-tests PATCH v2 6/5] x86/run: support accelerator controls Radim Krčmář
  2 siblings, 0 replies; 18+ messages in thread
From: Radim Krčmář @ 2017-06-29 14:28 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

Use the same mechanism as search_qemu_binary().

The simplified [ "$HOST" = "$ARCH_NAME" ] check will mishandle a case if
you compile for $arch on $arch then try to run it with $other_arch/run,
but that isn't really worth checking.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arm/run               | 22 ++--------------------
 powerpc/run           | 20 ++------------------
 s390x/run             | 20 ++------------------
 scripts/arch-run.bash | 26 ++++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 56 deletions(-)

diff --git a/arm/run b/arm/run
index 6fc9bb6bb111..fd280ee19837 100755
--- a/arm/run
+++ b/arm/run
@@ -10,26 +10,8 @@ if [ -z "$STANDALONE" ]; then
 fi
 processor="$PROCESSOR"
 
-if [ -c /dev/kvm ]; then
-	if [ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]; then
-		kvm_available=yes
-	elif [ "$HOST" = "aarch64" ]; then
-		kvm_available=yes
-	fi
-fi
-
-if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "KVM is needed, but not available on this host"
-	exit 2
-fi
-
-if [ -z "$ACCEL" ]; then
-	if [ "$kvm_available" = "yes" ]; then
-		ACCEL="kvm"
-	else
-		ACCEL="tcg"
-	fi
-fi
+ACCEL=$(get_qemu_accelerator) ||
+	exit $?
 
 qemu=$(search_qemu_binary) ||
 	exit $?
diff --git a/powerpc/run b/powerpc/run
index 288504e0c330..597ab96ed8a8 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -9,24 +9,8 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-if [ -c /dev/kvm ]; then
-	if [ "$HOST" = "ppc64" ] && [ "$ARCH" = "ppc64" ]; then
-		kvm_available=yes
-	fi
-fi
-
-if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "KVM is needed, but not available on this host"
-	exit 2
-fi
-
-if [ -z "$ACCEL" ]; then
-	if [ "$kvm_available" = "yes" ]; then
-		ACCEL="kvm"
-	else
-		ACCEL="tcg"
-	fi
-fi
+ACCEL=$(get_qemu_accelerator) ||
+	exit $?
 
 qemu=$(search_qemu_binary) ||
 	exit $?
diff --git a/s390x/run b/s390x/run
index af22e8b269d6..0980504448ce 100755
--- a/s390x/run
+++ b/s390x/run
@@ -9,24 +9,8 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
-if [ -c /dev/kvm ]; then
-	if [ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]; then
-		kvm_available=yes
-	fi
-fi
-
-if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "KVM is needed, but not available on this host"
-	exit 2
-fi
-
-if [ -z "$ACCEL" ]; then
-	if [ "$kvm_available" = "yes" ]; then
-		ACCEL="kvm"
-	else
-		ACCEL="tcg"
-	fi
-fi
+ACCEL=$(get_qemu_accelerator) ||
+	exit $?
 
 qemu=$(search_qemu_binary) ||
 	exit $?
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 994c1aa9c0cd..afad43d2d1ad 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -272,3 +272,29 @@ trap_exit_push ()
 	local old_exit=$(trap -p EXIT | sed "s/^[^']*'//;s/'[^']*$//")
 	trap -- "$1; $old_exit" EXIT
 }
+
+kvm_available ()
+{
+	[ -c /dev/kvm ] ||
+		return 1
+
+	[ "$HOST" = "$ARCH_NAME" ] ||
+		[ "$HOST" = aarch64 -a "$ARCH" = arm ] ||
+		[ "$HOST" = x86_64 -a "$ARCH" = i386 ]
+}
+
+get_qemu_accelerator ()
+{
+	if [ "$ACCEL" = "kvm" ] && ! kvm_available; then
+		echo "KVM is needed, but not available on this host" >&2
+		return 2
+	fi
+
+	if [ "$ACCEL" ]; then
+		echo $ACCEL
+	elif kvm_available; then
+		echo kvm
+	else
+		echo tcg
+	fi
+}
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [kvm-unit-tests PATCH v2 6/5] x86/run: support accelerator controls
  2017-06-28 20:08 ` [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection Radim Krčmář
  2017-06-28 20:48   ` Paolo Bonzini
  2017-06-29 14:28   ` [kvm-unit-tests PATCH v2 " Radim Krčmář
@ 2017-06-29 14:32   ` Radim Krčmář
  2017-06-30 10:42     ` Paolo Bonzini
  2 siblings, 1 reply; 18+ messages in thread
From: Radim Krčmář @ 2017-06-29 14:32 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Drew Jones, Laurent Vivier, Thomas Huth,
	David Hildenbrand

It's easy to support and we do have the option for other architectures.

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

diff --git a/x86/run b/x86/run
index f319b68ef693..e59c6c590a20 100755
--- a/x86/run
+++ b/x86/run
@@ -9,6 +9,9 @@ if [ -z "$STANDALONE" ]; then
 	source scripts/arch-run.bash
 fi
 
+ACCEL=$(get_qemu_accelerator) ||
+	exit $?
+
 qemu=$(search_qemu_binary) ||
 	exit $?
 
@@ -34,8 +37,8 @@ else
 	pc_testdev="-device testdev,chardev=testlog -chardev file,id=testlog,path=msr.out"
 fi
 
-command="${qemu} -nodefaults -enable-kvm $pc_testdev -vnc none -serial stdio $pci_testdev $hyperv_testdev"
-command+=" -kernel"
+command="${qemu} -nodefaults $pc_testdev -vnc none -serial stdio $pci_testdev $hyperv_testdev"
+command+=" -machine accel=$ACCEL -kernel"
 command="$(timeout_cmd) $command"
 
 run_qemu ${command} "$@"
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [kvm-unit-tests PATCH v2 6/5] x86/run: support accelerator controls
  2017-06-29 14:32   ` [kvm-unit-tests PATCH v2 6/5] x86/run: support accelerator controls Radim Krčmář
@ 2017-06-30 10:42     ` Paolo Bonzini
  0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2017-06-30 10:42 UTC (permalink / raw)
  To: Radim Krčmář, kvm
  Cc: Drew Jones, Laurent Vivier, Thomas Huth, David Hildenbrand

Applied both, thanks.

Paolo

On 29/06/2017 16:32, Radim Krčmář wrote:
> It's easy to support and we do have the option for other architectures.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  x86/run | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/run b/x86/run
> index f319b68ef693..e59c6c590a20 100755
> --- a/x86/run
> +++ b/x86/run
> @@ -9,6 +9,9 @@ if [ -z "$STANDALONE" ]; then
>  	source scripts/arch-run.bash
>  fi
>  
> +ACCEL=$(get_qemu_accelerator) ||
> +	exit $?
> +
>  qemu=$(search_qemu_binary) ||
>  	exit $?
>  
> @@ -34,8 +37,8 @@ else
>  	pc_testdev="-device testdev,chardev=testlog -chardev file,id=testlog,path=msr.out"
>  fi
>  
> -command="${qemu} -nodefaults -enable-kvm $pc_testdev -vnc none -serial stdio $pci_testdev $hyperv_testdev"
> -command+=" -kernel"
> +command="${qemu} -nodefaults $pc_testdev -vnc none -serial stdio $pci_testdev $hyperv_testdev"
> +command+=" -machine accel=$ACCEL -kernel"
>  command="$(timeout_cmd) $command"
>  
>  run_qemu ${command} "$@"
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-06-30 10:42 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-28 20:08 [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Radim Krčmář
2017-06-28 20:08 ` [kvm-unit-tests PATCH 1/5] arch/run: simplify copy-paste of the command line Radim Krčmář
2017-06-28 20:08 ` [kvm-unit-tests PATCH 2/5] run_tests: add summary to powerpc and s390 Radim Krčmář
2017-06-29  5:30   ` Thomas Huth
2017-06-29 12:53     ` Radim Krčmář
2017-06-28 20:08 ` [kvm-unit-tests PATCH 3/5] s390+powerpc/run: improve output handling Radim Krčmář
2017-06-28 20:08 ` [kvm-unit-tests PATCH 4/5] scripts/arch-run: fix qemu binary search failure path Radim Krčmář
2017-06-28 20:46   ` Paolo Bonzini
2017-06-29 12:32     ` Radim Krčmář
2017-06-28 20:08 ` [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection Radim Krčmář
2017-06-28 20:48   ` Paolo Bonzini
2017-06-29 12:45     ` Radim Krčmář
2017-06-29 13:46       ` Radim Krčmář
2017-06-29 13:49         ` Paolo Bonzini
2017-06-29 14:28   ` [kvm-unit-tests PATCH v2 " Radim Krčmář
2017-06-29 14:32   ` [kvm-unit-tests PATCH v2 6/5] x86/run: support accelerator controls Radim Krčmář
2017-06-30 10:42     ` Paolo Bonzini
2017-06-29 11:41 ` [kvm-unit-tests PATCH 0/5] Minor fixes while getting rid of code duplication Andrew Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.