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

[Vladimir Sementsov-Ogievskiy]

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 <address@hidden>
> ---
>   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


--
Best regards,
Vladimir