qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Thomas Huth <thuth@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH v3 4/6] iotests: Skip "make check-block" if QEMU does not support virtio-blk
Date: Mon, 11 Nov 2019 17:10:43 +0100	[thread overview]
Message-ID: <6b002caa-1e53-b12f-3b67-833b87bacd51@redhat.com> (raw)
In-Reply-To: <fcd271a6-2d90-5087-237d-f308b2367c04@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 2777 bytes --]

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 <mreitz@redhat.com>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2019-11-11 16:25 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-22  7:21 [PATCH v3 0/6] Enable more iotests during "make check-block" Thomas Huth
2019-10-22  7:21 ` [PATCH v3 1/6] iotests: remove 'linux' from default supported platforms Thomas Huth
2019-10-22  7:21 ` [PATCH v3 2/6] iotests: Test 041 only works on certain systems Thomas Huth
2019-10-22  7:21 ` [PATCH v3 3/6] iotests: Test 183 does not work on macOS and OpenBSD Thomas Huth
2019-10-22  7:21 ` [PATCH v3 4/6] iotests: Skip "make check-block" if QEMU does not support virtio-blk Thomas Huth
2019-10-30 11:21   ` Max Reitz
2019-11-11 14:02     ` Thomas Huth
2019-11-11 16:10       ` Max Reitz [this message]
2019-10-22  7:21 ` [PATCH v3 5/6] iotests: Enable more tests in the 'auto' group to improve test coverage Thomas Huth
2019-10-24 11:14   ` Alex Bennée
2019-11-27 14:11     ` Thomas Huth
2019-10-22  7:21 ` [PATCH v3 6/6] iotests: Remove 130 from the "auto" group Thomas Huth
2019-10-22  7:58 ` [PATCH v3 0/6] Enable more iotests during "make check-block" Thomas Huth
2019-10-22 11:39   ` Alex Bennée
2019-10-22 11:46     ` Thomas Huth
2019-10-22 13:09       ` Alex Bennée
2019-10-22 13:11 ` Alex Bennée
2019-10-22 13:39   ` Max Reitz
2019-10-22 13:48     ` Alex Bennée
2019-10-22 18:54       ` Thomas Huth
2019-10-22 21:16         ` Alex Bennée

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6b002caa-1e53-b12f-3b67-833b87bacd51@redhat.com \
    --to=mreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).