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

[Kevin Wolf <kwolf@redhat.com>]

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. 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?

> >   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