From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection
Date: Thu, 29 Jun 2017 14:45:50 +0200
Message-ID: <20170629124549.GB10661@potion>
References: <20170628200857.1718-1-rkrcmar@redhat.com>
 <20170628200857.1718-6-rkrcmar@redhat.com>
 <13f36817-1957-0c9d-2c7e-86850b695c53@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: kvm@vger.kernel.org, Drew Jones <drjones@redhat.com>,
        Laurent Vivier <lvivier@redhat.com>,
        Thomas Huth <thuth@redhat.com>,
        David Hildenbrand <david@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:33536 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751893AbdF2Mp4 (ORCPT <rfc822;kvm@vger.kernel.org>);
        Thu, 29 Jun 2017 08:45:56 -0400
Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])
        (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
        (No client certificate requested)
        by mx1.redhat.com (Postfix) with ESMTPS id 50CE33345B4
        for <kvm@vger.kernel.org>; Thu, 29 Jun 2017 12:45:56 +0000 (UTC)
Content-Disposition: inline
In-Reply-To: <13f36817-1957-0c9d-2c7e-86850b695c53@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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