Re: [Qemu-devel] [PATCH v2 5/6] qemu-iotests: add block-stream with invalid speed value test

[Stefan Hajnoczi]

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