Patchwork [v2,5/6] qemu-iotests: add block-stream with invalid speed value test

login
register
mail settings
Submitter Stefan Hajnoczi
Date April 24, 2012, 1:53 p.m.
Message ID <1335275640-3349-6-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/154687/
State New
Headers show

Comments

Stefan Hajnoczi - April 24, 2012, 1:53 p.m.
Add a test to exercise the BlockJobSpeedInvalid error code path in the
block-stream command.  The 'speed' parameter allows the speed limit of
the job to be applied immediately when the job starts instead of issuing
a separate block-job-set-speed command later.  If the parameter has an
invalid value we expect to get an error and the job is not created.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 tests/qemu-iotests/030     |   29 +++++++++++++++++++++++++++++
 tests/qemu-iotests/030.out |    4 ++--
 2 files changed, 31 insertions(+), 2 deletions(-)
Eric Blake - April 24, 2012, 3:22 p.m.
On 04/24/2012 07:53 AM, Stefan Hajnoczi wrote:
> Add a test to exercise the BlockJobSpeedInvalid error code path in the
> block-stream command.  The 'speed' parameter allows the speed limit of
> the job to be applied immediately when the job starts instead of issuing
> a separate block-job-set-speed command later.  If the parameter has an
> invalid value we expect to get an error and the job is not created.
> 

> +
> +        result = self.vm.qmp('block-stream', device='drive0')
> +        self.assert_qmp(result, 'return', {})
> +
> +        result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
> +        self.assert_qmp(result, 'error/class', 'InvalidParameter')
> +        self.assert_qmp(result, 'error/data/name', 'speed')

Here, you're at the mercy of the race that we just plugged by adding the
optional speed parameter.  What happens if your testsuite is so delayed
that the block job finishes before you get to this point?

> +
> +        result = self.vm.qmp('block-job-cancel', device='drive0')
> +        self.assert_qmp(result, 'return', {})

Same race - what happens if your testsuite is so delayed that the block
job finishes before you get to this point?

Are we sure that the testsuite is setting up a long-enough-running block
job that this will never bite us in practice?

You're missing a test: where do you test that block-stream with valid
speed argument, followed by a query-block-job, reads back the speed that
was initially set?
Stefan Hajnoczi - April 24, 2012, 3:50 p.m.
On Tue, Apr 24, 2012 at 4:22 PM, Eric Blake <eblake@redhat.com> wrote:
> On 04/24/2012 07:53 AM, Stefan Hajnoczi wrote:
>> Add a test to exercise the BlockJobSpeedInvalid error code path in the
>> block-stream command.  The 'speed' parameter allows the speed limit of
>> the job to be applied immediately when the job starts instead of issuing
>> a separate block-job-set-speed command later.  If the parameter has an
>> invalid value we expect to get an error and the job is not created.
>>
>
>> +
>> +        result = self.vm.qmp('block-stream', device='drive0')
>> +        self.assert_qmp(result, 'return', {})
>> +
>> +        result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
>> +        self.assert_qmp(result, 'error/class', 'InvalidParameter')
>> +        self.assert_qmp(result, 'error/data/name', 'speed')
>
> Here, you're at the mercy of the race that we just plugged by adding the
> optional speed parameter.  What happens if your testsuite is so delayed
> that the block job finishes before you get to this point?
>
>> +
>> +        result = self.vm.qmp('block-job-cancel', device='drive0')
>> +        self.assert_qmp(result, 'return', {})
>
> Same race - what happens if your testsuite is so delayed that the block
> job finishes before you get to this point?
>
> Are we sure that the testsuite is setting up a long-enough-running block
> job that this will never bite us in practice?

Yes, we can be confident about the race here.  The image size is 80 MB
and we follow up with block-job-set-speed immediately so it will take
a bunch of QEMU main loop iterations (and hence monitor processing) to
complete this job.

> You're missing a test: where do you test that block-stream with valid
> speed argument, followed by a query-block-job, reads back the speed that
> was initially set?

Yes, it's missing.  If I need to resend I can add it.

Stefan

Patch

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 978fd82..fdd7b3e 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -147,5 +147,34 @@  class TestSetSpeed(ImageStreamingTestCase):
 
         self.assert_no_active_streams()
 
+    def test_set_speed_invalid(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0', speed=-1)
+        self.assert_qmp(result, 'error/class', 'InvalidParameter')
+        self.assert_qmp(result, 'error/data/name', 'speed')
+
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
+        self.assert_qmp(result, 'error/class', 'InvalidParameter')
+        self.assert_qmp(result, 'error/data/name', 'speed')
+
+        result = self.vm.qmp('block-job-cancel', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        cancelled = False
+        while not cancelled:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_CANCELLED':
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    cancelled = True
+
+        self.assert_no_active_streams()
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index 8d7e996..89968f3 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -1,5 +1,5 @@ 
-...
+....
 ----------------------------------------------------------------------
-Ran 3 tests
+Ran 4 tests
 
 OK