On Tue, Apr 24, 2012 at 8:31 PM, Luiz Capitulino <lcapitul...@redhat.com> wrote:
> On Mon, 23 Apr 2012 16:39:48 +0100
> Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> wrote:
>
>> Allow streaming operations to be started with an initial speed limit.
>> This eliminates the window of time between starting streaming and
>> issuing block-job-set-speed. Users should use the new optional 'speed'
>> parameter instead so that speed limits are in effect immediately when
>> the job starts.
>>
>> Signed-off-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com>
>> ---
>> block.c | 18 ++++++++++++++++--
>> block/stream.c | 5 +++--
>> block_int.h | 9 ++++++---
>> blockdev.c | 6 ++++--
>> hmp-commands.hx | 4 ++--
>> hmp.c | 4 +++-
>> qapi-schema.json | 6 +++++-
>> qmp-commands.hx | 2 +-
>> 8 files changed, 40 insertions(+), 14 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 7056d8c..e3c1483 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -4072,8 +4072,8 @@ out:
>> }
>>
>> void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
>> - BlockDriverCompletionFunc *cb, void *opaque,
>> - Error **errp)
>> + int64_t speed, BlockDriverCompletionFunc *cb,
>> + void *opaque, Error **errp)
>> {
>> BlockJob *job;
>>
>> @@ -4089,6 +4089,20 @@ void *block_job_create(const BlockJobType *job_type,
>> BlockDriverState *bs,
>> job->cb = cb;
>> job->opaque = opaque;
>> bs->job = job;
>> +
>> + /* Only set speed when necessary to avoid NotSupported error */
>> + if (speed != 0) {
>
> Missed this small detail. Ideally, you should test against has_speed, but
> I think that there are only two possibilities for a false negativehere:
> 1. the client/user expects speed=0 to "work" 2. 'speed' is (probably
> incorrectly) initialized to some value != 0.
By the time we get here we've already checked has_speed and set
speed=0 when has_speed=false. (The qmp marshaller generated code does
indeed leave the int64_t uninitialized.)
Stefan
