[Qemu-devel] [PATCH 3/5] iotests: Compare error messages in 030

Max Reitz posted 5 patches 6 years, 7 months ago
Maintainers: Max Reitz <mreitz@redhat.com>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>
[Qemu-devel] [PATCH 3/5] iotests: Compare error messages in 030
Posted by Max Reitz 6 years, 7 months ago
Currently, 030 just compares the error class, which does not say
anything.

Before HEAD^ added throttling to test_overlapping_4, that test actually
usually failed because node2 was already gone, not because it was the
commit and stream job were not allowed to overlap.

Prevent such problems in the future by comparing the error description
instead.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/030 | 66 +++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 2cf8d54dc5..10fe1de89d 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -144,11 +144,12 @@ class TestSingleDrive(iotests.QMPTestCase):
 
     def test_device_not_found(self):
         result = self.vm.qmp('block-stream', device='nonexistent')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            'Cannot find device=nonexistent nor node_name=nonexistent')
 
     def test_job_id_missing(self):
         result = self.vm.qmp('block-stream', device='mid')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc', "Invalid job ID ''")
 
 
 class TestParallelOps(iotests.QMPTestCase):
@@ -245,24 +246,30 @@ class TestParallelOps(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-stream', device='node5', job_id='stream-node5', base=self.imgs[2])
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node4' is busy: block device is in use by block job: stream")
 
         result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3', base=self.imgs[2])
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node3' is busy: block device is in use by block job: stream")
 
         result = self.vm.qmp('block-stream', device='node4', job_id='stream-node4-v2')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node4' is busy: block device is in use by block job: stream")
 
         # block-commit should also fail if it touches nodes used by the stream job
         result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[4], job_id='commit-node4')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node4' is busy: block device is in use by block job: stream")
 
         result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[1], top=self.imgs[3], job_id='commit-node1')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node3' is busy: block device is in use by block job: stream")
 
         # This fails because it needs to modify the backing string in node2, which is blocked
         result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[0], top=self.imgs[1], job_id='commit-node0')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node2' is busy: block device is in use by block job: stream")
 
         result = self.vm.qmp('block-job-set-speed', device='stream-node4', speed=0)
         self.assert_qmp(result, 'return', {})
@@ -281,20 +288,25 @@ class TestParallelOps(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node3' is busy: block device is in use by block job: commit")
 
         result = self.vm.qmp('block-stream', device='node6', base=self.imgs[2], job_id='stream-node6')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node5' is busy: block device is in use by block job: commit")
 
         result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], job_id='stream-node4')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node4' is busy: block device is in use by block job: commit")
 
         result = self.vm.qmp('block-stream', device='node6', base=self.imgs[4], job_id='stream-node6-v2')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node5' is busy: block device is in use by block job: commit")
 
         # This fails because block-commit currently blocks the active layer even if it's not used
         result = self.vm.qmp('block-stream', device='drive0', base=self.imgs[5], job_id='stream-drive0')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'drive0' is busy: block device is in use by block job: commit")
 
         result = self.vm.qmp('block-job-set-speed', device='commit-node3', speed=0)
         self.assert_qmp(result, 'return', {})
@@ -312,7 +324,8 @@ class TestParallelOps(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-stream', device='node5', base=self.imgs[3], job_id='stream-node6')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node5' is busy: block device is in use by block job: commit")
 
         event = self.vm.event_wait(name='BLOCK_JOB_READY')
         self.assert_qmp(event, 'data/device', 'commit-drive0')
@@ -328,20 +341,21 @@ class TestParallelOps(iotests.QMPTestCase):
         self.wait_until_completed(drive='commit-drive0')
 
     # In this case the base node of the stream job is the same as the
-    # top node of commit job. Since block-commit removes the top node
-    # when it finishes, this is not allowed.
+    # top node of commit job. Since this results in the commit filter
+    # node being part of the stream chain, this is not allowed.
     def test_overlapping_4(self):
         self.assert_no_active_block_jobs()
 
         # Commit from node2 into node0
         result = self.vm.qmp('block-commit', device='drive0',
                              top=self.imgs[2], base=self.imgs[0],
-                             speed=1024*1024)
+                             filter_node_name='commit-filter', speed=1024*1024)
         self.assert_qmp(result, 'return', {})
 
         # Stream from node2 into node4
         result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='node4')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Cannot freeze 'backing' link to 'commit-filter'")
 
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
         self.assert_qmp(result, 'return', {})
