Re: [PATCH 3/3] iotests: Backup with different source/target size

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

30.04.2020 14:41, Kevin Wolf wrote:
> Am 29.04.2020 um 14:28 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 29.04.2020 14:15, Kevin Wolf wrote:
>>> This tests that the backup jobs catches situations where the target node
>>> has a different size than the source node. It must also forbid resize
>>> operations when the job is already running.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>    tests/qemu-iotests/055     | 60 ++++++++++++++++++++++++++++++++++++--
>>>    tests/qemu-iotests/055.out |  4 +--
> 
> One general remark and question that came up while I was running 055 a
> lot and really got annonyed by the long time it takes:
> 
> TestDriveCompression is quite unconventional in that 055 is raw/qcow2
> only per se, but some of the test cases always test qcow2 and vmdk.

Yes, that's bad. Oh, seems I have a patch for it not still sent. Will do soon.

> The
> slow one is vmdk.
> 
> I found out that zero writes in vmdk are completely broken (I'll send
> patches), but even after fixing this, it's still slow. I think this is
> the combination of VMDK not having writeback metadata caching and
> backup serving lots of tiny 64k zero writes.
> 
> Has anyone ever looked into making backup use more reasonable request
> sizes for the background copy like mirror does?

I am :)

The whole series improving backup is
  "[RFC 00/24] backup performance: block_status + async"
First part is already merged, current chunk close to be finished:
  "[PATCH v4 0/5] block-copy: use aio-task-pool"

> 
>>>    2 files changed, 60 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
>>> index 82b9f5f47d..243d66a62e 100755
>>> --- a/tests/qemu-iotests/055
>>> +++ b/tests/qemu-iotests/055
>>> @@ -48,8 +48,10 @@ class TestSingleDrive(iotests.QMPTestCase):
>>>        def setUp(self):
>>>            qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len))
>>> -        self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
>>> -        self.vm.add_drive(blockdev_target_img, interface="none")
>>> +        self.vm = iotests.VM()
>>> +        self.vm.add_drive('blkdebug::' + test_img, 'node-name=source')
>>> +        self.vm.add_drive(blockdev_target_img, 'node-name=target',
>>> +                          interface="none")
>>>            if iotests.qemu_default_machine == 'pc':
>>>                self.vm.add_drive(None, 'media=cdrom', 'ide')
>>>            self.vm.launch()
>>> @@ -112,6 +114,60 @@ class TestSingleDrive(iotests.QMPTestCase):
>>>        def test_pause_blockdev_backup(self):
>>>            self.do_test_pause('blockdev-backup', 'drive1', blockdev_target_img)
>>> +    def test_source_resize_blockdev_backup(self):
>>> +        self.assert_no_active_block_jobs()
>>
>> this will never fire, as vm is created a moment before, I'd drop it.
> 
> This pattern exists all over the place in 055, but you're right, it's
> kind of pointless.
> 
>>> +
>>> +        def pre_finalize():
>>> +            result = self.vm.qmp('block_resize', device='drive0', size=65536)
>>> +            self.assert_qmp(result, 'error/class', 'GenericError')
>>> +
>>> +            result = self.vm.qmp('block_resize', node_name='source', size=65536)
>>> +            self.assert_qmp(result, 'error/class', 'GenericError')
>>> +
>>> +        result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0',
>>> +                             target='drive1', sync='full', auto_finalize=False,
>>> +                             auto_dismiss=False)
>>> +        self.assert_qmp(result, 'return', {})
>>> +
>>> +        self.vm.run_job('job0', auto_finalize=False, pre_finalize=pre_finalize,
>>> +                        use_log=False)
>>> +
>>> +    def test_target_resize_blockdev_backup(self):
>>> +        self.assert_no_active_block_jobs()
>>> +
>>> +        def pre_finalize():
>>> +            result = self.vm.qmp('block_resize', device='drive1', size=65536)
>>> +            self.assert_qmp(result, 'error/class', 'GenericError')
>>> +
>>> +            result = self.vm.qmp('block_resize', node_name='target', size=65536)
>>> +            self.assert_qmp(result, 'error/class', 'GenericError')
>>> +
>>> +        result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0',
>>> +                             target='drive1', sync='full', auto_finalize=False,
>>> +                             auto_dismiss=False)
>>> +        self.assert_qmp(result, 'return', {})
>>> +
>>> +        self.vm.run_job('job0', auto_finalize=False, pre_finalize=pre_finalize,
>>> +                        use_log=False)
>>
>> these two functions are almost identical.. worth refactoring to be use common helper?
> 
> Ok, I'll see what I can do.
> 
> Kevin
> 


-- 
Best regards,
Vladimir