[PATCH 2/2] block: stream: Allow users to request only format driver names in backing file format

Peter Krempa posted 2 patches 1 year ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH 2/2] block: stream: Allow users to request only format driver names in backing file format
Posted by Peter Krempa 1 year ago
Introduce a new flag 'backing_file_format_no_protocol' for the
block-commit QMP command which instructs the internals to use 'raw'
instead of the protocol driver in case when a image is used without a
dummy 'raw' wrapper.

The flag is designed such that it can be always asserted by management
tools even when there isn't any update to backing files.

The flag will be used by libvirt so that the backing images still
reference the proper format even when libvirt will stop using the dummy
raw driver (raw driver with no other config). Libvirt needs this so that
the images stay compatible with older libvirt versions which didn't
expect that a protocol driver name can appear in the backing file format
field.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 block/monitor/block-hmp-cmds.c         |  2 +-
 block/stream.c                         | 10 +++++++++-
 blockdev.c                             |  7 +++++++
 include/block/block_int-global-state.h |  3 +++
 qapi/block-core.json                   | 11 ++++++++++-
 5 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index c729cbf1eb..28e708a981 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -509,7 +509,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
     const char *base = qdict_get_try_str(qdict, "base");
     int64_t speed = qdict_get_try_int(qdict, "speed", 0);