@@ -428,19 +442,23 @@ class TestParallelOps(iotests.QMPTestCase):
 
         # Error: the base node does not exist
         result = self.vm.qmp('block-stream', device='node4', base_node='none', job_id='stream')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            'Cannot find device= nor node_name=none')
 
         # Error: the base node is not a backing file of the top node
         result = self.vm.qmp('block-stream', device='node4', base_node='node6', job_id='stream')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node6' is not a backing image of 'node4'")
 
         # Error: the base node is the same as the top node
         result = self.vm.qmp('block-stream', device='node4', base_node='node4', job_id='stream')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Node 'node4' is not a backing image of 'node4'")
 
         # Error: cannot specify 'base' and 'base-node' at the same time
         result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], base_node='node2', job_id='stream')
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "'base' and 'base-node' cannot be specified at the same time")
 
         # Success: the base node is a backing file of the top node
         result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='stream')
@@ -873,7 +891,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('block-stream', device='drive0', speed=-1)
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
 
         self.assert_no_active_block_jobs()
 
@@ -882,7 +900,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
-        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
 
         self.cancel_and_wait(resume=True)
 
-- 
2.21.0


Re: [Qemu-devel] [PATCH 3/5] iotests: Compare error messages in 030
Posted by Alberto Garcia 6 years, 7 months ago
On Fri 28 Jun 2019 12:32:53 AM CEST, Max Reitz wrote:
> Currently, 030 just compares the error class, which does not say
> anything.
>
> Before HEAD^ added throttling to test_overlapping_4, that test actually
> usually failed because node2 was already gone, not because it was the
> commit and stream job were not allowed to overlap.
>
> Prevent such problems in the future by comparing the error description
> instead.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH 3/5] iotests: Compare error messages in 030
Posted by Andrey Shinkevich 6 years, 7 months ago

