On 09/12/2017 09:44 AM, Paolo Bonzini wrote: > Instead of ./check failing when a binary is missing, we try each test > case now and each one fails with tons of test case diffs. Also, all the > variables were initialized by "check" prior to "common" being sourced, > and then (uselessly) checked for emptiness again in "check". > > Centralize the search for programs in "common" (which will soon be > one with "check"), including the "realpath" invocation which can be done > just once in "check" rather than in the tests. > > For qnio_server, move the detection to "common", simplifying > set_prog_path to stop handling the unused second argument, and > embedding the "realpath" pass. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > tests/qemu-iotests/check | 41 --------------------- > tests/qemu-iotests/common | 77 > +++++++++++++++++++++++++++++++++++++--- > tests/qemu-iotests/common.config | 61 +------------------------------ > 3 files changed, 73 insertions(+), 106 deletions(-) >
> +++ b/tests/qemu-iotests/common
> @@ -37,6 +37,17 @@ _full_platform_details()
> echo "$os/$platform $host $kernel"
> }
>
> +# $1 = prog to look for
> +set_prog_path()
> +{
> + p=`command -v $1 2> /dev/null`
Are we sure that $1 will always respond to -v sanely?
> + if [ -n "$p" -a -x "$p" ]; then
[ ... -a ... ] should never be used; there are cases where it is
inherently ambiguous (even if it happens to work here, because we know
our shell is bash and because $p is not likely to contain text that
throws off the parser). Please spell that either:
if [ -n "$p" ] && [ -x "$p" ]; then
if [[ -n $p && -x $p ]]; then
(that is, I don't mind if you rely on bash's [[]] in which case you can
use less typing, but if you stick to [], then you should use code that
can be pasted to other shells)
> +then
> + if [ -x "$build_iotests/qemu" ]; then
> + export QEMU_PROG="$build_iotests/qemu"
> + elif [ -x "$build_root/$arch-softmmu/qemu-system-$arch" ]; then
> + export QEMU_PROG="$build_root/$arch-softmmu/qemu-system-$arch"
> + else
> + pushd "$build_root" > /dev/null
Shouldn't you check for failure to change directories?
> +++ b/tests/qemu-iotests/common.config
> @@ -22,6 +22,7 @@ export LANG=C
> PATH=".:$PATH"
>
> HOSTOS=`uname -s`
> +arch=`uname -m`
As long as we're touching this, should we prefer $() instead of ``?
>
> export PWD=`pwd`
>
> @@ -30,28 +31,6 @@ export _QEMU_HANDLE=0
> # make sure we have a standard umask
> umask 022
>
> -# $1 = prog to look for, $2* = default pathnames if not found in $PATH
> -set_prog_path()
> -{
> - p=`command -v $1 2> /dev/null`
> - if [ -n "$p" -a -x "$p" ]; then
Urrgh - your use of [ -a ] was pre-existing, and you just moved it.
Still worth fixing, though (either here or in a separate cleanup patch).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
