Re: [Qemu-devel] [PATCH v2 1/7] iotests: Allow out-of-tree run

[Max Reitz <mreitz@redhat.com>]

On 22.05.2014 08:45, Fam Zheng wrote:
> On Tue, 05/20 22:23, Max Reitz wrote:
>> As out-of-tree builds are preferred for qemu, running the qemu-iotests
>> in that out-of-tree build should be supported as well. To do so, a
>> symbolic link has to be created pointing to the check script in the
>> source directory. That script will check whether it has been run through
>> a symlink, and if so, will assume it is run in the build tree. All
>> output and temporary operations performed by iotests are then redirected
>> here and, unless specified otherwise by the user, QEMU_PROG etc. will be
>> set to paths appropriate for the build tree.
>>
>> Also, drop making every test case executable if it is not yet, as this
>> would modify the source tree which is not desired for out-of-tree runs
>> and should be fixed in the repository anyway.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/check         | 95 +++++++++++++++++++++++++++++++++-------
>>   tests/qemu-iotests/common        |  8 ++--
>>   tests/qemu-iotests/common.config |  2 +-
>>   tests/qemu-iotests/common.rc     |  8 ++--
>>   tests/qemu-iotests/iotests.py    |  3 +-
>>   5 files changed, 91 insertions(+), 25 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index e2ed5a9..aa30ce5 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -34,22 +34,86 @@ timestamp=${TIMESTAMP:=false}
>>   # generic initialization
>>   iam=check
>>   
>> +_init_error()
>> +{
>> +    echo "$iam: $1" >&2
>> +    exit 1
>> +}
>> +
>> +if [ -L "$0" ]
>> +then
>> +    # called from the build tree
>> +    source_iotests=$(dirname "$(readlink "$0")")
>> +    if [ -z "$source_iotests" ]
>> +    then
>> +        _init_error "failed to obtain source tree name from check symlink"
>> +    fi
>> +    source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree"
>> +    build_iotests=$PWD
>> +    build_root="$build_iotests/../.."
>> +
>> +    if [ -x "$build_iotests/socket_scm_helper" ]
>> +    then
>> +        export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
>> +    fi
>> +else
>> +    # called from the source tree
>> +    source_iotests=$PWD
>> +fi
>> +
>> +if [ -n "$build_root" ]
>> +then
>> +    if [ -z "$QEMU_PROG" ]
>> +    then
> I think it would be a good idea to uniform the two cases: if running from
> source tree, simply let build_root=$source_iotests/../.. So we don't need the
> condition here, and QEMU_*_PROG is automatically set for both cases, instead of
> only for out-of-tree.

Yes, I thought about this, too. I wasn't sure what to do, as this would 
change the current behavior. As these are tests, I guess it would be 
fine to break with current behavior, though.

On the other hand, this will probably not make the code simpler, as then 
we'll have to check whether symbolic links/executables named "qemu", 
"qemu-io" etc. exist in the current directory.

As I was unsure and you seem to prefer adding auto-detection of the 
built executables for in-tree builds, too, I'll send a v3 doing so.

Max

