Re: [Qemu-devel] [PATCH v6 for 2.1 07/10] block: add ability for block-stream to use node-name

[Benoît Canet]

The Tuesday 17 Jun 2014 à 17:53:55 (-0400), Jeff Cody wrote :
> This adds the ability for block-stream to use node-name arguments
> for base, to specify the backing image to stream from.
> 
> Both 'base' and 'base-node-name' are optional, but mutually exclusive.
> Either can be specified, but not both together.
> 
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  blockdev.c           | 48 +++++++++++++++++++++++++++++++++++++++---------
>  hmp.c                |  2 +-
>  qapi/block-core.json | 16 ++++++++++++----
>  qmp-commands.hx      |  2 +-
>  4 files changed, 53 insertions(+), 15 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 7a1c966..2926738 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1868,38 +1868,68 @@ static void block_job_cb(void *opaque, int ret)
>      bdrv_put_ref_bh_schedule(bs);
>  }
>  
> -void qmp_block_stream(const char *device, bool has_base,
> -                      const char *base, bool has_speed, int64_t speed,
> +void qmp_block_stream(const char *device,
> +                      bool has_base, const char *base,
> +                      bool has_base_node_name, const char *base_node_name,
> +                      bool has_speed, int64_t speed,
>                        bool has_on_error, BlockdevOnError on_error,
>                        Error **errp)
>  {
> -    BlockDriverState *bs;
> +    BlockDriverState *bs = NULL;
>      BlockDriverState *base_bs = NULL;
> +    BlockDriverState *tmp_bs;
>      Error *local_err = NULL;
> +    const char *base_name = NULL;
>  
>      if (!has_on_error) {
>          on_error = BLOCKDEV_ON_ERROR_REPORT;
>      }
>  
> +    if (has_base && has_base_node_name) {
> +        error_setg(errp, "'base' and 'base-node-name' are mutually 
> exclusive");
> +        return;
> +    }
> +
>      bs = bdrv_find(device);
>      if (!bs) {
>          error_set(errp, QERR_DEVICE_NOT_FOUND, device);
>          return;
>      }
>  
> +    if (has_base_node_name) {
> +        base_bs = bdrv_lookup_bs(NULL, base_node_name, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +        tmp_bs = bdrv_find_overlay(bs, base_bs);
> +        if (tmp_bs) {
> +            base_name = tmp_bs->backing_file;
> +        }
> +    }
> +
>      if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
>          return;
>      }
>  
> -    if (base) {
> +    if (has_base) {
>          base_bs = bdrv_find_backing_image(bs, base);
> -        if (base_bs == NULL) {
> -            error_set(errp, QERR_BASE_NOT_FOUND, base);
> -            return;
> -        }
> +        base_name = base;
> +    }
> +
> +    if (base_bs == NULL && (has_base || has_base_node_name)) {
> +        error_set(errp, QERR_BASE_NOT_FOUND, base);
> +        return;
> +    }
> +
> +    /* Verify that 'base' is in the same chain as 'bs', if 'base' was
> +     * specified */
> +    if (base_bs && !bdrv_chain_contains(bs, base_bs)) {
> +        error_setg(errp, "'device' and 'base' are not in the same chain");
> +        return;
>      }
>  
> -    stream_start(bs, base_bs, base, has_speed ? speed : 0,
> +    stream_start(bs, base_bs, base_name, has_speed ? speed : 0,
>                   on_error, block_job_cb, bs, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> diff --git a/hmp.c b/hmp.c
> index ccc35d4..bb934df 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1173,7 +1173,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
>      int64_t speed = qdict_get_try_int(qdict, "speed", 0);
>  
>      qmp_block_stream(device, base != NULL, base,
> -                     qdict_haskey(qdict, "speed"), speed,
> +                     false, NULL, qdict_haskey(qdict, "speed"), speed,
>                       true, BLOCKDEV_ON_ERROR_REPORT, &error);
>  
>      hmp_handle_error(mon, &error);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ae1dde9..d17e349 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -904,9 +904,17 @@
>  # On successful completion the image file is updated to drop the backing file
>  # and the BLOCK_JOB_COMPLETED event is emitted.
>  #
> -# @device: the device name
> +# @device:                  The device name.
> +#
> +# For 'base', either @base or @base-node-name may be set but not both. If
> +# neither is specified, the entire chain will be streamed into the active 
> image,
> +# and the chain will consist of a single image (the current active layer) 
> with
> +# no backing file.
>  #
> -# @base:   #optional the common backing file name
> +# @base:           #optional the common backing file name
> +#
> +# @base-node-name: #optional the block driver state node name of the
> +#                            common backing file.  (Since 2.1)
>  #
>  # @speed:  #optional the maximum speed, in bytes per second
>  #
> @@ -920,8 +928,8 @@
>  # Since: 1.1
>  ##
>  { 'command': 'block-stream',
> -  'data': { 'device': 'str', '*base': 'str', '*speed': 'int',
> -            '*on-error': 'BlockdevOnError' } }
> +  'data': { 'device': 'str', '*base': 'str', '*base-node-name': 'str',
> +            '*speed': 'int', '*on-error': 'BlockdevOnError' } }
>  
>  ##
>  # @block-job-set-speed:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 7eb90a9..41e3853 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -979,7 +979,7 @@ EQMP
>  
>      {
>          .name       = "block-stream",
> -        .args_type  = "device:B,base:s?,speed:o?,on-error:s?",
> +        .args_type  = 
> "device:B,base:s?,base-node-name:s?,speed:o?,on-error:s?",
>          .mhandler.cmd_new = qmp_marshal_input_block_stream,
>      },
>  
> -- 
> 1.9.3
> 
Reviewed-by: Benoit Canet <address@hidden>