qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/4] block: add 'speed' optional parameter to bl


From: Luiz Capitulino
Subject: Re: [Qemu-devel] [PATCH 3/4] block: add 'speed' optional parameter to block-stream
Date: Tue, 24 Apr 2012 16:31:24 -0300

On Mon, 23 Apr 2012 16:39:48 +0100
Stefan Hajnoczi <address@hidden> 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 <address@hidden>
> ---
>  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,
>      },
>  




reply via email to

[Prev in Thread] Current Thread [Next in Thread]