On Tue, Apr 24, 2012 at 4:22 PM, Eric Blake <ebl...@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