> Fam
>
>> +        arch=$(uname -m 2> /dev/null)
>> +
>> +        if [[ -n $arch && -x "$build_root/$arch-softmmu/qemu-system-$arch" ]]
>> +        then
>> +            export QEMU_PROG="$build_root/$arch-softmmu/qemu-system-$arch"
>> +        else
>> +            pushd -q "$build_root"
>> +            for binary in "*-softmmu/qemu-system-*"
>> +            do
>> +                if [ -x "$binary" ]
>> +                then
>> +                    export QEMU_PROG="$build_root/$binary"
>> +                    break
>> +                fi
>> +            done
>> +            popd -q
>> +        fi
>> +    fi
>> +
>> +    if [[ -z $QEMU_IMG_PROG && -x "$build_root/qemu-img" ]]
>> +    then
>> +        export QEMU_IMG_PROG="$build_root/qemu-img"
>> +    fi
>> +
>> +    if [[ -z $QEMU_IO_PROG && -x "$build_root/qemu-io" ]]
>> +    then
>> +        export QEMU_IO_PROG="$build_root/qemu-io"
>> +    fi
>> +
>> +    if [[ -z $QEMU_NBD_PROG && -x "$build_root/qemu-nbd" ]]
>> +    then
>> +        export QEMU_NBD_PROG="$build_root/qemu-nbd"
>> +    fi
>> +fi
>> +
>>   # we need common.config
>> -if ! . ./common.config
>> +if ! . "$source_iotests/common.config"
>>   then
>> -    echo "$iam: failed to source common.config"
>> -    exit 1
>> +    _init_error "failed to source common.config"
>>   fi
>>   
>>   # we need common.rc
>> -if ! . ./common.rc
>> +if ! . "$source_iotests/common.rc"
>>   then
>> -    echo "check: failed to source common.rc"
>> -    exit 1
>> +    _init_error "failed to source common.rc"
>>   fi
>>   
>>   # we need common
>> -. ./common
>> +. "$source_iotests/common"
>>   
>>   #if [ `id -u` -ne 0 ]
>>   #then
>> @@ -194,7 +258,7 @@ do
>>           echo " - expunged"
>>           rm -f $seq.out.bad
>>           echo "/^$seq\$/d" >>$tmp.expunged
>> -    elif [ ! -f $seq ]
>> +    elif [ ! -f "$source_iotests/$seq" ]
>>       then
>>           echo " - no such test?"
>>           echo "/^$seq\$/d" >>$tmp.expunged
>> @@ -215,9 +279,10 @@ do
>>   
>>           start=`_wallclock`
>>           $timestamp && echo -n "        ["`date "+%T"`"]"
>> -        [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
>> +        export OUTPUT_DIR=$PWD
>> +        (cd "$source_iotests";
>>           MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
>> -                ./$seq >$tmp.out 2>&1
>> +                ./$seq >$tmp.out 2>&1)
>>           sts=$?
>>           $timestamp && _timestamp
>>           stop=`_wallclock`
>> @@ -242,17 +307,17 @@ do
>>                   err=true
>>               fi
>>   
>> -            reference=$seq.out
>> +            reference="$source_iotests/$seq.out"
>>               if [ "$CACHEMODE" = "none" ]; then
>> -                [ -f $seq.out.nocache ] && reference=$seq.out.nocache
>> +                [ -f "$source_iotests/$seq.out.nocache" ] && reference="$source_iotests/$seq.out.nocache"
>>               fi
>>   
>> -            if [ ! -f $reference ]
>> +            if [ ! -f "$reference" ]
>>               then
>>                   echo " - no qualified output"
>>                   err=true
>>               else
>> -                if diff -w $reference $tmp.out >/dev/null 2>&1
>> +                if diff -w "$reference" $tmp.out >/dev/null 2>&1
>>                   then
>>                       echo ""
>>                       if $err
>> @@ -264,7 +329,7 @@ do
>>                   else
>>                       echo " - output mismatch (see $seq.out.bad)"
>>                       mv $tmp.out $seq.out.bad
>> -                    $diff -w $reference $seq.out.bad
>> +                    $diff -w "$reference" $seq.out.bad
>>                       err=true
>>                   fi
>>               fi
>> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
>> index 0aaf84d..3c53c4f 100644
>> --- a/tests/qemu-iotests/common
>> +++ b/tests/qemu-iotests/common
>> @@ -59,7 +59,7 @@ do
>>       if $group
>>       then
>>           # arg after -g
>> -        group_list=`sed -n <group -e 's/$/ /' -e "/^[0-9][0-9][0-9].* $r /"'{
>> +        group_list=`sed -n <"$source_iotests/group" -e 's/$/ /' -e "/^[0-9][0-9][0-9].* $r /"'{
>>   s/ .*//p
>>   }'`
>>           if [ -z "$group_list" ]
>> @@ -84,7 +84,7 @@ s/ .*//p
>>       then
>>           # arg after -x
>>           [ ! -s $tmp.list ] && ls [0-9][0-9][0-9] [0-9][0-9][0-9][0-9] >$tmp.list 2>/dev/null
>> -        group_list=`sed -n <group -e 's/$/ /' -e "/^[0-9][0-9][0-9].* $r /"'{
>> +        group_list=`sed -n <"$source_iotests/group" -e 's/$/ /' -e "/^[0-9][0-9][0-9].* $r /"'{
>>   s/ .*//p
>>   }'`
>>           if [ -z "$group_list" ]
>> @@ -366,7 +366,7 @@ testlist options
>>   BEGIN        { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \
>>           | while read id
>>           do
>> -            if grep -s "^$id " group >/dev/null
>> +            if grep -s "^$id " "$source_iotests/group" >/dev/null
>>               then
>>                   # in group file ... OK
>>                   echo $id >>$tmp.list
>> @@ -402,7 +402,7 @@ else
>>           touch $tmp.list
>>       else
>>           # no test numbers, do everything from group file
>> -        sed -n -e '/^[0-9][0-9][0-9]*/s/[         ].*//p' <group >$tmp.list
>> +        sed -n -e '/^[0-9][0-9][0-9]*/s/[         ].*//p' <"$source_iotests/group" >$tmp.list
>>       fi
>>   fi
>>   
>> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
>> index d90a8bc..bd6790b 100644
>> --- a/tests/qemu-iotests/common.config
>> +++ b/tests/qemu-iotests/common.config
>> @@ -126,7 +126,7 @@ fi
>>   export TEST_DIR
>>   
>>   if [ -z "$SAMPLE_IMG_DIR" ]; then
>> -        SAMPLE_IMG_DIR=`pwd`/sample_images
>> +        SAMPLE_IMG_DIR="$source_iotests/sample_images"
>>   fi
>>   
>>   if [ ! -d "$SAMPLE_IMG_DIR" ]; then
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 195c564..e0ea7e3 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -318,9 +318,9 @@ _do()
>>           status=1; exit
>>       fi
>>   
>> -    (eval "echo '---' \"$_cmd\"") >>$here/$seq.full
>> +    (eval "echo '---' \"$_cmd\"") >>"$OUTPUT_DIR/$seq.full"
>>       (eval "$_cmd") >$tmp._out 2>&1; ret=$?
>> -    cat $tmp._out >>$here/$seq.full
>> +    cat $tmp._out >>"$OUTPUT_DIR/$seq.full"
>>       if [ $# -eq 2 ]; then
>>           if [ $ret -eq 0 ]; then
>>               echo "done"
>> @@ -344,7 +344,7 @@ _do()
>>   #
>>   _notrun()
>>   {
>> -    echo "$*" >$seq.notrun
>> +    echo "$*" >"$OUTPUT_DIR/$seq.notrun"
>>       echo "$seq not run: $*"
>>       status=0
>>       exit
>> @@ -354,7 +354,7 @@ _notrun()
>>   #
>>   _fail()
>>   {
>> -    echo "$*" | tee -a $here/$seq.full
>> +    echo "$*" | tee -a "$OUTPUT_DIR/$seq.full"
>>       echo "(see $seq.full for details)"
>>       status=1
>>       exit 1
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index f6c437c..39a4cfc 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -37,6 +37,7 @@ qemu_args = os.environ.get('QEMU', 'qemu').strip().split(' ')
>>   imgfmt = os.environ.get('IMGFMT', 'raw')
>>   imgproto = os.environ.get('IMGPROTO', 'file')
>>   test_dir = os.environ.get('TEST_DIR', '/var/tmp')
>> +output_dir = os.environ.get('OUTPUT_DIR', '.')
>>   cachemode = os.environ.get('CACHEMODE')
>>   
>>   socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
>> @@ -278,7 +279,7 @@ def notrun(reason):
>>       # Each test in qemu-iotests has a number ("seq")
>>       seq = os.path.basename(sys.argv[0])
>>   
>> -    open('%s.notrun' % seq, 'wb').write(reason + '\n')
>> +    open('%s/%s.notrun' % (output_dir, seq), 'wb').write(reason + '\n')
>>       print '%s not run: %s' % (seq, reason)
>>       sys.exit(0)
>>   
>> -- 
>> 1.9.2
>>