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 <vsement...@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
