Skip to content

Commit

Permalink
commit: Allow users to request only format driver names in backing fi…
Browse files Browse the repository at this point in the history
…le format

Introduce a new flag 'backing-mask-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>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <2cb46e37093ce793ea1604abc8bbb90f4c8e434b.1701796348.git.pkrempa@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
pipo authored and kevmw committed Jan 19, 2024
1 parent 14f2a37 commit 5ce5461
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 12 deletions.
37 changes: 30 additions & 7 deletions block.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_mask_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) {
Expand All @@ -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_mask_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");
}
Expand Down Expand Up @@ -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_mask_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_mask_protocol,
errp);
}
return 0;
}
Expand Down Expand Up @@ -5803,7 +5824,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_mask_protocol)
{
BlockDriverState *explicit_top = top;
bool update_inherits_from;
Expand Down Expand Up @@ -5869,6 +5891,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_mask_protocol,
&local_err);
if (ret < 0) {
/*
Expand Down
6 changes: 5 additions & 1 deletion block/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ typedef struct CommitBlockJob {
bool base_read_only;
bool chain_frozen;
char *backing_file_str;
bool backing_mask_protocol;
} CommitBlockJob;

static int commit_prepare(Job *job)
Expand All @@ -61,7 +62,8 @@ static int commit_prepare(Job *job)
/* FIXME: bdrv_drop_intermediate treats total failures and partial failures
* identically. Further work is needed to disambiguate these cases. */
return bdrv_drop_intermediate(s->commit_top_bs, s->base_bs,
s->backing_file_str);
s->backing_file_str,
s->backing_mask_protocol);
}

static void commit_abort(Job *job)
Expand Down Expand Up @@ -254,6 +256,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *base, BlockDriverState *top,
int creation_flags, int64_t speed,
BlockdevOnError on_error, const char *backing_file_str,
bool backing_mask_protocol,
const char *filter_node_name, Error **errp)
{
CommitBlockJob *s;
Expand Down Expand Up @@ -408,6 +411,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
blk_set_disable_request_queuing(s->top, true);

s->backing_file_str = g_strdup(backing_file_str);
s->backing_mask_protocol = backing_mask_protocol;
s->on_error = on_error;

trace_commit_start(bs, base, top, s);
Expand Down
6 changes: 6 additions & 0 deletions blockdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -2420,6 +2420,8 @@ void qmp_block_commit(const char *job_id, const char *device,
const char *top_node,
const char *top,
const char *backing_file,
bool has_backing_mask_protocol,
bool backing_mask_protocol,
bool has_speed, int64_t speed,
bool has_on_error, BlockdevOnError on_error,
const char *filter_node_name,
Expand Down Expand Up @@ -2450,6 +2452,9 @@ void qmp_block_commit(const char *job_id, const char *device,
if (has_auto_dismiss && !auto_dismiss) {
job_flags |= JOB_MANUAL_DISMISS;
}
if (!has_backing_mask_protocol) {
backing_mask_protocol = false;
}

/* Important Note:
* libvirt relies on the DeviceNotFound error class in order to probe for
Expand Down Expand Up @@ -2591,6 +2596,7 @@ void qmp_block_commit(const char *job_id, const char *device,
}
commit_start(job_id, bs, base_bs, top_bs, job_flags,
speed, on_error, backing_file,
backing_mask_protocol,
filter_node_name, &local_err);
}
if (local_err != NULL) {
Expand Down
3 changes: 2 additions & 1 deletion include/block/block-global-state.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ int GRAPH_RDLOCK bdrv_make_empty(BdrvChild *c, Error **errp);

void bdrv_register(BlockDriver *bdrv);
int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
const char *backing_file_str);
const char *backing_file_str,
bool backing_mask_protocol);

BlockDriverState * GRAPH_RDLOCK
bdrv_find_overlay(BlockDriverState *active, BlockDriverState *bs);
Expand Down
4 changes: 3 additions & 1 deletion include/block/block_int-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,9 @@ struct BdrvChildClass {
* can update its reference.
*/
int (*update_filename)(BdrvChild *child, BlockDriverState *new_base,
const char *filename, Error **errp);
const char *filename,
bool backing_mask_protocol,
Error **errp);

bool (*change_aio_ctx)(BdrvChild *child, AioContext *ctx,
GHashTable *visited, Transaction *tran,
Expand Down
3 changes: 3 additions & 0 deletions include/block/block_int-global-state.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ void stream_start(const char *job_id, BlockDriverState *bs,
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
* @on_error: The action to take upon error.
* @backing_file_str: String to use as the backing file in @top's overlay
* @backing_mask_protocol: Replace potential protocol name with 'raw' in
* 'backing file format' header
* @filter_node_name: The node name that should be assigned to the filter
* driver that the commit job inserts into the graph above @top. NULL means
* that a node name should be autogenerated.
Expand All @@ -92,6 +94,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *base, BlockDriverState *top,
int creation_flags, int64_t speed,
BlockdevOnError on_error, const char *backing_file_str,
bool backing_mask_protocol,
const char *filter_node_name, Error **errp);
/**
* commit_active_start:
Expand Down
8 changes: 7 additions & 1 deletion qapi/block-core.json
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,11 @@
# Care should be taken when specifying the string, to specify a
# valid filename or protocol. (Since 2.1)
#
# @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).
#
# @speed: the maximum speed, in bytes per second
#
# @on-error: the action to take on an error. 'ignore' means that the
Expand Down Expand Up @@ -1856,7 +1861,8 @@
'*base': { 'type': 'str', 'features': [ 'deprecated' ] },
'*top-node': 'str',
'*top': { 'type': 'str', 'features': [ 'deprecated' ] },
'*backing-file': 'str', '*speed': 'int',
'*backing-file': 'str', '*backing-mask-protocol': 'bool',
'*speed': 'int',
'*on-error': 'BlockdevOnError',
'*filter-node-name': 'str',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool' },
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test-bdrv-drain.c
Original file line number Diff line number Diff line change
Expand Up @@ -1591,6 +1591,7 @@ static const BlockJobDriver test_simple_job_driver = {
static int drop_intermediate_poll_update_filename(BdrvChild *child,
BlockDriverState *new_base,
const char *filename,
bool backing_mask_protocol,
Error **errp)
{
/*
Expand Down Expand Up @@ -1702,7 +1703,7 @@ static void test_drop_intermediate_poll(void)
job->should_complete = true;

g_assert(!job_has_completed);
ret = bdrv_drop_intermediate(chain[1], chain[0], NULL);
ret = bdrv_drop_intermediate(chain[1], chain[0], NULL, false);
aio_poll(qemu_get_aio_context(), false);
g_assert(ret == 0);
g_assert(job_has_completed);
Expand Down

0 comments on commit 5ce5461

Please sign in to comment.