Re: [PATCH v3 4/6] iotests: Skip "make check-block" if QEMU does not support virtio-blk

[Max Reitz]

On 11.11.19 15:02, Thomas Huth wrote:
> On 30/10/2019 12.21, Max Reitz wrote:
>> On 22.10.19 09:21, Thomas Huth wrote:
>>> The next patch is going to add some python-based tests to the "auto"
>>> group, and these tests require virtio-blk to work properly. Running
>>> iotests without virtio-blk likely does not make too much sense anyway,
>>> so instead of adding a check for the availability of virtio-blk to each
>>> and every test (which does not sound very appealing), let's rather add
>>> a check for this at the top level in the check-block.sh script instead
>>> (so that it is possible to run "make check" without the "check-block"
>>> part for qemu-system-tricore for example).
>>>
>>> Reviewed-by: Max Reitz <address@hidden>
>>> Signed-off-by: Thomas Huth <address@hidden>
>>> ---
>>>  tests/check-block.sh | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/check-block.sh b/tests/check-block.sh
>>> index 679aedec50..e9e2978818 100755
>>> --- a/tests/check-block.sh
>>> +++ b/tests/check-block.sh
>>> @@ -26,10 +26,24 @@ if grep -q "CFLAGS.*-fsanitize" config-host.mak 
>>> 2>/dev/null ; then
>>>      exit 0
>>>  fi
>>>  
>>> -if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
>>> +if [ -n "$QEMU_PROG" ]; then
>>> +    qemu_prog="$QEMU_PROG"
>>> +else
>>> +    for binary in *-softmmu/qemu-system-* ; do
>>
>> Hm, I know I’ve already given my R-b, but looking at this again – what
>> if the user builds qemu for multiple targets?  Then this will just test
>> any target, whereas the iotests might test something else, because the
>> algorithm there is slightly different:
>>
>> First, check $QEMU_PROG (same as here).
>>
>> Second, check $build_iotests/qemu.  I think we can do this here, because
>> we know that $build_iotests is $PWD/tests/qemu-iotests (or invoking
>> ./check below wouldn’t work).
>>
>> Third, and this is actually important, I think, is that we first look
>> for the qemu that matches the host architecture (uname -m, with an
>> exception for ppc64).  I think we really should do that here, too.
>>
>> Fourth, look for any qemu, as is done here.
>>
>> So I think we could do without #2, but it probably doesn’t hurt to check
>> that, too.  I don’t think we should do without #3, though.
> 
> Maybe we should simply move the check into tests/qemu-iotests/check to
> avoid duplication of that logic?
> We could then also only simply skip the python tests instead of skipping
> everything, in case the chosen QEMU binary does not support virtio-blk.

You mean by adding a new flag e.g. -batch that’s supposed to generally
skip cases when in doubt that they can be run on the current host?
Sounds good to me.

Max

signature.asc
Description: OpenPGP digital signature