-    qmp_block_stream(device, device, base, NULL, NULL, NULL,
+    qmp_block_stream(device, device, base, NULL, NULL, NULL, false, false,
                      qdict_haskey(qdict, "speed"), speed,
                      true, BLOCKDEV_ON_ERROR_REPORT, NULL,
                      false, false, false, false, &error);
diff --git a/block/stream.c b/block/stream.c
index 01fe7c0f16..42befd6b1d 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -39,6 +39,7 @@ typedef struct StreamBlockJob {
     BlockDriverState *target_bs;
     BlockdevOnError on_error;
     char *backing_file_str;
+    bool backing_file_format_no_protocol;
     bool bs_read_only;
 } StreamBlockJob;

@@ -95,7 +96,12 @@ static int stream_prepare(Job *job)
         if (unfiltered_base) {
             base_id = s->backing_file_str ?: unfiltered_base->filename;
             if (unfiltered_base->drv) {
-                base_fmt = unfiltered_base->drv->format_name;
+                if (s->backing_file_format_no_protocol &&
+                    unfiltered_base->drv->protocol_name) {
+                    base_fmt = "raw";
+                } else {
+                    base_fmt = unfiltered_base->drv->format_name;
+                }
             }
         }

@@ -247,6 +253,7 @@ static const BlockJobDriver stream_job_driver = {

 void stream_start(const char *job_id, BlockDriverState *bs,
                   BlockDriverState *base, const char *backing_file_str,
+                  bool backing_file_format_no_protocol,
                   BlockDriverState *bottom,
                   int creation_flags, int64_t speed,
                   BlockdevOnError on_error,
@@ -398,6 +405,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
     s->base_overlay = base_overlay;
     s->above_base = above_base;
     s->backing_file_str = g_strdup(backing_file_str);
+    s->backing_file_format_no_protocol = backing_file_format_no_protocol;
     s->cor_filter_bs = cor_filter_bs;
     s->target_bs = bs;
     s->bs_read_only = bs_read_only;
diff --git a/blockdev.c b/blockdev.c
index 038031bb03..dc477c4f7e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2408,6 +2408,8 @@ void qmp_block_stream(const char *job_id, const char *device,
                       const char *base,
                       const char *base_node,
                       const char *backing_file,
+                      bool has_backing_file_format_no_protocol,
+                      bool backing_file_format_no_protocol,
                       const char *bottom,
                       bool has_speed, int64_t speed,
                       bool has_on_error, BlockdevOnError on_error,
@@ -2443,6 +2445,10 @@ void qmp_block_stream(const char *job_id, const char *device,
         return;
     }

+    if (!has_backing_file_format_no_protocol) {
+        backing_file_format_no_protocol = false;
+    }
+
     if (!has_on_error) {
         on_error = BLOCKDEV_ON_ERROR_REPORT;
     }
@@ -2531,6 +2537,7 @@ void qmp_block_stream(const char *job_id, const char *device,
     }

     stream_start(job_id, bs, base_bs, backing_file,
+                 backing_file_format_no_protocol,
                  bottom_bs, job_flags, has_speed ? speed : 0, on_error,
                  filter_node_name, &local_err);
     if (local_err) {
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index 4f253ff362..4301061048 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -46,6 +46,8 @@
  * flatten the whole backing file chain onto @bs.
  * @backing_file_str: The file name that will be written to @bs as the
  * the new backing file if the job completes. Ignored if @base is %NULL.
+ * @backing_file_format_no_protocol: Use format name instead of potental
+ *                                   protocol name as backing image format
  * @creation_flags: Flags that control the behavior of the Job lifetime.
  *                  See @BlockJobCreateFlags
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
@@ -64,6 +66,7 @@
  */
 void stream_start(const char *job_id, BlockDriverState *bs,
                   BlockDriverState *base, const char *backing_file_str,
+                  bool backing_file_format_no_protocol,
                   BlockDriverState *bottom,
                   int creation_flags, int64_t speed,
                   BlockdevOnError on_error,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 367e896905..796cc582d5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2829,6 +2829,13 @@
 #     Care should be taken when specifying the string, to specify a
 #     valid filename or protocol.  (Since 2.1)
 #
+# @backing-file-format-no-protocol: If true always use a 'format' driver name
+#     for the 'backing file format' field if updating the image header of 'top'.
+#     Otherwise the real name of the driver of the backing
+#     image may be used which may be a protocol driver.
+#
+#     (default: false; since: 8.2)
+#
 # @speed: the maximum speed, in bytes per second
 #
 # @on-error: the action to take on an error (default report). 'stop'
@@ -2867,7 +2874,9 @@
 ##
 { 'command': 'block-stream',
   'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
-            '*base-node': 'str', '*backing-file': 'str', '*bottom': 'str',
+            '*base-node': 'str', '*backing-file': 'str',
+            '*backing-file-format-no-protocol': 'bool',
+            '*bottom': 'str',
             '*speed': 'int', '*on-error': 'BlockdevOnError',
             '*filter-node-name': 'str',
             '*auto-finalize': 'bool', '*auto-dismiss': 'bool' },
-- 
2.43.0
Re: [PATCH 2/2] block: stream: Allow users to request only format driver names in backing file format
Posted by Vladimir Sementsov-Ogievskiy 12 months ago
On 24.11.23 17:52, Peter Krempa wrote:
> Introduce a new flag 'backing_file_format_no_protocol' for the
> block-commit QMP command which instructs the internals to use 'raw'
> instead of the protocol driver in case when a image is used without a
> dummy 'raw' wrapper.
> 
> The flag is designed such that it can be always asserted by management
> tools even when there isn't any update to backing files.
> 
> The flag will be used by libvirt so that the backing images still
> reference the proper format even when libvirt will stop using the dummy
> raw driver (raw driver with no other config). Libvirt needs this so that
> the images stay compatible with older libvirt versions which didn't
> expect that a protocol driver name can appear in the backing file format
> field.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>   block/monitor/block-hmp-cmds.c         |  2 +-
>   block/stream.c                         | 10 +++++++++-
>   blockdev.c                             |  7 +++++++
>   include/block/block_int-global-state.h |  3 +++
>   qapi/block-core.json                   | 11 ++++++++++-
>   5 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index c729cbf1eb..28e708a981 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -509,7 +509,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
>       const char *base = qdict_get_try_str(qdict, "base");
>       int64_t speed = qdict_get_try_int(qdict, "speed", 0);
> 
> -    qmp_block_stream(device, device, base, NULL, NULL, NULL,
> +    qmp_block_stream(device, device, base, NULL, NULL, NULL, false, false,

that should be

+    qmp_block_stream(device, device, base, NULL, NULL, false, false, NULL,


( definitely we lack named arguments of python in C :)


with that fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

>                        qdict_haskey(qdict, "speed"), speed,
>                        true, BLOCKDEV_ON_ERROR_REPORT, NULL,
>                        false, false, false, false, &error);

...

> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2408,6 +2408,8 @@ void qmp_block_stream(const char *job_id, const char *device,
>                         const char *base,
>                         const char *base_node,
>                         const char *backing_file,
> +                      bool has_backing_file_format_no_protocol,
> +                      bool backing_file_format_no_protocol,
>                         const char *bottom,
>                         bool has_speed, int64_t speed,
>                         bool has_on_error, BlockdevOnError on_error,

-- 
Best regards,
Vladimir