qemu-block
[Top][All Lists]
Advanced

[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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]