Re: [Qemu-devel] [PATCH 2/3] block: Add QMP support for streaming to an intermediate layer

[Kevin Wolf]

Am 20.02.2015 um 14:53 hat Alberto Garcia geschrieben:
> This adds the 'top' parameter to the 'block-stream' QMP command and
> checks that its value is valid before passing it to stream_start().
> 
> Signed-off-by: Alberto Garcia <address@hidden>

> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1013,6 +1013,9 @@
>  # with query-block-jobs.  The operation can be stopped before it has 
> completed
>  # using the block-job-cancel command.
>  #
> +# Data is copied to the top image, which defaults to the active layer if no 
> other
> +# file is selected.
> +#
>  # If a base file is specified then sectors are not copied from that base 
> file and
>  # its backing chain.  When streaming completes the image file will have the 
> base
>  # file as its backing file.  This can be used to stream a subset of the 
> backing
> @@ -1025,8 +1028,14 @@
>  #
>  # @base:   #optional the common backing file name
>  #
> -# @backing-file: #optional The backing file string to write into the active
> -#                          layer. This filename is not validated.
> +# @top:    #optional Top image, only sectors below this image are streamed
> +#                    into it.
> +#
> +#                    If not specified, the top image is the active layer.
> +#                    (Since 2.3)
> +#
> +# @backing-file: #optional The backing file string to write into the top
> +#                          image. This filename is not validated.
>  #
>  #                          If a pathname string is such that it cannot be
>  #                          resolved by QEMU, that means that subsequent QMP 
> or
> @@ -1052,8 +1061,9 @@
>  # Since: 1.1
>  ##
>  { 'command': 'block-stream',
> -  'data': { 'device': 'str', '*base': 'str', '*backing-file': 'str',
> -            '*speed': 'int', '*on-error': 'BlockdevOnError' } }
> +  'data': { 'device': 'str', '*base': 'str', '*top': 'str',
> +            '*backing-file': 'str', '*speed': 'int',
> +            '*on-error': 'BlockdevOnError' } }

While in patch 1 it would only be nice to avoid the additional argument,
I think we absolutely have to avoid it here in the external interface.

There is no point in specifying some root node as 'device' that isn't
actually involved in the operation; worse, it isn't even possible in the
general case because 'top' could have multiple users/parents.

A better interface would probably be to allow node names for 'device'
and leave everything else as it is.

Kevin