* [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them @ 2019-08-19 7:53 Thomas Huth 2019-08-19 8:22 ` no-reply 2019-08-20 15:01 ` Max Reitz 0 siblings, 2 replies; 7+ messages in thread From: Thomas Huth @ 2019-08-19 7:53 UTC (permalink / raw) To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Max Reitz It is possible to enable only a subset of the block drivers with the "--block-drv-rw-whitelist" option of the "configure" script. All other drivers are marked as unusable (or only included as read-only with the "--block-drv-ro-whitelist" option). If an iotest is now using such a disabled block driver, it is failing - which is bad, since at least the tests in the "auto" group should be able to deal with this situation. Thus let's introduce a "_require_drivers" function that can be used by the shell tests to check for the availability of certain drivers first, and marks the test as "not run" if one of the drivers is missing. Signed-off-by: Thomas Huth <thuth@redhat.com> --- tests/qemu-iotests/071 | 1 + tests/qemu-iotests/081 | 1 + tests/qemu-iotests/099 | 1 + tests/qemu-iotests/184 | 1 + tests/qemu-iotests/common.rc | 13 +++++++++++++ 5 files changed, 17 insertions(+) diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071 index 1cca9233d0..fab526666b 100755 --- a/tests/qemu-iotests/071 +++ b/tests/qemu-iotests/071 @@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto file +_require_drivers blkdebug blkverify do_run_qemu() { diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 index c418bab093..1695781bc0 100755 --- a/tests/qemu-iotests/081 +++ b/tests/qemu-iotests/081 @@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt raw _supported_proto file _supported_os Linux +_require_drivers quorum do_run_qemu() { diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099 index ae02f27afe..c3cf66798a 100755 --- a/tests/qemu-iotests/099 +++ b/tests/qemu-iotests/099 @@ -42,6 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 qed vdi vhdx vmdk vpc _supported_proto file _supported_os Linux +_require_drivers blkdebug blkverify _unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \ "subformat=twoGbMaxExtentSparse" diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184 index cb0c181228..33dd8d2a4f 100755 --- a/tests/qemu-iotests/184 +++ b/tests/qemu-iotests/184 @@ -33,6 +33,7 @@ trap "exit \$status" 0 1 2 3 15 . ./common.filter _supported_os Linux +_require_drivers throttle do_run_qemu() { diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 5502c3da2f..7d4e68846f 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -520,5 +520,18 @@ _require_command() [ -x "$c" ] || _notrun "$1 utility required, skipped this test" } +# Check that a set of drivers has been whitelisted in the QEMU binary +# +_require_drivers() +{ + available=$($QEMU -drive format=help | grep 'Supported formats:') + for driver + do + if ! echo "$available" | grep -q "$driver"; then + _notrun "$driver not available" + fi + done +} + # make sure this script returns success true -- 2.18.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them 2019-08-19 7:53 [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them Thomas Huth @ 2019-08-19 8:22 ` no-reply 2019-08-20 15:01 ` Max Reitz 1 sibling, 0 replies; 7+ messages in thread From: no-reply @ 2019-08-19 8:22 UTC (permalink / raw) To: thuth; +Cc: kwolf, qemu-devel, qemu-block, mreitz Patchew URL: https://patchew.org/QEMU/20190819075348.4078-1-thuth@redhat.com/ Hi, This series failed build test on s390x host. Please find the details below. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo echo "=== ENV ===" env echo echo "=== PACKAGES ===" rpm -qa echo echo "=== UNAME ===" uname -a CC=$HOME/bin/cc INSTALL=$PWD/install BUILD=$PWD/build mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --cc=$CC --prefix=$INSTALL make -j4 # XXX: we need reliable clean up # make check -j4 V=1 make install === TEST SCRIPT END === CC mips-softmmu/trace/control-target.o CC mips-softmmu/trace/generated-helpers.o LINK mips-softmmu/qemu-system-mips collect2: error: ld returned 1 exit status make[1]: *** [Makefile:209: qemu-system-mipsel] Error 1 make: *** [Makefile:472: mipsel-softmmu/all] Error 2 make: *** Waiting for unfinished jobs.... --- CC hppa-softmmu/hw/block/vhost-user-blk.o CC microblaze-softmmu/balloon.o CC hppa-softmmu/hw/block/dataplane/virtio-blk.o collect2: error: ld returned 1 exit status make[1]: *** [Makefile:209: qemu-system-mips] Error 1 make: *** [Makefile:472: mips-softmmu/all] Error 2 CC hppa-softmmu/hw/char/virtio-serial-bus.o --- CC microblaze-softmmu/trace/generated-helpers.o LINK microblaze-softmmu/qemu-system-microblaze LINK hppa-softmmu/qemu-system-hppa collect2: error: ld returned 1 exit status make[1]: *** [Makefile:209: qemu-system-microblaze] Error 1 make: *** [Makefile:472: microblaze-softmmu/all] Error 2 collect2: error: ld returned 1 exit status make[1]: *** [Makefile:209: qemu-system-hppa] Error 1 make: *** [Makefile:472: hppa-softmmu/all] Error 2 === OUTPUT END === The full log is available at http://patchew.org/logs/20190819075348.4078-1-thuth@redhat.com/testing.s390x/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them 2019-08-19 7:53 [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them Thomas Huth 2019-08-19 8:22 ` no-reply @ 2019-08-20 15:01 ` Max Reitz 2019-08-20 16:01 ` Thomas Huth 1 sibling, 1 reply; 7+ messages in thread From: Max Reitz @ 2019-08-20 15:01 UTC (permalink / raw) To: Thomas Huth, qemu-block; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1.1: Type: text/plain, Size: 4141 bytes --] On 19.08.19 09:53, Thomas Huth wrote: > It is possible to enable only a subset of the block drivers with the > "--block-drv-rw-whitelist" option of the "configure" script. All other > drivers are marked as unusable (or only included as read-only with the > "--block-drv-ro-whitelist" option). If an iotest is now using such a > disabled block driver, it is failing - which is bad, since at least the > tests in the "auto" group should be able to deal with this situation. > Thus let's introduce a "_require_drivers" function that can be used by > the shell tests to check for the availability of certain drivers first, > and marks the test as "not run" if one of the drivers is missing. Well, the reasoning for generally not making blkdebug/blkverify explicit requirements was that you should just have both enabled when running iotests. Of course, that no longer works as an argument now that we unconditionally run some iotests in make check. But still, the question is how strict you want to be. If blkdebug cannot be assumed to be present, what about null-co? What about raw? > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/qemu-iotests/071 | 1 + > tests/qemu-iotests/081 | 1 + > tests/qemu-iotests/099 | 1 + > tests/qemu-iotests/184 | 1 + > tests/qemu-iotests/common.rc | 13 +++++++++++++ > 5 files changed, 17 insertions(+) > > diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071 > index 1cca9233d0..fab526666b 100755 > --- a/tests/qemu-iotests/071 > +++ b/tests/qemu-iotests/071 > @@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > > _supported_fmt qcow2 > _supported_proto file > +_require_drivers blkdebug blkverify Because this test also requires the raw driver. > > do_run_qemu() > { > diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 > index c418bab093..1695781bc0 100755 > --- a/tests/qemu-iotests/081 > +++ b/tests/qemu-iotests/081 > @@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt raw > _supported_proto file > _supported_os Linux > +_require_drivers quorum This test has already a check whether quorum is supported, that should be removed now. (Also, this test requires the raw driver.) > do_run_qemu() > { [...] > diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184 > index cb0c181228..33dd8d2a4f 100755 > --- a/tests/qemu-iotests/184 > +++ b/tests/qemu-iotests/184 > @@ -33,6 +33,7 @@ trap "exit \$status" 0 1 2 3 15 > . ./common.filter > > _supported_os Linux > +_require_drivers throttle This test also requires null-co. > do_run_qemu() > { I found two more check-block tests that may or may not require use of _require_drivers (depending on which drivers we deem absolutely essential): - 120: Needs raw - 186: Needs null-co > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc > index 5502c3da2f..7d4e68846f 100644 > --- a/tests/qemu-iotests/common.rc > +++ b/tests/qemu-iotests/common.rc > @@ -520,5 +520,18 @@ _require_command() > [ -x "$c" ] || _notrun "$1 utility required, skipped this test" > } > > +# Check that a set of drivers has been whitelisted in the QEMU binary > +# > +_require_drivers() > +{ > + available=$($QEMU -drive format=help | grep 'Supported formats:') Seems a bit shortcut-y to not remove the “Supported formats:” prefix, but I don’t suppose we’ll ever have block drivers with either name... > + for driver > + do > + if ! echo "$available" | grep -q "$driver"; then 162 greps like this: > grep '^Supported formats:.* ssh\( \|$\)' Maybe the same should be done here, i.e. grep -q " $driver\( \|\$\)"? I can well imagine that something like “ssh” might appear as a substring in some other driver. (Speaking of which, why not change 162 to using this new function? Yes, it isn’t in auto, but still...) Max > + _notrun "$driver not available" > + fi > + done > +} > + > # make sure this script returns success > true > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them 2019-08-20 15:01 ` Max Reitz @ 2019-08-20 16:01 ` Thomas Huth 2019-08-20 18:48 ` Max Reitz 0 siblings, 1 reply; 7+ messages in thread From: Thomas Huth @ 2019-08-20 16:01 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1.1: Type: text/plain, Size: 5662 bytes --] On 8/20/19 5:01 PM, Max Reitz wrote: > On 19.08.19 09:53, Thomas Huth wrote: >> It is possible to enable only a subset of the block drivers with the >> "--block-drv-rw-whitelist" option of the "configure" script. All other >> drivers are marked as unusable (or only included as read-only with the >> "--block-drv-ro-whitelist" option). If an iotest is now using such a >> disabled block driver, it is failing - which is bad, since at least the >> tests in the "auto" group should be able to deal with this situation. >> Thus let's introduce a "_require_drivers" function that can be used by >> the shell tests to check for the availability of certain drivers first, >> and marks the test as "not run" if one of the drivers is missing. > > Well, the reasoning for generally not making blkdebug/blkverify explicit > requirements was that you should just have both enabled when running > iotests. Well, we disable blkverify in our downstream RHEL version of QEMU - so it would be great if the iotests could at least adapt to that missing driver. > Of course, that no longer works as an argument now that we > unconditionally run some iotests in make check. > > But still, the question is how strict you want to be. If blkdebug > cannot be assumed to be present, what about null-co? What about raw? I tried to disable everything beside qcow2 - but that causes so many things to fail that it hardly makes sense to try to get that working. I think we can assume that at least null-co, qcow2 and raw are enabled. (If anybody still wants to try to run "make check" with one of these drivers disabled, I think we should rather add a superior check to tests/check-block.sh or tests/qemu-iotests/check instead and skip the iotests completely in that case). >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> tests/qemu-iotests/071 | 1 + >> tests/qemu-iotests/081 | 1 + >> tests/qemu-iotests/099 | 1 + >> tests/qemu-iotests/184 | 1 + >> tests/qemu-iotests/common.rc | 13 +++++++++++++ >> 5 files changed, 17 insertions(+) >> >> diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071 >> index 1cca9233d0..fab526666b 100755 >> --- a/tests/qemu-iotests/071 >> +++ b/tests/qemu-iotests/071 >> @@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 >> >> _supported_fmt qcow2 >> _supported_proto file >> +_require_drivers blkdebug blkverify > > Because this test also requires the raw driver. The test also works for me when I configured QEMU with: --block-drv-rw-whitelist="qcow2 file null-co blkdebug blkverify" i.e. the raw driver should be disabled in that case? >> >> do_run_qemu() >> { >> diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 >> index c418bab093..1695781bc0 100755 >> --- a/tests/qemu-iotests/081 >> +++ b/tests/qemu-iotests/081 >> @@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 >> _supported_fmt raw >> _supported_proto file >> _supported_os Linux >> +_require_drivers quorum > > This test has already a check whether quorum is supported, that should > be removed now. Hmm, true ... apparently that test was not working for my case ... could it be that qemu-img ignores the whitelist and simply says that all drivers are supported? > (Also, this test requires the raw driver.) Agreed, this test indeed does not work without 'raw'. But it is already marked with "_supported_fmt raw", so you can indeed only run it with "raw". And running a raw-only test with a qemu binary where raw is disabled could be considered as user error, I guess ;-) >> diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184 >> index cb0c181228..33dd8d2a4f 100755 >> --- a/tests/qemu-iotests/184 >> +++ b/tests/qemu-iotests/184 >> @@ -33,6 +33,7 @@ trap "exit \$status" 0 1 2 3 15 >> . ./common.filter >> >> _supported_os Linux >> +_require_drivers throttle > > This test also requires null-co. > >> do_run_qemu() >> { > > I found two more check-block tests that may or may not require use of > _require_drivers (depending on which drivers we deem absolutely essential): > - 120: Needs raw > - 186: Needs null-co I think we really have to assume that null-co is available, otherwise too many things break... (also some qtests are using null-co). But I could for sure add a check for raw in 120 if desired. >> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc >> index 5502c3da2f..7d4e68846f 100644 >> --- a/tests/qemu-iotests/common.rc >> +++ b/tests/qemu-iotests/common.rc >> @@ -520,5 +520,18 @@ _require_command() >> [ -x "$c" ] || _notrun "$1 utility required, skipped this test" >> } >> >> +# Check that a set of drivers has been whitelisted in the QEMU binary >> +# >> +_require_drivers() >> +{ >> + available=$($QEMU -drive format=help | grep 'Supported formats:') > Seems a bit shortcut-y to not remove the “Supported formats:” prefix, > but I don’t suppose we’ll ever have block drivers with either name... I can remove it, just to be sure. >> + for driver >> + do >> + if ! echo "$available" | grep -q "$driver"; then > > 162 greps like this: > >> grep '^Supported formats:.* ssh\( \|$\)' > > Maybe the same should be done here, i.e. grep -q " $driver\( \|\$\)"? I > can well imagine that something like “ssh” might appear as a substring > in some other driver. Fair, I'll add the spaces. > (Speaking of which, why not change 162 to using this new function? Yes, > it isn’t in auto, but still...) Yes, I can add that. Thomas [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them 2019-08-20 16:01 ` Thomas Huth @ 2019-08-20 18:48 ` Max Reitz 2019-08-20 19:19 ` Thomas Huth 0 siblings, 1 reply; 7+ messages in thread From: Max Reitz @ 2019-08-20 18:48 UTC (permalink / raw) To: Thomas Huth, qemu-block; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1.1: Type: text/plain, Size: 6318 bytes --] On 20.08.19 18:01, Thomas Huth wrote: > On 8/20/19 5:01 PM, Max Reitz wrote: >> On 19.08.19 09:53, Thomas Huth wrote: >>> It is possible to enable only a subset of the block drivers with the >>> "--block-drv-rw-whitelist" option of the "configure" script. All other >>> drivers are marked as unusable (or only included as read-only with the >>> "--block-drv-ro-whitelist" option). If an iotest is now using such a >>> disabled block driver, it is failing - which is bad, since at least the >>> tests in the "auto" group should be able to deal with this situation. >>> Thus let's introduce a "_require_drivers" function that can be used by >>> the shell tests to check for the availability of certain drivers first, >>> and marks the test as "not run" if one of the drivers is missing. >> >> Well, the reasoning for generally not making blkdebug/blkverify explicit >> requirements was that you should just have both enabled when running >> iotests. > > Well, we disable blkverify in our downstream RHEL version of QEMU - so > it would be great if the iotests could at least adapt to that missing > driver. I would like to say that RHEL is not a gold standard, but then I have this series of selfish patches that specifically addresses RHEL whitelisting issues. It feels a bit weird to me to say “blkverify is not essential, because RHEL disables it, but null-co is” – even though there is no reason why anyone would need null-co except for testing either. >> Of course, that no longer works as an argument now that we >> unconditionally run some iotests in make check. >> >> But still, the question is how strict you want to be. If blkdebug >> cannot be assumed to be present, what about null-co? What about raw? > > I tried to disable everything beside qcow2 - but that causes so many > things to fail that it hardly makes sense to try to get that working. Hm, really? I just whitelisted qcow2 and file and running the auto group worked rather well (except for the failing tests you address here, and the two others I mentioned). > I think we can assume that at least null-co, qcow2 and raw are enabled. > (If anybody still wants to try to run "make check" with one of these > drivers disabled, I think we should rather add a superior check to > tests/check-block.sh or tests/qemu-iotests/check instead and skip the > iotests completely in that case). I’m OK either way: We can add a global check, or we just make a decision on what users definitely have to have enabled or the qemu build would be a bit boring. Assuming file, qcow2, and raw to be enabled seems reasonable. I’m not so sure about null-co. (I mean, I personally don’t really care. I never added such checks myself, even a bit purposefully so, because it was my opinion that you should probably have all block drivers whitelisted before running the iotests. But then came CI and make check-block integration...) ((Also, you’ll notice that I was really inconsequential about adding null-co checks in my “selfish patches” series, precisely because I assumed everyone would have whitelisted null-co. So I’m indeed OK with just making it an implicit prerequisite.)) >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> tests/qemu-iotests/071 | 1 + >>> tests/qemu-iotests/081 | 1 + >>> tests/qemu-iotests/099 | 1 + >>> tests/qemu-iotests/184 | 1 + >>> tests/qemu-iotests/common.rc | 13 +++++++++++++ >>> 5 files changed, 17 insertions(+) >>> >>> diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071 >>> index 1cca9233d0..fab526666b 100755 >>> --- a/tests/qemu-iotests/071 >>> +++ b/tests/qemu-iotests/071 >>> @@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 >>> >>> _supported_fmt qcow2 >>> _supported_proto file >>> +_require_drivers blkdebug blkverify >> >> Because this test also requires the raw driver. > > The test also works for me when I configured QEMU with: > > --block-drv-rw-whitelist="qcow2 file null-co blkdebug blkverify" > > i.e. the raw driver should be disabled in that case? Ah, it’s just used by qemu-io, which of course ignores whitelisting. >>> >>> do_run_qemu() >>> { >>> diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 >>> index c418bab093..1695781bc0 100755 >>> --- a/tests/qemu-iotests/081 >>> +++ b/tests/qemu-iotests/081 >>> @@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 >>> _supported_fmt raw >>> _supported_proto file >>> _supported_os Linux >>> +_require_drivers quorum >> >> This test has already a check whether quorum is supported, that should >> be removed now. > > Hmm, true ... apparently that test was not working for my case ... could > it be that qemu-img ignores the whitelist and simply says that all > drivers are supported? Ah, yeah. The whitelist is relevant only for the system emulator. I forgot why we even had the existing check, then. Maybe just a mistake to use qemu-img. >> (Also, this test requires the raw driver.) > > Agreed, this test indeed does not work without 'raw'. But it is already > marked with "_supported_fmt raw", so you can indeed only run it with > "raw". And running a raw-only test with a qemu binary where raw is > disabled could be considered as user error, I guess ;-) Oops, didn’t even notice somehow. O:-) >>> diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184 >>> index cb0c181228..33dd8d2a4f 100755 >>> --- a/tests/qemu-iotests/184 >>> +++ b/tests/qemu-iotests/184 >>> @@ -33,6 +33,7 @@ trap "exit \$status" 0 1 2 3 15 >>> . ./common.filter >>> >>> _supported_os Linux >>> +_require_drivers throttle >> >> This test also requires null-co. >> >>> do_run_qemu() >>> { >> >> I found two more check-block tests that may or may not require use of >> _require_drivers (depending on which drivers we deem absolutely essential): >> - 120: Needs raw >> - 186: Needs null-co > > I think we really have to assume that null-co is available, otherwise > too many things break... (also some qtests are using null-co). > > But I could for sure add a check for raw in 120 if desired. If we assume that null-co is there, raw is definitely going to be there. Max [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them 2019-08-20 18:48 ` Max Reitz @ 2019-08-20 19:19 ` Thomas Huth 2019-08-20 19:23 ` Max Reitz 0 siblings, 1 reply; 7+ messages in thread From: Thomas Huth @ 2019-08-20 19:19 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1.1: Type: text/plain, Size: 1666 bytes --] On 8/20/19 8:48 PM, Max Reitz wrote: > On 20.08.19 18:01, Thomas Huth wrote: [...] >> Well, we disable blkverify in our downstream RHEL version of QEMU - so >> it would be great if the iotests could at least adapt to that missing >> driver. > > I would like to say that RHEL is not a gold standard Well, let's put it this way: The less changes we have to carry along downstream (and thus review each time we rebase the downstream tree), the more time we have to work on upstream. > It feels a bit weird to me to say “blkverify is not essential, because > RHEL disables it, but null-co is” – even though there is no reason why > anyone would need null-co except for testing either. Ok, fine for me, too, if we also declare "null-co" as optional for the iotests - let's make sure that the tests in the "auto" group also work without them. >>> Of course, that no longer works as an argument now that we >>> unconditionally run some iotests in make check. >>> >>> But still, the question is how strict you want to be. If blkdebug >>> cannot be assumed to be present, what about null-co? What about raw? >> >> I tried to disable everything beside qcow2 - but that causes so many >> things to fail that it hardly makes sense to try to get that working. > > Hm, really? I just whitelisted qcow2 and file and running the auto > group worked rather well (except for the failing tests you address here, > and the two others I mentioned). IIRC I tried to run all qcow2 tests when I disabled null-co and saw lots of failures ... but anyway, let's just focus on the "auto" tests right now, that should be doable. Thomas [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them 2019-08-20 19:19 ` Thomas Huth @ 2019-08-20 19:23 ` Max Reitz 0 siblings, 0 replies; 7+ messages in thread From: Max Reitz @ 2019-08-20 19:23 UTC (permalink / raw) To: Thomas Huth, qemu-block; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1.1: Type: text/plain, Size: 2011 bytes --] On 20.08.19 21:19, Thomas Huth wrote: > On 8/20/19 8:48 PM, Max Reitz wrote: >> On 20.08.19 18:01, Thomas Huth wrote: > [...] >>> Well, we disable blkverify in our downstream RHEL version of QEMU - so >>> it would be great if the iotests could at least adapt to that missing >>> driver. >> >> I would like to say that RHEL is not a gold standard > > Well, let's put it this way: The less changes we have to carry along > downstream (and thus review each time we rebase the downstream tree), > the more time we have to work on upstream. As I said, I’m guilty myself. >> It feels a bit weird to me to say “blkverify is not essential, because >> RHEL disables it, but null-co is” – even though there is no reason why >> anyone would need null-co except for testing either. > > Ok, fine for me, too, if we also declare "null-co" as optional for the > iotests - let's make sure that the tests in the "auto" group also work > without them. Well, should we or not? You said there are other tests (outside of the iotests) that break without null-co. If so, I don’t think there’s any point in making it optional here. >>>> Of course, that no longer works as an argument now that we >>>> unconditionally run some iotests in make check. >>>> >>>> But still, the question is how strict you want to be. If blkdebug >>>> cannot be assumed to be present, what about null-co? What about raw? >>> >>> I tried to disable everything beside qcow2 - but that causes so many >>> things to fail that it hardly makes sense to try to get that working. >> >> Hm, really? I just whitelisted qcow2 and file and running the auto >> group worked rather well (except for the failing tests you address here, >> and the two others I mentioned). > > IIRC I tried to run all qcow2 tests when I disabled null-co and saw lots > of failures ... but anyway, let's just focus on the "auto" tests right > now, that should be doable. OK, I didn’t bother running all. :-) Max [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-20 19:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-19 7:53 [Qemu-devel] [PATCH] iotests: Check for enabled drivers before testing them Thomas Huth 2019-08-19 8:22 ` no-reply 2019-08-20 15:01 ` Max Reitz 2019-08-20 16:01 ` Thomas Huth 2019-08-20 18:48 ` Max Reitz 2019-08-20 19:19 ` Thomas Huth 2019-08-20 19:23 ` Max Reitz
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).