Re: [Qemu-block] [PATCH v1 07/15] iotests: fix 097 when run with qcow

[Max Reitz]

On 17.01.2017 10:59, Daniel P. Berrange wrote:
> On Mon, Jan 16, 2017 at 09:04:31PM +0100, Max Reitz wrote:
>> On 03.01.2017 19:27, Daniel P. Berrange wrote:
>>> The previous commit:
>>>
>>>   commit a3e1505daec31ef56f0489f8c8fff1b8e4ca92bd
>>>   Author: Eric Blake <address@hidden>
>>>   Date:   Mon Dec 5 09:49:34 2016 -0600
>>>
>>>     qcow2: Don't strand clusters near 2G intervals during commit
>>>
>>> extended the 097 test case so that it did two passes, once
>>> with an internal snapshot, once without.
>>>
>>> qcow (v1) does not support internal snapshots, so this change
>>> broke test 097 when run against qcow.
>>>
>>> This splits 097 in two, creating a new 173 that tests the
>>> internal snapshot codepath, effectively putting 097 back
>>> to its content before the above commit.
>>>
>>> Signed-off-by: Daniel P. Berrange <address@hidden>
>>> ---
>>>  tests/qemu-iotests/097     |  10 +---
>>>  tests/qemu-iotests/097.out | 125 
>>> ++------------------------------------------
>>>  tests/qemu-iotests/173     | 126 
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>  tests/qemu-iotests/173.out | 119 ++++++++++++++++++++++++++++++++++++++++++
>>>  tests/qemu-iotests/group   |   1 +
>>>  5 files changed, 251 insertions(+), 130 deletions(-)
>>>  create mode 100755 tests/qemu-iotests/173
>>>  create mode 100644 tests/qemu-iotests/173.out
>>
>> I don't think the effort is worth it, considering that probably
>> literally nobody is still using qcow -- or so I hope, at least.
> 
> The reason I fixed this was because I wanted to be able to verify my
> refactoring to qcow didn't break anything else. It is much easier if
> I can just run "check -qcow" and not have to worry about which failures
> are just test bugs, vs genuine code bugs I've created. IOW, as long as
> qcow.c exists in the code base, IMHO, we should make sure the iotests
> continue to pass

Right, what I meant is that I don't think it's worth fixing tests for
qcow. If they don't work and it isn't qcow that's broken (but just the
test requiring something that qcow does not have), just removing it from
the _supported_fmt list is usually sufficient in my opinion.

> On that point, IMHO, it would be beneficial if we had some CI system
> that was setup to run the iotests on all new changes, across all the
> different disk formats we expect the iotests to work on. We get far
> too many regressions with iotests breaking and no one noticing right
> now :-(

Indeed, but that would require all of the iotests consistently passing
first...

Max

signature.asc
Description: OpenPGP digital signature