Patchwork [v3,5/5] qemu-iotests: add block-stream speed value test case

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

Comments

Stefan Hajnoczi - April 25, 2012, 1:17 p.m.
Add tests to exercise the InvalidParameter 'speed' error code path, as
well as the regular success case for setting the speed.  The
block-stream '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.

It turns out that cancelling a block job is a common operation in these
test cases, let's extract a cancel_and_wait() function instead of
duplicating the QMP commands.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/030     |   91 ++++++++++++++++++++++++++++++++++----------
 tests/qemu-iotests/030.out |    4 +-
 2 files changed, 72 insertions(+), 23 deletions(-)
Stefan Hajnoczi - April 25, 2012, 3:49 p.m.
On Wed, Apr 25, 2012 at 3:21 PM, Eric Blake <eblake@redhat.com> wrote:
> On 04/25/2012 07:17 AM, Stefan Hajnoczi wrote:
>> Add tests to exercise the InvalidParameter 'speed' error code path, as
>> well as the regular success case for setting the speed.  The
>> block-stream '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.
>>
>> It turns out that cancelling a block job is a common operation in these
>> test cases, let's extract a cancel_and_wait() function instead of
>> duplicating the QMP commands.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>> Acked-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>
>> +
>> +    def test_set_speed_invalid(self):
>>          self.assert_no_active_streams()
>>
>> +        result = self.vm.qmp('block-stream', device='drive0', speed=2 * 1024 * 1024)
>> +        self.assert_qmp(result, {})
>> +
>> +        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')
>> +
>> +        self.cancel_and_wait()
>> +
>> +    def test_set_speed_invalid(self):
>
> Duplicate def test_set_speed_invalid.  Bad copy-and-paste issue?  The
> first one looks bogus, the second one looks correct.

Hmm...not my finest patch series.  I made a mess, it was indeed a copy
paste error.  Interestingly the tests still passed :).

I'm sending a new revision that fixes this.  There are three cases for
set speed:
1. A throughput performance test which is not automatically run
(because it takes several seconds to complete and is really only good
for testing throttling).
2. A success case test that makes sure query-block-jobs reports the
speed that we set.
3. A failure case for InvalidParameter.

Stefan

Patch

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 978fd82..11b56fa 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -32,6 +32,21 @@  class ImageStreamingTestCase(iotests.QMPTestCase):
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return', [])
 
+    def cancel_and_wait(self, drive='drive0'):
+        '''Cancel a block job and wait for it to finish'''
+        result = self.vm.qmp('block-job-cancel', device=drive)
+        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', drive)
+                    cancelled = True
+
+        self.assert_no_active_streams()
+
 class TestSingleDrive(ImageStreamingTestCase):
     image_len = 1 * 1024 * 1024 # MB
 
@@ -97,18 +112,7 @@  class TestStreamStop(ImageStreamingTestCase):
         events = self.vm.get_qmp_events(wait=False)
         self.assertEqual(events, [], 'unexpected QMP event: %s' % events)
 
-        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()
+        self.cancel_and_wait()
 
 # This is a short performance test which is not run by default.
 # Invoke "IMGFMT=qed ./030 TestSetSpeed.perf_test_set_speed"
@@ -132,20 +136,65 @@  class TestSetSpeed(ImageStreamingTestCase):
         result = self.vm.qmp('block-stream', device='drive0')
         self.assert_qmp(result, 'return', {})
 
+        # Default speed is 0
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/device', 'drive0')
+        self.assert_qmp(result, 'return[0]/speed', 0)
+
         result = self.vm.qmp('block-job-set-speed', device='drive0', value=8 * 1024 * 1024)
         self.assert_qmp(result, 'return', {})
 
-        completed = False
-        while not completed:
-            for event in self.vm.get_qmp_events(wait=True):
-                if event['event'] == 'BLOCK_JOB_COMPLETED':
-                    self.assert_qmp(event, 'data/type', 'stream')
-                    self.assert_qmp(event, 'data/device', 'drive0')
-                    self.assert_qmp(event, 'data/offset', self.image_len)
-                    self.assert_qmp(event, 'data/len', self.image_len)
-                    completed = True
+        # Ensure the speed we set was accepted
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/device', 'drive0')
+        self.assert_qmp(result, 'return[0]/speed', 8 * 1024 * 1024)
+
+        self.cancel_and_wait()
+
+        # Check setting speed in block-stream works
+        result = self.vm.qmp('block-stream', device='drive0', speed=4 * 1024 * 1024)
+        self.assert_qmp(result, 'return', {})
 
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/device', 'drive0')
+        self.assert_qmp(result, 'return[0]/speed', 4 * 1024 * 1024)
+
+        self.cancel_and_wait()
+
+    def test_set_speed_invalid(self):
         self.assert_no_active_streams()
 
+        result = self.vm.qmp('block-stream', device='drive0', speed=2 * 1024 * 1024)
+        self.assert_qmp(result, {})
+
+        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')
+
+        self.cancel_and_wait()
+
+    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')
+
+        self.cancel_and_wait()
+
 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