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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2820 bytes --]

On 30.11.18 16:24, Vladimir Sementsov-Ogievskiy wrote:
> 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<vsementsov@virtuozzo.com>
>>>>>>>>> ---
>>>>>>>>>      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?

The data isn't written, so less than 1 MB are actually allocated on-disk.

The L2 tables do point to offsets over a range of 1 GB, but they should
all be holes on the filesystem.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]