On 28/06/2019 01:32, Max Reitz wrote:
> Currently, 030 just compares the error class, which does not say
> anything.
> 
> Before HEAD^ added throttling to test_overlapping_4, that test actually
> usually failed because node2 was already gone, not because it was the
> commit and stream job were not allowed to overlap.
> 
> Prevent such problems in the future by comparing the error description
> instead.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/030 | 66 +++++++++++++++++++++++++++---------------
>   1 file changed, 42 insertions(+), 24 deletions(-)
> 
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 2cf8d54dc5..10fe1de89d 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -144,11 +144,12 @@ class TestSingleDrive(iotests.QMPTestCase):
>   
>       def test_device_not_found(self):
>           result = self.vm.qmp('block-stream', device='nonexistent')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            'Cannot find device=nonexistent nor node_name=nonexistent')
>   
>       def test_job_id_missing(self):
>           result = self.vm.qmp('block-stream', device='mid')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc', "Invalid job ID ''")
>   
>   
>   class TestParallelOps(iotests.QMPTestCase):
> @@ -245,24 +246,30 @@ class TestParallelOps(iotests.QMPTestCase):
>           self.assert_qmp(result, 'return', {})
>   
>           result = self.vm.qmp('block-stream', device='node5', job_id='stream-node5', base=self.imgs[2])
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node4' is busy: block device is in use by block job: stream")
>   
>           result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3', base=self.imgs[2])
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node3' is busy: block device is in use by block job: stream")
>   
>           result = self.vm.qmp('block-stream', device='node4', job_id='stream-node4-v2')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node4' is busy: block device is in use by block job: stream")
>   
>           # block-commit should also fail if it touches nodes used by the stream job
>           result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[4], job_id='commit-node4')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node4' is busy: block device is in use by block job: stream")
>   
>           result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[1], top=self.imgs[3], job_id='commit-node1')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node3' is busy: block device is in use by block job: stream")
>   
>           # This fails because it needs to modify the backing string in node2, which is blocked
>           result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[0], top=self.imgs[1], job_id='commit-node0')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node2' is busy: block device is in use by block job: stream")
>   
>           result = self.vm.qmp('block-job-set-speed', device='stream-node4', speed=0)
>           self.assert_qmp(result, 'return', {})
> @@ -281,20 +288,25 @@ class TestParallelOps(iotests.QMPTestCase):
>           self.assert_qmp(result, 'return', {})
>   
>           result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node3' is busy: block device is in use by block job: commit")
>   
>           result = self.vm.qmp('block-stream', device='node6', base=self.imgs[2], job_id='stream-node6')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node5' is busy: block device is in use by block job: commit")
>   
>           result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], job_id='stream-node4')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node4' is busy: block device is in use by block job: commit")
>   
>           result = self.vm.qmp('block-stream', device='node6', base=self.imgs[4], job_id='stream-node6-v2')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node5' is busy: block device is in use by block job: commit")
>   
>           # This fails because block-commit currently blocks the active layer even if it's not used
>           result = self.vm.qmp('block-stream', device='drive0', base=self.imgs[5], job_id='stream-drive0')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'drive0' is busy: block device is in use by block job: commit")
>   
>           result = self.vm.qmp('block-job-set-speed', device='commit-node3', speed=0)
>           self.assert_qmp(result, 'return', {})
> @@ -312,7 +324,8 @@ class TestParallelOps(iotests.QMPTestCase):
>           self.assert_qmp(result, 'return', {})
>   
>           result = self.vm.qmp('block-stream', device='node5', base=self.imgs[3], job_id='stream-node6')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node5' is busy: block device is in use by block job: commit")
>   
>           event = self.vm.event_wait(name='BLOCK_JOB_READY')
>           self.assert_qmp(event, 'data/device', 'commit-drive0')
> @@ -328,20 +341,21 @@ class TestParallelOps(iotests.QMPTestCase):
>           self.wait_until_completed(drive='commit-drive0')
>   
>       # In this case the base node of the stream job is the same as the
> -    # top node of commit job. Since block-commit removes the top node
> -    # when it finishes, this is not allowed.
> +    # top node of commit job. Since this results in the commit filter
> +    # node being part of the stream chain, this is not allowed.
>       def test_overlapping_4(self):
>           self.assert_no_active_block_jobs()
>   
>           # Commit from node2 into node0
>           result = self.vm.qmp('block-commit', device='drive0',
>                                top=self.imgs[2], base=self.imgs[0],
> -                             speed=1024*1024)
> +                             filter_node_name='commit-filter', speed=1024*1024)
>           self.assert_qmp(result, 'return', {})
>   
>           # Stream from node2 into node4
>           result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='node4')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Cannot freeze 'backing' link to 'commit-filter'")
>   
>           result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
>           self.assert_qmp(result, 'return', {})
> @@ -428,19 +442,23 @@ class TestParallelOps(iotests.QMPTestCase):
>   
>           # Error: the base node does not exist
>           result = self.vm.qmp('block-stream', device='node4', base_node='none', job_id='stream')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            'Cannot find device= nor node_name=none')
>   
>           # Error: the base node is not a backing file of the top node
>           result = self.vm.qmp('block-stream', device='node4', base_node='node6', job_id='stream')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node6' is not a backing image of 'node4'")
>   
>           # Error: the base node is the same as the top node
>           result = self.vm.qmp('block-stream', device='node4', base_node='node4', job_id='stream')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "Node 'node4' is not a backing image of 'node4'")
>   
>           # Error: cannot specify 'base' and 'base-node' at the same time
>           result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], base_node='node2', job_id='stream')
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc',
> +            "'base' and 'base-node' cannot be specified at the same time")
>   
>           # Success: the base node is a backing file of the top node
>           result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='stream')
> @@ -873,7 +891,7 @@ class TestSetSpeed(iotests.QMPTestCase):
>           self.assert_no_active_block_jobs()
>   
>           result = self.vm.qmp('block-stream', device='drive0', speed=-1)
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
>   
>           self.assert_no_active_block_jobs()
>   
> @@ -882,7 +900,7 @@ class TestSetSpeed(iotests.QMPTestCase):
>           self.assert_qmp(result, 'return', {})
>   
>           result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
> -        self.assert_qmp(result, 'error/class', 'GenericError')
> +        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
>   
>           self.cancel_and_wait(resume=True)
>   
> 

Tested-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-- 
With the best regards,
Andrey Shinkevich