Re: [PATCH v15 10/13] qapi: block-stream: add "bottom" argument

[Max Reitz <mreitz@redhat.com>]

On 16.12.20 07:17, Vladimir Sementsov-Ogievskiy wrote:
> The code already don't freeze base node and we try to make it prepared
> for the situation when base node is changed during the operation. In
> other words, block-stream doesn't own base node.
> 
> Let's introduce a new interface which should replace the current one,
> which will in better relations with the code. Specifying bottom node
> instead of base, and requiring it to be non-filter gives us the
> following benefits:
> 
>   - drop difference between above_base and base_overlay, which will be
>     renamed to just bottom, when old interface dropped
> 
>   - clean way to work with parallel streams/commits on the same backing
>     chain, which otherwise become a problem when we introduce a filter
>     for stream job
> 
>   - cleaner interface. Nobody will surprised the fact that base node may
>     disappear during block-stream, when there is no word about "base" in
>     the interface.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   qapi/block-core.json           | 12 ++++---
>   include/block/block_int.h      |  1 +
>   block/monitor/block-hmp-cmds.c |  3 +-
>   block/stream.c                 | 50 +++++++++++++++++++---------
>   blockdev.c                     | 59 ++++++++++++++++++++++++++++------
>   5 files changed, 94 insertions(+), 31 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b8094a5ec7..cb0066fd5c 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2517,10 +2517,14 @@
>   # @device: the device or node name of the top image
>   #
>   # @base: the common backing file name.
> -#        It cannot be set if @base-node is also set.
> +#        It cannot be set if @base-node or @bottom is also set.
>   #
>   # @base-node: the node name of the backing file.
> -#             It cannot be set if @base is also set. (Since 2.8)
> +#             It cannot be set if @base or @bottom is also set. (Since 2.8)
> +#
> +# @bottom: the last node in the chain that should be streamed into
> +#          top. It cannot be set if @base or @base-node is also set.
> +#          It cannot be filter node. (Since 6.0)

As far as I can make out, one of the results of our discussion on v14 
was that when using backing-file + bottom, we want to require the user 
to specify backing-fmt as well.  Now, backing-fmt isn’t present yet. 
Doesn’t that mean we have to make bottom + backing-file an error until 
we have backing-fmt (like it was in v14)?

Or do you consider the change to patch 9 sufficient to make at least the 
case work for which backing-file was purportedly introduced, i.e. FD 
passing?  (Patch 9’s new version will just take the format of the base 
post-streaming, which would be most likely a correct guess when using FD 
passing.)

(I.e., now with patch 9 being more liberal in guessing, perhaps you 
decided to no longer make backing-fmt mandatory after all.)

Max