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

[Andrey Shinkevich]

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 <address@hidden>
> ---
>   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 <address@hidden>
-- 
With the best regards,
Andrey Shinkevich