Re: [Qemu-devel] [PATCH 3/3] qemu-iotests: Test blockdev-backup in 055

[Markus Armbruster]

Fam Zheng <address@hidden> writes:

> This applies cases on drive-backup on blockdev-backup, except cases with
> target format and mode.
>
> Also add a case to check source == target.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  tests/qemu-iotests/055     | 277 
> ++++++++++++++++++++++++++++++++++++++-------
>  tests/qemu-iotests/055.out |   4 +-
>  2 files changed, 236 insertions(+), 45 deletions(-)
>
> diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
> index 451b67d..ab47d59 100755
> --- a/tests/qemu-iotests/055
> +++ b/tests/qemu-iotests/055
> @@ -1,8 +1,8 @@
>  #!/usr/bin/env python
>  #
> -# Tests for drive-backup
> +# Tests for drive-backup and blockdev-backup
>  #
> -# Copyright (C) 2013 Red Hat, Inc.
> +# Copyright (C) 2013, 2014 Red Hat, Inc.
>  #
>  # Based on 041.
>  #
> @@ -27,6 +27,7 @@ from iotests import qemu_img, qemu_io
>  
>  test_img = os.path.join(iotests.test_dir, 'test.img')
>  target_img = os.path.join(iotests.test_dir, 'target.img')
> +blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img')
>  
>  class TestSingleDrive(iotests.QMPTestCase):
>      image_len = 64 * 1024 * 1024 # MB
> @@ -38,34 +39,48 @@ class TestSingleDrive(iotests.QMPTestCase):
>          qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
>          qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
>          qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
> +        qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
> str(TestSingleDrive.image_len))
>  
> -        self.vm = iotests.VM().add_drive(test_img)
> +        self.vm = 
> iotests.VM().add_drive(test_img).add_drive(blockdev_target_img)
>          self.vm.launch()
>  
>      def tearDown(self):
>          self.vm.shutdown()
>          os.remove(test_img)
> +        os.remove(blockdev_target_img)
>          try:
>              os.remove(target_img)
>          except OSError:
>              pass
>  
> -    def test_cancel(self):
> +    def do_test_cancel(self, test_drive_backup):
>          self.assert_no_active_block_jobs()
>  
> -        result = self.vm.qmp('drive-backup', device='drive0',
> -                             target=target_img, sync='full')
> +        if test_drive_backup:
> +            result = self.vm.qmp('drive-backup', device='drive0',
> +                                 target=target_img, sync='full')
> +        else:
> +            result = self.vm.qmp('blockdev-backup', device='drive0',
> +                                 target='drive1', sync='full')
>          self.assert_qmp(result, 'return', {})
>  
>          event = self.cancel_and_wait()
>          self.assert_qmp(event, 'data/type', 'backup')
>  
> -    def test_pause(self):
> +    def test_cancel(self):
> +        self.do_test_cancel(True)
> +        self.do_test_cancel(False)
> +

The bool parameter feels inelegant.  Perhaps you'd prefer something like
this (untested):

    def do_test_cancel(self, cmd, **args):
        self.assert_no_active_block_jobs()

        result = self.vm.qmp(cmd, **args)
        self.assert_qmp(result, 'return', {})

        event = self.cancel_and_wait()
        self.assert_qmp(event, 'data/type', 'backup')


    def test_cancel(self):
        self.do_test_cancel('drive-backup', device='drive0',
                            target=target_img, sync='full'))
        self.do_test_cancel('blockdev-backup', device='drive0',
                            target='drive1', sync='full')

Entirely up to you.  More of the same below.

