Re: [PATCH v2 1/2] block: commit: Allow users to request only format driver names in backing file format
On Thu, Nov 30, 2023 at 13:24:18 -0600, Eric Blake wrote: > On Thu, Nov 30, 2023 at 05:06:03PM +0100, 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 > > Reviewed-by: Vladimir Sementsov-Ogievskiy > > --- > > > +++ b/qapi/block-core.json > > @@ -1810,6 +1810,14 @@ > > # 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 > > the > > +# overlay of 'top'. Otherwise the real name of the driver of the > > backing > > +# image may be used which may be a protocol driver. > > +# > > +# Can be used also when no image header will be updated. > > +# (default: false; since: 9.0) As I've previously stated, I don't really care about a name as long as I don't have to keep re-sending, > This is a long name. What about: But is the long name really a problem? > @backing-mask-protocol: If true, replace any protocol mentioned in the > 'backing file format' with 'raw', rather than storing the protocol > name as the backing format. Can be used even when no image header > will be updated (default false; since 9.0). Sounds okay to me. In the end, nobody will really see this as libvirt will be using it internally
Re: [PATCH v2 1/2] block: commit: Allow users to request only format driver names in backing file format
On Thu, Nov 30, 2023 at 05:06:03PM +0100, 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 > Reviewed-by: Vladimir Sementsov-Ogievskiy > --- > +++ b/qapi/block-core.json > @@ -1810,6 +1810,14 @@ > # 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 the > +# overlay of 'top'. Otherwise the real name of the driver of the backing > +# image may be used which may be a protocol driver. > +# > +# Can be used also when no image header will be updated. > +# (default: false; since: 9.0) This is a long name. What about: @backing-mask-protocol: If true, replace any protocol mentioned in the 'backing file format' with 'raw', rather than storing the protocol name as the backing format. Can be used even when no image header will be updated (default false; since 9.0). or s/mask/hide/ if that sounds better. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
[PATCH v2 1/2] block: commit: Allow users to request only format driver names in backing file format
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
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block.c| 37 +-
block/commit.c | 6 -
blockdev.c | 6 +
include/block/block-global-state.h | 3 ++-
include/block/block_int-common.h | 4 ++-
include/block/block_int-global-state.h | 3 +++
qapi/block-core.json | 11 +++-
tests/unit/test-bdrv-drain.c | 3 ++-
8 files changed, 61 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index bfb0861ec6..986a529941 100644
--- a/block.c
+++ b/block.c
@@ -1309,11 +1309,14 @@ static void bdrv_backing_detach(BdrvChild *c)
}
static int bdrv_backing_update_filename(BdrvChild *c, BlockDriverState *base,
-const char *filename, Error **errp)
+const char *filename,
+bool backing_file_format_no_protocol,
+Error **errp)
{
BlockDriverState *parent = c->opaque;
bool read_only = bdrv_is_read_only(parent);
int ret;
+const char *format_name;
GLOBAL_STATE_CODE();
if (read_only) {
@@ -1323,9 +1326,23 @@ static int bdrv_backing_update_filename(BdrvChild *c,
BlockDriverState *base,
}
}
-ret = bdrv_change_backing_file(parent, filename,
- base->drv ? base->drv->format_name : "",
- false);
+if (base->drv) {
+/*
+ * If the new base image doesn't have a format driver layer, which we
+ * detect by the fact that @base is a protocol driver, we record
+ * 'raw' as the format instead of putting the protocol name as the
+ * backing format
+ */
+if (backing_file_format_no_protocol && base->drv->protocol_name) {
+format_name = "raw";
+} else {
+format_name = base->drv->format_name;
+}
+} else {
+format_name = "";
+}
+
+ret = bdrv_change_backing_file(parent, filename, format_name, false);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not update backing file link");
}
@@ -1479,10 +1496,14 @@ static void GRAPH_WRLOCK bdrv_child_cb_detach(BdrvChild
*child)
}
static int bdrv_child_cb_update_filename(BdrvChild *c, BlockDriverState *base,
- const char *filename, Error **errp)
+ const char *filename,
+ bool backing_file_format_no_protocol,
+ Error **errp)
{
if (c->role & BDRV_CHILD_COW) {
-return bdrv_backing_update_filename(c, base, filename, errp);
+return bdrv_backing_update_filename(c, base, filename,
+backing_file_format_no_protocol,
+errp);
}
return 0;
}
@@ -5961,7 +5982,8 @@ void bdrv_unfreeze_backing_chain(BlockDriverState *bs,
BlockDriverState *base)
*
*/
int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
- const char *backing_file_str)
+ const char *backing_file_str,
+ bool backing_file_format_no_protocol)
{
BlockDriverState *explicit_top = top;
bool update_inherits_from;
@@ -6027,6 +6049,7 @@ int bdrv_drop_intermediate(BlockDriverState *top,
BlockDriverState *base,
if (c->klass->update_filename) {
ret = c->klass->update_filename(c, base, backing_file_str,
+backing_file_format_no_protocol,
_err);
if (ret < 0) {
/*
diff --git a/block/commit.c b/block/commit.c
index 69cc75be0c..5a584b712e 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -42,6 +42,7 @@ typedef struct CommitBlockJob {
bool base_read_only;
bool chain_frozen;
char *backing_file_str;
+bool backing_file_format_no_protocol;
} CommitBlockJob;
static int commit_prepare(Job *job)
@@ -61,7 +62,8 @@ static int
