Re: [Qemu-stable] [PATCH v2 2/2] iotests: simple mirror test with kvm on 1G image

[Vladimir Sementsov-Ogievskiy]

30.11.2018 17:59, Max Reitz wrote:
> On 30.11.18 15:51, Vladimir Sementsov-Ogievskiy wrote:
>> 30.11.2018 17:10, Kevin Wolf wrote:
>>> Am 30.11.2018 um 14:48 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> 30.11.2018 16:13, Max Reitz wrote:
>>>>> On 30.11.18 14:06, Vladimir Sementsov-Ogievskiy wrote:
>>>>>> 30.11.2018 15:30, Max Reitz wrote:
>>>>>>> On 29.11.18 11:18, Vladimir Sementsov-Ogievskiy wrote:
>>>>>>>> This test is broken without previous commit fixing dead-lock in mirror.
>>>>>>>>
>>>>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy<address@hidden>
>>>>>>>> ---
>>>>>>>>      tests/qemu-iotests/235     | 59 
>>>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>>>      tests/qemu-iotests/235.out |  1 +
>>>>>>>>      tests/qemu-iotests/group   |  1 +
>>>>>>>>      3 files changed, 61 insertions(+)
>>>>>>>>      create mode 100755 tests/qemu-iotests/235
>>>>>>>>      create mode 100644 tests/qemu-iotests/235.out
>>>>>>> I'll get to the first patch in a second, but first a suggestion for this
>>>>>>> patch: I think it's not so good to use 2 GB of space for a test (1 GB
>>>>>>> for the source, 1 GB for the target).  So I tried my luck and found that
>>>>>>> the test works, too, if you just use preallocation=metadata for the
>>>>>>> source (instead of actually writing data) and blockdev-mirror'ing the
>>>>>>> data to a throttled null-co device.
>>>>>>
>>>>>> Hmm, so parsing metadata is enough for qcow2 to yield on write, yes?
>>>>>
>>>>> Apparently so.  If you can confirm that applying those changes to the
>>>>> test still make it work (i.e., fail before patch 1, pass afterwards),
>>>>> then I think it is just as good.
>>>>
>>>> Ok, I've checked that your changes works for me.
>>>>
>>>> hm, but we write to null, so, yield on write comes from throttling, 
>>>> however,
>>>> without preallocation=metadata, it don't work.., do you know, why we need
>>>> preallocation to reproduce?
>>>
>>> If I should take a guess, probably because mirror only copies allocated
>>> clusters?
>>>
>>> Kevin
>>>
>>
>> hm, so, what preallocation=metadata does? I thought, it preallocates tables, 
>> but all qcow2 clusters would be unallocated..
> 
> No, it initializes all metadata structures (i.e., L2 tables) so they
> point to data clusters; which is why you cannot use it with backing
> files.  It's just that the data clusters are not written.

so, clusters are allocated? in this case, we don't avoid allocating of 1G, but 
only have a performance gain?

> 
> It might make sense to use preallocated zero clusters in qcow2 v3; but
> then writing to the image would cause COWs again, so there wouldn't be
> too much benefit over just not preallocating.
> 
> Max
> 


-- 
Best regards,
Vladimir