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

[Vladimir Sementsov-Ogievskiy]

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?

> 
> (Also, the test as-is passes even without patch 1 on tmpfs, at least for
> me.  But with the configuration described above, it fails there, too.)
> 
> I've attached a diff.
> 
> Max
> 

[...]

>
> 
> 235.diff
> 
> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
> index e5d1ae6593..c5264972b9 100755
> --- a/tests/qemu-iotests/235
> +++ b/tests/qemu-iotests/235
> @@ -21,7 +21,7 @@
>   import sys
>   import os
>   import iotests
> -from iotests import qemu_img_create, qemu_io, file_path
> +from iotests import qemu_img_create, qemu_io, file_path, log
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 
> 'scripts'))
>   
> @@ -38,22 +38,34 @@ from qemu import QEMUMachine
>   #    for me)
>   # 4. add iothread
>   
> -size = '1G'
> +size = 1 * 1024 * 1024 * 1024
>   
>   iotests.verify_image_format(supported_fmts=['raw', 'qcow2'])
>   
> -disk, target = file_path('disk', 'target')
> +disk = file_path('disk')
>   
>   # prepare source image
> -qemu_img_create('-f', iotests.imgfmt, disk, size)
> -qemu_io('-f', iotests.imgfmt, '-c', 'write 0 ' + size, disk)
> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk,
> +                str(size))
>   
>   vm = QEMUMachine(iotests.qemu_prog)
>   vm.add_args('-machine', 'pc,accel=kvm')
>   vm.add_args('-drive', 'id=src,file=' + disk)
>   vm.launch()
>   
> -print(vm.qmp('drive-mirror', device='src', target=target, sync='full'))
> +log(vm.qmp('object-add', qom_type='throttle-group', id='tg0',
> +           props={ 'x-bps-total': 64 * 1048576 }))

and you add throttling. interesting, isn't enough to mirror empty qcow2(no 
preallocation)
or even zero driver (without a qcow2 at all) to null with throttling? Or, 
otherwise, why we need throttling here?

> +
> +log(vm.qmp('blockdev-add',
> +           **{ 'node-name': 'target',
> +               'driver': 'throttle',
> +               'throttle-group': 'tg0',
> +               'file': {
> +                   'driver': 'null-co',
> +                   'size': size
> +                } }))
> +
> +log(vm.qmp('blockdev-mirror', device='src', target='target', sync='full'))
>   vm.event_wait('BLOCK_JOB_READY')
>   
>   vm.shutdown()
> diff --git a/tests/qemu-iotests/235.out b/tests/qemu-iotests/235.out
> index d6ccbad8fa..39db621e04 100644
> --- a/tests/qemu-iotests/235.out
> +++ b/tests/qemu-iotests/235.out
> @@ -1 +1,3 @@
> -{u'return': {}}
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> 


anyway, it's good thing to avoid 2G allocation in test, I agree.

-- 
Best regards,
Vladimir