Re: [Qemu-devel] [PATCH 00/17] iotests: don't choke on disabled drivers

[Max Reitz]

On 2018-05-30 15:47, Roman Kagan wrote:
> On Wed, May 30, 2018 at 02:35:34PM +0200, Max Reitz wrote:
>> On 2018-04-26 18:19, Roman Kagan wrote:
>>> Some iotests assume availability of certain block drivers, and fail if
>>> the driver is not supported by QEMU because it was disabled at configure
>>> time.
>>>
>>> This series tries to address that, by making QEMU report the actual list
>>> of supported block drivers in response to "-drive format=?", and using
>>> this information to skip the parts of the io testsuite that can not be
>>> run in this configuration.
>>>
>>> Roman Kagan (17):
>>>   block: iterate_format with account of whitelisting
>>>   iotests: iotests.py: prevent deadlock in subprocess
>>>   iotests: ask qemu for supported formats
>>>   iotest 030: skip quorum test setup/teardown too
>>>   iotest 030: require blkdebug
>>>   iotest 055: skip unsupported backup target formats
>>>   iotest 055: require blkdebug
>>>   iotest 056: skip testcases using blkdebug if disabled
>>>   iotest 071: notrun if blkdebug or blkverify is disabled
>>>   iotest 081: notrun if quorum is disabled
>>>   iotest 087: notrun if null-co is disabled
>>>   iotest 093: notrun if null-co or null-aio is disabled
>>>   iotest 099: notrun if blkdebug or blkverify is disabled
>>>   iotest 124: skip testcases using blkdebug if disabled
>>>   iotest 139: skip testcases using disabled drivers
>>>   iotest 147: notrun if nbd is disabled
>>>   iotest 184: notrun if null-co or throttle is disabled
>>>
>>>  include/block/block.h         |  2 +-
>>>  block.c                       | 23 ++++++++++++++++++----
>>>  blockdev.c                    |  4 +++-
>>>  qemu-img.c                    |  2 +-
>>>  tests/qemu-iotests/030        |  7 +++++++
>>>  tests/qemu-iotests/055        | 13 ++++++++++++
>>>  tests/qemu-iotests/056        |  3 +++
>>>  tests/qemu-iotests/071        |  1 +
>>>  tests/qemu-iotests/081        |  1 +
>>>  tests/qemu-iotests/087        |  1 +
>>>  tests/qemu-iotests/093        |  1 +
>>>  tests/qemu-iotests/099        |  1 +
>>>  tests/qemu-iotests/124        |  5 +++++
>>>  tests/qemu-iotests/139        |  4 ++++
>>>  tests/qemu-iotests/147        |  1 +
>>>  tests/qemu-iotests/184        |  1 +
>>>  tests/qemu-iotests/common.rc  | 19 ++++++++++++++++++
>>>  tests/qemu-iotests/iotests.py | 46 
>>> ++++++++++++++++++++++++++++++++-----------
>>>  18 files changed, 117 insertions(+), 18 deletions(-)
>>
>> I'll stop reviewing this series for now, because there are more iotests
>> that use drivers outside of their format/protocol combination.
>>
>> For instance:
>>
>> $ grep -l null-co ??? | wc -l
>> 15
>> $ grep -l blkdebug ??? | wc -l
>> 30
>> $ (grep -l '"raw"' ???; grep -l "'raw'" ???) | wc -l
>> 22
>>
>> As I've written in my reply to patch 8, I'm not sure whether it's the
>> right solution to check for the availability of these block drivers in
>> every single test that needs them.  It makes sense for quorum, because
>> quorum needs an external library for hashing, so it may not be trivial
>> to enable.  But it does not seem too useful for other formats that do
>> not have such a dependency (e.g. null-co, blkdebug, raw).
>>
>> The thing is that it's OK to whitelist everything for testing, and then
>> disable some drivers when building a release.  I don't think one needs
>> to run the iotests with the release version if the whole difference is
>> whether some drivers have been disabled or not.
> 
> This patchset was created when we started to run (quick) iotests as a part
> of the package build, which looked like a decent alternative to building
> separate CI infrastructure.
> 
> If there's no general interest in running iotests against QEMU built in
> a release configuration with certain drivers blacklisted, I think we
> should be fine handling this locally.

It's not like the majority of drivers uses blkdebug, but still, it seems
at least suboptimal to run the tests if you then disable a big chunk of
them because some drivers were not whitelisted.

If you're OK with a local solution...  OK, but naively I'd think that
it'd be better to just build two versions for packaging: One that's
actually going to be packaged, and one that's used for testing.  (Though
I don't know how well that fits into your process.)

In any case, I'm not fully opposed to this series, but it would need to
be complete and make every test require every block driver that is not
part of its protocol/format combination.

Max

signature.asc
Description: OpenPGP digital signature