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. > + Error *local_err = NULL; > + > + block_job_set_speed(job, speed, &local_err); > + if (error_is_set(&local_err)) { > + bs->job = NULL; > + g_free(job); > + bdrv_set_in_use(bs, 0); > + error_propagate(errp, local_err); > + return NULL; > + } > + } > return job; > } > > diff --git a/block/stream.c b/block/stream.c > index f0486a3..dc15fb6 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -281,13 +281,14 @@ static BlockJobType stream_job_type = { > }; > > void stream_start(BlockDriverState *bs, BlockDriverState *base, > - const char *base_id, BlockDriverCompletionFunc *cb, > + const char *base_id, int64_t speed, > + BlockDriverCompletionFunc *cb, > void *opaque, Error **errp) > { > StreamBlockJob *s; > Coroutine *co; > > - s = block_job_create(&stream_job_type, bs, cb, opaque, errp); > + s = block_job_create(&stream_job_type, bs, speed, cb, opaque, errp); > if (!s) { > return; /* bs must already be in use */ > } > diff --git a/block_int.h b/block_int.h > index 6015c27..bffca35 100644 > --- a/block_int.h > +++ b/block_int.h > @@ -343,6 +343,7 @@ int is_windows_drive(const char *filename); > * block_job_create: > * @job_type: The class object for the newly-created job. > * @bs: The block > + * @speed: The maximum speed, in bytes per second, or 0 for unlimited. > * @cb: Completion function for the job. > * @opaque: Opaque pointer value passed to @cb. > * @errp: A location to return DeviceInUse. > @@ -357,8 +358,8 @@ int is_windows_drive(const char *filename); > * called from a wrapper that is specific to the job type. > */ > 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); > > /** > * block_job_complete: > @@ -417,6 +418,7 @@ void block_job_cancel_sync(BlockJob *job); > * flatten the whole backing file chain onto @bs. > * @base_id: The file name that will be written to @bs as the new > * backing file if the job completes. Ignored if @base is %NULL. > + * @speed: The maximum speed, in bytes per second, or 0 for unlimited. > * @cb: Completion function for the job. > * @opaque: Opaque pointer value passed to @cb. > * @errp: A location to return DeviceInUse. > @@ -428,7 +430,8 @@ void block_job_cancel_sync(BlockJob *job); > * @base_id in the written image and to @base in the live BlockDriverState. > */ > void stream_start(BlockDriverState *bs, BlockDriverState *base, > - const char *base_id, BlockDriverCompletionFunc *cb, > + const char *base_id, int64_t speed, > + BlockDriverCompletionFunc *cb, > void *opaque, Error **errp); > > #endif /* BLOCK_INT_H */ > diff --git a/blockdev.c b/blockdev.c > index c484259..f18af16 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1091,7 +1091,8 @@ static void block_stream_cb(void *opaque, int ret) > } > > void qmp_block_stream(const char *device, bool has_base, > - const char *base, Error **errp) > + const char *base, bool has_speed, > + int64_t speed, Error **errp) > { > BlockDriverState *bs; > BlockDriverState *base_bs = NULL; > @@ -1110,7 +1111,8 @@ void qmp_block_stream(const char *device, bool has_base, > } > } > > - stream_start(bs, base_bs, base, block_stream_cb, bs, errp); > + stream_start(bs, base_bs, base, has_speed ? speed : 0, > + block_stream_cb, bs, errp); > if (error_is_set(errp)) { > return; > } > diff --git a/hmp-commands.hx b/hmp-commands.hx > index a6f5a84..66b70e1 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -71,8 +71,8 @@ ETEXI > > { > .name = "block_stream", > - .args_type = "device:B,base:s?", > - .params = "device [base]", > + .args_type = "device:B,speed:o?,base:s?", > + .params = "device [speed] [base]", > .help = "copy data from a backing file into a block device", > .mhandler.cmd = hmp_block_stream, > }, > diff --git a/hmp.c b/hmp.c > index f3e5163..eb96618 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -835,8 +835,10 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict) > Error *error = NULL; > const char *device = qdict_get_str(qdict, "device"); > const char *base = qdict_get_try_str(qdict, "base"); > + int64_t speed = qdict_get_try_int(qdict, "speed", 0); > > - qmp_block_stream(device, base != NULL, base, &error); > + qmp_block_stream(device, base != NULL, base, > + qdict_haskey(qdict, "speed"), speed, &error); > > hmp_handle_error(mon, &error); > } > diff --git a/qapi-schema.json b/qapi-schema.json > index d0d4f693..9950451 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1571,15 +1571,19 @@ > # > # @base: #optional the common backing file name > # > +# @speed: #optional the maximum speed, in bytes per second > +# > # Returns: Nothing on success > # If streaming is already active on this device, DeviceInUse > # If @device does not exist, DeviceNotFound > # If image streaming is not supported by this device, NotSupported > # If @base does not exist, BaseNotFound > +# If @speed is invalid, BlockJobSpeedInvalid > # > # Since: 1.1 > ## > -{ 'command': 'block-stream', 'data': { 'device': 'str', '*base': 'str' } } > +{ 'command': 'block-stream', 'data': { 'device': 'str', '*base': 'str', > + '*speed': 'int' } } > > ## > # @block-job-set-speed: > diff --git a/qmp-commands.hx b/qmp-commands.hx > index f972332..a5ed6d0 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -688,7 +688,7 @@ EQMP > > { > .name = "block-stream", > - .args_type = "device:B,base:s?", > + .args_type = "device:B,base:s?,speed:o?", > .mhandler.cmd_new = qmp_marshal_input_block_stream, > }, >