qemu-block
[Top][All Lists]
Advanced

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

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


From: Max Reitz
Subject: Re: [PATCH v15 10/13] qapi: block-stream: add "bottom" argument
Date: Tue, 5 Jan 2021 13:51:53 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0

On 22.12.20 19:00, Vladimir Sementsov-Ogievskiy wrote:
22.12.2020 19:07, Max Reitz wrote:
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)?

See my answer on 09. I just have some doubts around backing-fmt and decided to keep it as is.

I don't think that we really need backing-fmt. We shouldn't have use-cases when backing-fmt is set to something another than final base node. Therefore, using format_name of final base node is a correct thing to do. So, I don't see the reason now for introducing new option.

Yup, yup, all good.

Reviewed-by: Max Reitz <mreitz@redhat.com>




reply via email to

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