copy-before-write: allow specifying minimum cluster size

In the context of backup fleecing, discarding the source will not work
when the fleecing image has a larger granularity than the one used for
block-copy operations (can happen if the backup target has smaller
cluster size), because cbw_co_pdiscard_snapshot() will align down the
discard requests and thus effectively ignore then.

To make @discard-source work in such a scenario, allow specifying the
minimum cluster size used for block-copy operations and thus in
particular also the granularity for discard requests to the source.

The type 'size' (corresponding to uint64_t in C) is used in QAPI to
rule out negative inputs and for consistency with already existing
@cluster-size parameters. Since block_copy_calculate_cluster_size()
uses int64_t for its result, a check that the input is not too large
is added in block_copy_state_new() before calling it. The calculation
in block_copy_calculate_cluster_size() is done in the target int64_t
type.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Markus Armbruster <armbru@redhat.com> (QAPI schema)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-Id: <20240711120915.310243-2-f.ebner@proxmox.com>
[vsementsov: switch version to 9.2 in QAPI doc]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
This commit is contained in:
Fiona Ebner 2024-07-11 14:09:14 +02:00 committed by Vladimir Sementsov-Ogievskiy
parent 3b14a767ea
commit 9484ad6c17
4 changed files with 38 additions and 12 deletions

View File

@ -310,6 +310,7 @@ void block_copy_set_copy_opts(BlockCopyState *s, bool use_copy_range,
}
static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
int64_t min_cluster_size,
Error **errp)
{
int ret;
@ -319,6 +320,9 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
min_cluster_size = MAX(min_cluster_size,
(int64_t)BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
target_does_cow = bdrv_backing_chain_next(target);
/*
@ -329,13 +333,13 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
ret = bdrv_get_info(target, &bdi);
if (ret == -ENOTSUP && !target_does_cow) {
/* Cluster size is not defined */
warn_report("The target block device doesn't provide "
"information about the block size and it doesn't have a "
"backing file. The default block size of %u bytes is "
"used. If the actual block size of the target exceeds "
"this default, the backup may be unusable",
BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
warn_report("The target block device doesn't provide information about "
"the block size and it doesn't have a backing file. The "
"(default) block size of %" PRIi64 " bytes is used. If the "
"actual block size of the target exceeds this value, the "
"backup may be unusable",
min_cluster_size);
return min_cluster_size;
} else if (ret < 0 && !target_does_cow) {
error_setg_errno(errp, -ret,
"Couldn't determine the cluster size of the target image, "
@ -345,16 +349,17 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
return ret;
} else if (ret < 0 && target_does_cow) {
/* Not fatal; just trudge on ahead. */
return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
return min_cluster_size;
}
return MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
return MAX(min_cluster_size, bdi.cluster_size);
}
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
BlockDriverState *copy_bitmap_bs,
const BdrvDirtyBitmap *bitmap,
bool discard_source,
uint64_t min_cluster_size,
Error **errp)
{
ERRP_GUARD();
@ -365,7 +370,18 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
GLOBAL_STATE_CODE();
cluster_size = block_copy_calculate_cluster_size(target->bs, errp);
if (min_cluster_size > INT64_MAX) {
error_setg(errp, "min-cluster-size too large: %" PRIu64 " > %" PRIi64,
min_cluster_size, INT64_MAX);
return NULL;
} else if (min_cluster_size && !is_power_of_2(min_cluster_size)) {
error_setg(errp, "min-cluster-size needs to be a power of 2");
return NULL;
}
cluster_size = block_copy_calculate_cluster_size(target->bs,
(int64_t)min_cluster_size,
errp);
if (cluster_size < 0) {
return NULL;
}

View File

@ -417,6 +417,7 @@ static BlockdevOptions *cbw_parse_options(QDict *options, Error **errp)
qdict_extract_subqdict(options, NULL, "bitmap");
qdict_del(options, "on-cbw-error");
qdict_del(options, "cbw-timeout");
qdict_del(options, "min-cluster-size");
out:
visit_free(v);
@ -476,8 +477,10 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
bs->file->bs->supported_zero_flags);
s->discard_source = flags & BDRV_O_CBW_DISCARD_SOURCE;
s->bcs = block_copy_state_new(bs->file, s->target, bs, bitmap,
flags & BDRV_O_CBW_DISCARD_SOURCE, errp);
flags & BDRV_O_CBW_DISCARD_SOURCE,
opts->min_cluster_size, errp);
if (!s->bcs) {
error_prepend(errp, "Cannot create block-copy-state: ");
return -EINVAL;

View File

@ -28,6 +28,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
BlockDriverState *copy_bitmap_bs,
const BdrvDirtyBitmap *bitmap,
bool discard_source,
uint64_t min_cluster_size,
Error **errp);
/* Function should be called prior any actual copy request */

View File

@ -4639,12 +4639,18 @@
# @on-cbw-error parameter will decide how this failure is handled.
# Default 0. (Since 7.1)
#
# @min-cluster-size: Minimum size of blocks used by copy-before-write
# operations. Has to be a power of 2. No effect if smaller than
# the maximum of the target's cluster size and 64 KiB. Default 0.
# (Since 9.2)
#
# Since: 6.2
##
{ 'struct': 'BlockdevOptionsCbw',
'base': 'BlockdevOptionsGenericFormat',
'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap',
'*on-cbw-error': 'OnCbwError', '*cbw-timeout': 'uint32' } }
'*on-cbw-error': 'OnCbwError', '*cbw-timeout': 'uint32',
'*min-cluster-size': 'size' } }
##
# @BlockdevOptions: