Re: [kvm-unit-tests PATCH 5/5] arch/run: unify accelerator detection

[Paolo Bonzini <pbonzini@redhat.com>]

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
>