x86-run: Print a meaningful message if the qemu binary isn't found

Commit Message

Bandan Das April 9, 2015, 5:14 p.m. UTC

Paolo Bonzini <[e-mail]> writes:
...
> This breaks "QEMU=... ./x86-run ./x86/msr.flat"
>
> Fixed as follows:
>
> diff --git a/x86/run b/x86/run
> index d5adf8d..219a93b 100755
> --- a/x86/run
> +++ b/x86/run
> @@ -20,16 +20,15 @@ else
>  		    break
>  		fi
>  	done
> -fi
> -
> -if      [ -z "${QEMUFOUND}" ]
> -then
> -	echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable "
> -	exit ${NOTFOUND}
> -elif    [ -z "${qemu}" ]
> -then
> -	echo "No Qemu test device support found"
> -	exit ${TESTDEVNOTSUPP}
> +	if      [ -z "${QEMUFOUND}" ]
> +	then
> +		echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable "
> +		exit ${NOTFOUND}
> +	elif    [ -z "${qemu}" ]
> +	then
> +		echo "No Qemu test device support found"

I think we still want to print this if the user supplied qemu has no test
device support. I think this is better -

> +		exit ${TESTDEVNOTSUPP}
> +	fi
>  fi
>  
>  if
>
> which apart from reindentation is just moving a "fi" down.
> Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paolo Bonzini April 9, 2015, 5:17 p.m. UTC | #1

On 09/04/2015 19:14, Bandan Das wrote:
> Paolo Bonzini <[e-mail]> writes:
> ...
>> This breaks "QEMU=... ./x86-run ./x86/msr.flat"
>>
>> Fixed as follows:
>>
>> diff --git a/x86/run b/x86/run
>> index d5adf8d..219a93b 100755
>> --- a/x86/run
>> +++ b/x86/run
>> @@ -20,16 +20,15 @@ else
>>  		    break
>>  		fi
>>  	done
>> -fi
>> -
>> -if      [ -z "${QEMUFOUND}" ]
>> -then
>> -	echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable "
>> -	exit ${NOTFOUND}
>> -elif    [ -z "${qemu}" ]
>> -then
>> -	echo "No Qemu test device support found"
>> -	exit ${TESTDEVNOTSUPP}
>> +	if      [ -z "${QEMUFOUND}" ]
>> +	then
>> +		echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable "
>> +		exit ${NOTFOUND}
>> +	elif    [ -z "${qemu}" ]
>> +	then
>> +		echo "No Qemu test device support found"
> 
> I think we still want to print this if the user supplied qemu has no test
> device support.

Yes, but we do not go through the "for" in that case...

Can you make a patch on top of master?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bandan Das April 9, 2015, 5:20 p.m. UTC | #2

Bandan Das <[e-mail]> writes:
...
>
> I think we still want to print this if the user supplied qemu has no test
> device support. I think this is better -
> --- a/x86/run
> +++ b/x86/run
> @@ -8,6 +8,7 @@ qemusystem="${QEMU:-qemu-system-x86_64}"
>  if      ! [ -z "${QEMU}" ]
>  then
>         qemu="${QEMU}"
> +       QEMUFOUND=1
>  else

Damn, please ignore the previous message. This does nothing and I
need more sleep.

Bandan

>         for qemucmds in ${qemukvm} ${qemusystem}
>         do
>
>> +		exit ${TESTDEVNOTSUPP}
>> +	fi
>>  fi
>>  
>>  if
>>
>> which apart from reindentation is just moving a "fi" down.
>> Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- a/x86/run
+++ b/x86/run
@@ -8,6 +8,7 @@  qemusystem="${QEMU:-qemu-system-x86_64}"
 if      ! [ -z "${QEMU}" ]
 then
        qemu="${QEMU}"
+       QEMUFOUND=1
 else
        for qemucmds in ${qemukvm} ${qemusystem}
        do