[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 14/15] qcow2_format.py: dump bitmaps header extension
From: |
Max Reitz |
Subject: |
Re: [PULL 14/15] qcow2_format.py: dump bitmaps header extension |
Date: |
Thu, 18 Jun 2020 17:09:39 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 |
On 18.06.20 15:28, Vladimir Sementsov-Ogievskiy wrote:
> 18.06.2020 16:13, Max Reitz wrote:
>> On 09.06.20 22:52, Eric Blake wrote:
>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>> Add class for bitmap extension and dump its fields. Further work is to
>>> dump bitmap directory.
>>>
>>> Test new functionality inside 291 iotest.
>>
>> Unfortunately, it also breaks 291 with an external data file, which
>> worked before. (The problems being that an external data file gives you
>> an additional header extension, and that the bitmap directory offset
>> changes.)
>>
>> I think if we want to test testing tools, we have to do that in a
>> controlled environment where we exactly know what the image is. It
>> looks to me now as if 291 is not such an environment. Or phrased
>> differently, we probably shouldn’t test some testing tool in normal
>> tests that test qemu itself.
>>
>> If we only test qcow2.py in normal tests, then I don’t think we have to
>> explicitly test it at all. (Because if you test qcow2.py in a normal
>> test, and the test breaks, it’s unclear what’s broken. So I think you
>> might as well forego the qcow2.py test altogether, because if it breaks,
>> that’ll probably show up in some other test case that uses it.)
>>
>> In this case here, I can see three things we could do:
>>
>> First, could just filter out the data file header extension and the
>> bitmap_directory_offset. But I’m not sure whether that’s the best thing
>> to do, because it might break with some other obscure IMGOPTS that I
>> personally never use for the iotests.
>>
>> I think that if we want a real qcow2.py test somewhere, it should be its
>> own test. No custom IMGOPTS allowed. So the second idea would be to
>> move it there, and drop the qcow2.py usage from here.
>>
>> Or, third, maybe we actually don’t care that much about a real qcow2.py
>> test, and really want to just *use* (as opposed to “test”) qcow2.py
>> here. Then we should filter what we really need from its output instead
>> of dropping what we don’t need.
>>
>>
>> (We could also disable 291 for external data files, but I don’t really
>> see why, if the only justification for this addition to it is to test
>> qcow2.py – which 291 isn’t for.)
>>
> I see your point, agree that the most correct thing is to drop qcow2.py
> testing from 291, reverting test chunks from this commit. I can send a
> patch.
OK, thanks!
> I do think, that we'll need some testing for qcow2.py, as we are going
> to improve it a lot, to be used as separate debugging tool. And we just
> need a separate iotest for it.
Great.
Max
signature.asc
Description: OpenPGP digital signature