[3/3] iotests: Backup with different source/target size
diff mbox series

Message ID 20200429111539.42103-4-kwolf@redhat.com
State New
Headers show
Series
  • backup: Make sure that source and target size match
Related show

Commit Message

Kevin Wolf April 29, 2020, 11:15 a.m. UTC
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(-)

Comments

Vladimir Sementsov-Ogievskiy April 29, 2020, 12:28 p.m. UTC | #1
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
>
Kevin Wolf April 30, 2020, 11:41 a.m. UTC | #2
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
Vladimir Sementsov-Ogievskiy April 30, 2020, 12:16 p.m. UTC | #3
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
>

Patch
diff mbox series

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