| Message ID | 1335275640-3349-6-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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?
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
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
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(-)