The main consumer of cluster-size is block-copy. Let's calculate it
here instead of passing through backup-top.
We are going to publish copy-before-write filter soon, so it will be
created through options. But we don't want for now to make explicit
option for cluster-size, let's continue to calculate it automatically.
So, now is the time to get rid of cluster_size argument for
bdrv_cbw_append().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/copy-before-write.h | 1 -
include/block/block-copy.h | 5 +--
block/backup.c | 62 ++++++--------------------------------
block/block-copy.c | 51 ++++++++++++++++++++++++++++++-
block/copy-before-write.c | 10 +++---
5 files changed, 66 insertions(+), 63 deletions(-)
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 538aab8bdb..b386fd8f01 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -32,7 +32,6 @@
BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
- uint64_t cluster_size,
bool compress,
BlockCopyState **bcs,
Error **errp);
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index c013a20e1e..8138686eb4 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -23,8 +23,8 @@ typedef struct BlockCopyState BlockCopyState;
typedef struct BlockCopyCallState BlockCopyCallState;
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
- int64_t cluster_size, bool use_copy_range,
- bool compress, Error **errp);
+ bool use_copy_range, bool compress,
+ Error **errp);
void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm);
@@ -86,6 +86,7 @@ void block_copy_kick(BlockCopyCallState *call_state);
void block_copy_call_cancel(BlockCopyCallState *call_state);
BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s);
+int64_t block_copy_cluster_size(BlockCopyState *s);
void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip);
#endif /* BLOCK_COPY_H */
diff --git a/block/backup.c b/block/backup.c
index 29db3a0ef6..af17149f22 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -29,8 +29,6 @@
#include "block/copy-before-write.h"
-#define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
-
typedef struct BackupBlockJob {
BlockJob common;
BlockDriverState *cbw;
@@ -354,43 +352,6 @@ static const BlockJobDriver backup_job_driver = {
.set_speed = backup_set_speed,
};
-static int64_t backup_calculate_cluster_size(BlockDriverState *target,
- Error **errp)
-{
- int ret;
- BlockDriverInfo bdi;
- bool target_does_cow = bdrv_backing_chain_next(target);
-
- /*
- * If there is no backing file on the target, we cannot rely on COW if our
- * backup cluster size is smaller than the target cluster size. Even for
- * targets with a backing file, try to avoid COW if possible.
- */
- 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",
- BACKUP_CLUSTER_SIZE_DEFAULT);
- return BACKUP_CLUSTER_SIZE_DEFAULT;
- } else if (ret < 0 && !target_does_cow) {
- error_setg_errno(errp, -ret,
- "Couldn't determine the cluster size of the target image, "
- "which has no backing file");
- error_append_hint(errp,
- "Aborting, since this may create an unusable destination image\n");
- return ret;
- } else if (ret < 0 && target_does_cow) {
- /* Not fatal; just trudge on ahead. */
- return BACKUP_CLUSTER_SIZE_DEFAULT;
- }
-
- return MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
-}
-
BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BlockDriverState *target, int64_t speed,
MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
@@ -448,11 +409,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
return NULL;
}
- cluster_size = backup_calculate_cluster_size(target, errp);
- if (cluster_size < 0) {
- goto error;
- }
-
if (perf->max_workers < 1) {
error_setg(errp, "max-workers must be greater than zero");
return NULL;
@@ -464,13 +420,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
return NULL;
}
- if (perf->max_chunk && perf->max_chunk < cluster_size) {
- error_setg(errp, "Required max-chunk (%" PRIi64 ") is less than backup "
- "cluster size (%" PRIi64 ")", perf->max_chunk, cluster_size);
- return NULL;
- }
-
-
if (sync_bitmap) {
/* If we need to write to this bitmap, check that we can: */
if (bitmap_mode != BITMAP_SYNC_MODE_NEVER &&
@@ -503,12 +452,19 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
goto error;
}
- cbw = bdrv_cbw_append(bs, target, filter_node_name,
- cluster_size, compress, &bcs, errp);
+ cbw = bdrv_cbw_append(bs, target, filter_node_name, compress, &bcs, errp);
if (!cbw) {
goto error;
}
+ cluster_size = block_copy_cluster_size(bcs);
+
+ if (perf->max_chunk && perf->max_chunk < cluster_size) {
+ error_setg(errp, "Required max-chunk (%" PRIi64 ") is less than backup "
+ "cluster size (%" PRIi64 ")", perf->max_chunk, cluster_size);
+ goto error;
+ }
+
/* job->len is fixed, so we can't allow resize */
job = block_job_create(job_id, &backup_job_driver, txn, cbw,
0, BLK_PERM_ALL,
diff --git a/block/block-copy.c b/block/block-copy.c
index daa1a2bf9f..9389c7c6c8 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -27,6 +27,7 @@
#define BLOCK_COPY_MAX_MEM (128 * MiB)
#define BLOCK_COPY_MAX_WORKERS 64
#define BLOCK_COPY_SLICE_TIME 100000000ULL /* ns */
+#define BLOCK_COPY_CLUSTER_SIZE_DEFAULT (1 << 16)
static coroutine_fn int block_copy_task_entry(AioTask *task);
@@ -243,13 +244,56 @@ static uint32_t block_copy_max_transfer(BdrvChild *source, BdrvChild *target)
target->bs->bl.max_transfer));
}
+static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
+ Error **errp)
+{
+ int ret;
+ BlockDriverInfo bdi;
+ bool target_does_cow = bdrv_backing_chain_next(target);
+
+ /*
+ * If there is no backing file on the target, we cannot rely on COW if our
+ * backup cluster size is smaller than the target cluster size. Even for
+ * targets with a backing file, try to avoid COW if possible.
+ */
+ 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;
+ } else if (ret < 0 && !target_does_cow) {
+ error_setg_errno(errp, -ret,
+ "Couldn't determine the cluster size of the target image, "
+ "which has no backing file");
+ error_append_hint(errp,
+ "Aborting, since this may create an unusable destination image\n");
+ return ret;
+ } else if (ret < 0 && target_does_cow) {
+ /* Not fatal; just trudge on ahead. */
+ return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
+ }
+
+ return MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+}
+
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
- int64_t cluster_size, bool use_copy_range,
+ bool use_copy_range,
bool compress, Error **errp)
{
BlockCopyState *s;
+ int64_t cluster_size;
BdrvDirtyBitmap *copy_bitmap;
+ cluster_size = block_copy_calculate_cluster_size(target->bs, errp);
+ if (cluster_size < 0) {
+ return NULL;
+ }
+
copy_bitmap = bdrv_create_dirty_bitmap(source->bs, cluster_size, NULL,
errp);
if (!copy_bitmap) {
@@ -843,6 +887,11 @@ BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s)
return s->copy_bitmap;
}
+int64_t block_copy_cluster_size(BlockCopyState *s)
+{
+ return s->cluster_size;
+}
+
void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip)
{
s->skip_unallocated = skip;
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 235251a640..a7996d54db 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -37,7 +37,6 @@
typedef struct BDRVCopyBeforeWriteState {
BlockCopyState *bcs;
BdrvChild *target;
- int64_t cluster_size;
} BDRVCopyBeforeWriteState;
static coroutine_fn int cbw_co_preadv(
@@ -52,13 +51,14 @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
{
BDRVCopyBeforeWriteState *s = bs->opaque;
uint64_t off, end;
+ int64_t cluster_size = block_copy_cluster_size(s->bcs);
if (flags & BDRV_REQ_WRITE_UNCHANGED) {
return 0;
}
- off = QEMU_ALIGN_DOWN(offset, s->cluster_size);
- end = QEMU_ALIGN_UP(offset + bytes, s->cluster_size);
+ off = QEMU_ALIGN_DOWN(offset, cluster_size);
+ end = QEMU_ALIGN_UP(offset + bytes, cluster_size);
return block_copy(s->bcs, off, end - off, true);
}
@@ -169,7 +169,6 @@ BlockDriver bdrv_cbw_filter = {
BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
- uint64_t cluster_size,
bool compress,
BlockCopyState **bcs,
Error **errp)
@@ -214,9 +213,8 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
}
appended = true;
- state->cluster_size = cluster_size;
state->bcs = block_copy_state_new(top->backing, state->target,
- cluster_size, false, compress, errp);
+ false, compress, errp);
if (!state->bcs) {
error_prepend(errp, "Cannot create block-copy-state: ");
goto fail;
--
2.29.2