All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: John Snow <jsnow@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [PATCH 4/5] iotests: Skip "make check-block" if QEMU does not support virtio-blk
Date: Mon, 14 Oct 2019 16:17:47 +0200	[thread overview]
Message-ID: <20191014141747.GG7173@localhost.localdomain> (raw)
In-Reply-To: <d82aa777-e193-a484-980a-d0152989e920@redhat.com>

Am 14.10.2019 um 13:27 hat Thomas Huth geschrieben:
> On 14/10/2019 13.21, Kevin Wolf wrote:
> > Am 11.10.2019 um 16:50 hat Thomas Huth geschrieben:
> >> 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).
> >>
> >> 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..7582347ec2 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
> >> +        if [ -x "$binary" ]; then
> >> +            qemu_prog="$binary"
> >> +            break
> >> +        fi
> > 
> > Wouldn't it be better to check the availability of virtio-blk here, so
> > that if the current binary doesn't support it, we keep searching and
> > maybe pick up a different binary that supports it?
> 
> That's a good idea, indeed, but then I also need to adjust the code in
> the "check" script accordingly.

I see. If this just copies the logic that qemu-iotests already uses, I
think I would be okay with taking it as is for now.

> > Or actually, should we work with a whitelist?
> 
> I don't think that a hard-coded list will work well: Since we introduced
> the Kconfig build system, it's now possible for example to also build an
> qemu-system-aarch64 binary that does not contain any of the boards that
> support virtio. So while virtio-blk is available by default in
> qemu-system-aarch64, some builds might not contain it.

Hm, good point. I'm just worried that the default config will end up
running the tests on a machine type where it works, but if you use the
wrong set of configure options, you end up with a setup where 'make
check' fails because it uses a machine type that the iotests don't
support.

Kevin


  reply	other threads:[~2019-10-14 14:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 14:50 [PATCH 0/5] Enable more iotests during "make check-block" Thomas Huth
2019-10-11 14:50 ` [PATCH 1/5] iotests: remove 'linux' from default supported platforms Thomas Huth
2019-10-18 16:15   ` Max Reitz
2019-10-11 14:50 ` [PATCH 2/5] iotests: Test 041 does not work on macOS Thomas Huth
2019-10-18 16:19   ` Max Reitz
2019-10-21  8:08     ` Thomas Huth
2019-10-11 14:50 ` [PATCH 3/5] iotests: Test 183 does not work on macOS and OpenBSD Thomas Huth
2019-10-11 14:50 ` [PATCH 4/5] iotests: Skip "make check-block" if QEMU does not support virtio-blk Thomas Huth
2019-10-14 11:21   ` Kevin Wolf
2019-10-14 11:27     ` Thomas Huth
2019-10-14 14:17       ` Kevin Wolf [this message]
2019-10-18 17:08   ` Max Reitz
2019-10-11 14:50 ` [PATCH 5/5] iotests: Enable more tests in the 'auto' group to improve test coverage Thomas Huth
2019-10-18 17:13   ` Max Reitz

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=20191014141747.GG7173@localhost.localdomain \
    --to=kwolf@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=mreitz@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.