| Message ID | 20200429111539.42103-4-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | backup: Make sure that source and target size match | expand |
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 +-- > 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. > + > + 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? > + > + def test_small_target(self): > + short_len = image_len // 2 > + result = self.vm.qmp('block_resize', device='drive1', size=short_len) > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0', > + target='drive1', sync='full') > + self.assert_qmp(result, 'error/class', 'GenericError') > + > + def test_large_target(self): > + short_len = image_len * 2 > + result = self.vm.qmp('block_resize', device='drive1', size=short_len) > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0', > + target='drive1', sync='full') > + self.assert_qmp(result, 'error/class', 'GenericError') these are identical too, still they are smaller and hardly needs refactoring > + > def test_medium_not_found(self): > if iotests.qemu_default_machine != 'pc': > return > diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out > index 5ce2f9a2ed..88bf7fa73a 100644 > --- a/tests/qemu-iotests/055.out > +++ b/tests/qemu-iotests/055.out > @@ -1,5 +1,5 @@ > -.............................. > +.................................. > ---------------------------------------------------------------------- > -Ran 30 tests > +Ran 34 tests > > OK >
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
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 >
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() + + 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) + + def test_small_target(self): + short_len = image_len // 2 + result = self.vm.qmp('block_resize', device='drive1', size=short_len) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0', + target='drive1', sync='full') + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_large_target(self): + short_len = image_len * 2 + result = self.vm.qmp('block_resize', device='drive1', size=short_len) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0', + target='drive1', sync='full') + self.assert_qmp(result, 'error/class', 'GenericError') + def test_medium_not_found(self): if iotests.qemu_default_machine != 'pc': return diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out index 5ce2f9a2ed..88bf7fa73a 100644 --- a/tests/qemu-iotests/055.out +++ b/tests/qemu-iotests/055.out @@ -1,5 +1,5 @@ -.............................. +.................................. ---------------------------------------------------------------------- -Ran 30 tests +Ran 34 tests OK
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 +-- 2 files changed, 60 insertions(+), 4 deletions(-)