> +    def do_test_pause(self, test_drive_backup):
>          self.assert_no_active_block_jobs()
>  
>          self.vm.pause_drive('drive0')
> -        result = self.vm.qmp('drive-backup', device='drive0',
> -                             target=target_img, sync='full')
> +        if test_drive_backup:
> +            result = self.vm.qmp('drive-backup', device='drive0',
> +                                 target=target_img, sync='full')
> +        else:
> +            result = self.vm.qmp('blockdev-backup', device='drive0',
> +                                 target='drive1', sync='full')
>          self.assert_qmp(result, 'return', {})
>  
>          result = self.vm.qmp('block-job-pause', device='drive0')
> @@ -86,14 +101,28 @@ class TestSingleDrive(iotests.QMPTestCase):
>          self.wait_until_completed()
>  
>          self.vm.shutdown()
> -        self.assertTrue(iotests.compare_images(test_img, target_img),
> -                        'target image does not match source after backup')
> +        if test_drive_backup:
> +            self.assertTrue(iotests.compare_images(test_img, target_img),
> +                            'target image does not match source after 
> backup')
> +        else:
> +            self.assertTrue(iotests.compare_images(test_img, 
> blockdev_target_img),
> +                            'target image does not match source after 
> backup')
> +
> +    def test_pause_drive_backup(self):
> +        self.do_test_pause(True)
> +
> +    def test_pause_blockdev_backup(self):
> +        self.do_test_pause(False)
>  
>      def test_medium_not_found(self):
>          result = self.vm.qmp('drive-backup', device='ide1-cd0',
>                               target=target_img, sync='full')
>          self.assert_qmp(result, 'error/class', 'GenericError')
>  
> +        result = self.vm.qmp('blockdev-backup', device='ide1-cd0',
> +                             target='drive1', sync='full')
> +        self.assert_qmp(result, 'error/class', 'GenericError')
> +
>      def test_image_not_found(self):
>          result = self.vm.qmp('drive-backup', device='drive0',
>                               target=target_img, sync='full', mode='existing')
> @@ -110,26 +139,56 @@ class TestSingleDrive(iotests.QMPTestCase):
>                               target=target_img, sync='full')
>          self.assert_qmp(result, 'error/class', 'DeviceNotFound')
>  
> +        result = self.vm.qmp('blockdev-backup', device='nonexistent',
> +                             target='drive0', sync='full')
> +        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
> +
> +        result = self.vm.qmp('blockdev-backup', device='drive0',
> +                             target='nonexistent', sync='full')
> +        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
> +
> +        result = self.vm.qmp('blockdev-backup', device='nonexistent',
> +                             target='nonexistent', sync='full')
> +        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
> +
> +    def test_target_is_source(self):
> +        result = self.vm.qmp('blockdev-backup', device='drive0',
> +                             target='drive0', sync='full')
> +        self.assert_qmp(result, 'error/class', 'GenericError')
> +
>  class TestSetSpeed(iotests.QMPTestCase):
>      image_len = 80 * 1024 * 1024 # MB
>  
>      def setUp(self):
>          qemu_img('create', '-f', iotests.imgfmt, test_img, 
> str(TestSetSpeed.image_len))
>          qemu_io('-c', 'write -P1 0 512', test_img)
> -        self.vm = iotests.VM().add_drive(test_img)
> +        qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
> str(TestSingleDrive.image_len))
> +
> +        self.vm = 
> iotests.VM().add_drive(test_img).add_drive(blockdev_target_img)
>          self.vm.launch()
>  
>      def tearDown(self):
>          self.vm.shutdown()
>          os.remove(test_img)
> -        os.remove(target_img)
> +        try:
> +            os.remove(blockdev_target_img)
> +        except OSError:
> +            pass

Elsewhere, you remove blockdev_target_img without a try.  Why not here?

> +        try:
> +            os.remove(target_img)
> +        except OSError:
> +            pass

Why do you need to wrap removal of target_img in a try now?

>  
> -    def test_set_speed(self):
> +    def do_test_set_speed(self, test_drive_backup):
>          self.assert_no_active_block_jobs()
>  
>          self.vm.pause_drive('drive0')
> -        result = self.vm.qmp('drive-backup', device='drive0',
> -                             target=target_img, sync='full')
> +        if test_drive_backup:
> +            result = self.vm.qmp('drive-backup', device='drive0',
> +                                 target=target_img, sync='full')
> +        else:
> +            result = self.vm.qmp('blockdev-backup', device='drive0',
> +                                 target='drive1', sync='full')
>          self.assert_qmp(result, 'return', {})
>  
>          # Default speed is 0
[...]