From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection Date: Thu, 29 Jun 2017 15:49:37 +0200 Message-ID: <7f4b57cf-4283-2ce3-6358-c38bbf87bd69@redhat.com> References: <20170628200857.1718-1-rkrcmar@redhat.com> <20170628200857.1718-6-rkrcmar@redhat.com> <13f36817-1957-0c9d-2c7e-86850b695c53@redhat.com> <20170629124549.GB10661@potion> <20170629134657.GA14932@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, Drew Jones , Laurent Vivier , Thomas Huth , David Hildenbrand To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mail-wm0-f53.google.com ([74.125.82.53]:36649 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752698AbdF2Ntl (ORCPT ); Thu, 29 Jun 2017 09:49:41 -0400 Received: by mail-wm0-f53.google.com with SMTP id 62so83133575wmw.1 for ; Thu, 29 Jun 2017 06:49:41 -0700 (PDT) In-Reply-To: <20170629134657.GA14932@potion> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: 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 >