* [PULL 1/8] block: get type of block allocation in commit_run
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 1/5] copy-before-write: allow specifying minimum cluster size Vladimir Sementsov-Ogievskiy
` (13 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Vincent Vanlaer, Vladimir Sementsov-Ogievskiy
From: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
bdrv_co_common_block_status_above not only returns whether the block is
allocated, but also if it contains zeroes.
Signed-off-by: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20241026163010.2865002-2-libvirt-e6954efa@volkihar.be>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/commit.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 5df3d05346..ba0ba59316 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -15,6 +15,8 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
#include "trace.h"
+#include "block/block-common.h"
+#include "block/coroutines.h"
#include "block/block_int.h"
#include "block/blockjob_int.h"
#include "qapi/error.h"
@@ -167,9 +169,13 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
break;
}
/* Copy if allocated above the base */
- ret = blk_co_is_allocated_above(s->top, s->base_overlay, true,
- offset, COMMIT_BUFFER_SIZE, &n);
- copy = (ret > 0);
+ WITH_GRAPH_RDLOCK_GUARD() {
+ ret = bdrv_co_common_block_status_above(blk_bs(s->top),
+ s->base_overlay, true, true, offset, COMMIT_BUFFER_SIZE,
+ &n, NULL, NULL, NULL);
+ }
+
+ copy = (ret >= 0 && ret & BDRV_BLOCK_ALLOCATED);
trace_commit_one_iteration(s, offset, n, ret);
if (copy) {
assert(n < SIZE_MAX);
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 1/5] copy-before-write: allow specifying minimum cluster size
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 1/8] block: get type of block allocation in commit_run Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 2/5] backup: add minimum cluster size to performance options Vladimir Sementsov-Ogievskiy
` (12 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Fiona Ebner, Vladimir Sementsov-Ogievskiy,
Markus Armbruster
From: Fiona Ebner <f.ebner@proxmox.com>
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>
---
block/block-copy.c | 36 ++++++++++++++++++++++++++----------
block/copy-before-write.c | 5 ++++-
include/block/block-copy.h | 1 +
qapi/block-core.json | 8 +++++++-
4 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index cc618e4561..93eb1b2664 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -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;
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 853e01a1eb..a919b1f41b 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -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;
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index bdc703bacd..dd5cc82f3b 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -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 */
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9f6dd59298..6751022428 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 2/5] backup: add minimum cluster size to performance options
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 1/8] block: get type of block allocation in commit_run Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 1/5] copy-before-write: allow specifying minimum cluster size Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 2/8] block: move commit_run loop to separate function Vladimir Sementsov-Ogievskiy
` (11 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Fiona Ebner, Vladimir Sementsov-Ogievskiy,
Markus Armbruster
From: Fiona Ebner <f.ebner@proxmox.com>
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.
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-3-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>
---
block/backup.c | 2 +-
block/copy-before-write.c | 9 +++++++++
block/copy-before-write.h | 1 +
blockdev.c | 3 +++
qapi/block-core.json | 9 +++++++--
5 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 3dd2e229d2..a1292c01ec 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -458,7 +458,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
}
cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
- &bcs, errp);
+ perf->min_cluster_size, &bcs, errp);
if (!cbw) {
goto error;
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index a919b1f41b..e835987e52 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -548,6 +548,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
bool discard_source,
+ uint64_t min_cluster_size,
BlockCopyState **bcs,
Error **errp)
{
@@ -567,6 +568,14 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
qdict_put_str(opts, "file", bdrv_get_node_name(source));
qdict_put_str(opts, "target", bdrv_get_node_name(target));
+ if (min_cluster_size > INT64_MAX) {
+ error_setg(errp, "min-cluster-size too large: %" PRIu64 " > %" PRIi64,
+ min_cluster_size, INT64_MAX);
+ qobject_unref(opts);
+ return NULL;
+ }
+ qdict_put_int(opts, "min-cluster-size", (int64_t)min_cluster_size);
+
top = bdrv_insert_node(source, opts, flags, errp);
if (!top) {
return NULL;
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 01af0cd3c4..2a5d4ba693 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -40,6 +40,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
bool discard_source,
+ uint64_t min_cluster_size,
BlockCopyState **bcs,
Error **errp);
void bdrv_cbw_drop(BlockDriverState *bs);
diff --git a/blockdev.c b/blockdev.c
index 835064ed03..6740663fda 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2655,6 +2655,9 @@ static BlockJob *do_backup_common(BackupCommon *backup,
if (backup->x_perf->has_max_chunk) {
perf.max_chunk = backup->x_perf->max_chunk;
}
+ if (backup->x_perf->has_min_cluster_size) {
+ perf.min_cluster_size = backup->x_perf->min_cluster_size;
+ }
}
if ((backup->sync == MIRROR_SYNC_MODE_BITMAP) ||
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6751022428..c3b0a2376b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1551,11 +1551,16 @@
# it should not be less than job cluster size which is calculated
# as maximum of target image cluster size and 64k. Default 0.
#
+# @min-cluster-size: Minimum size of blocks used by copy-before-write
+# and background copy 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.0
##
{ 'struct': 'BackupPerf',
- 'data': { '*use-copy-range': 'bool',
- '*max-workers': 'int', '*max-chunk': 'int64' } }
+ 'data': { '*use-copy-range': 'bool', '*max-workers': 'int',
+ '*max-chunk': 'int64', '*min-cluster-size': 'size' } }
##
# @BackupCommon:
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 2/8] block: move commit_run loop to separate function
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2025-04-29 11:33 ` [PULL 2/5] backup: add minimum cluster size to performance options Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 3/8] block: refactor error handling of commit_iteration Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Vincent Vanlaer, Vladimir Sementsov-Ogievskiy
From: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Signed-off-by: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20241026163010.2865002-3-libvirt-e6954efa@volkihar.be>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/commit.c | 89 +++++++++++++++++++++++++++++---------------------
1 file changed, 52 insertions(+), 37 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index ba0ba59316..3ee0ade7df 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -128,6 +128,55 @@ static void commit_clean(Job *job)
blk_unref(s->top);
}
+static int commit_iteration(CommitBlockJob *s, int64_t offset,
+ int64_t *n, void *buf)
+{
+ int ret = 0;
+ bool copy;
+ bool error_in_source = true;
+
+ /* Copy if allocated above the base */
+ WITH_GRAPH_RDLOCK_GUARD() {
+ ret = bdrv_co_common_block_status_above(blk_bs(s->top),
+ s->base_overlay, true, true, offset, COMMIT_BUFFER_SIZE,
+ n, NULL, NULL, NULL);
+ }
+
+ copy = (ret >= 0 && ret & BDRV_BLOCK_ALLOCATED);
+ trace_commit_one_iteration(s, offset, *n, ret);
+ if (copy) {
+ assert(*n < SIZE_MAX);
+
+ ret = blk_co_pread(s->top, offset, *n, buf, 0);
+ if (ret >= 0) {
+ ret = blk_co_pwrite(s->base, offset, *n, buf, 0);
+ if (ret < 0) {
+ error_in_source = false;
+ }
+ }
+ }
+ if (ret < 0) {
+ BlockErrorAction action = block_job_error_action(&s->common,
+ s->on_error,
+ error_in_source,
+ -ret);
+ if (action == BLOCK_ERROR_ACTION_REPORT) {
+ return ret;
+ } else {
+ *n = 0;
+ return 0;
+ }
+ }
+ /* Publish progress */
+ job_progress_update(&s->common.job, *n);
+
+ if (copy) {
+ block_job_ratelimit_processed_bytes(&s->common, *n);
+ }
+
+ return 0;
+}
+
static int coroutine_fn commit_run(Job *job, Error **errp)
{
CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
@@ -158,9 +207,6 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE);
for (offset = 0; offset < len; offset += n) {
- bool copy;
- bool error_in_source = true;
-
/* Note that even when no rate limit is applied we need to yield
* with no pending I/O here so that bdrv_drain_all() returns.
*/
@@ -168,42 +214,11 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
if (job_is_cancelled(&s->common.job)) {
break;
}
- /* Copy if allocated above the base */
- WITH_GRAPH_RDLOCK_GUARD() {
- ret = bdrv_co_common_block_status_above(blk_bs(s->top),
- s->base_overlay, true, true, offset, COMMIT_BUFFER_SIZE,
- &n, NULL, NULL, NULL);
- }
- copy = (ret >= 0 && ret & BDRV_BLOCK_ALLOCATED);
- trace_commit_one_iteration(s, offset, n, ret);
- if (copy) {
- assert(n < SIZE_MAX);
-
- ret = blk_co_pread(s->top, offset, n, buf, 0);
- if (ret >= 0) {
- ret = blk_co_pwrite(s->base, offset, n, buf, 0);
- if (ret < 0) {
- error_in_source = false;
- }
- }
- }
- if (ret < 0) {
- BlockErrorAction action =
- block_job_error_action(&s->common, s->on_error,
- error_in_source, -ret);
- if (action == BLOCK_ERROR_ACTION_REPORT) {
- return ret;
- } else {
- n = 0;
- continue;
- }
- }
- /* Publish progress */
- job_progress_update(&s->common.job, n);
+ ret = commit_iteration(s, offset, &n, buf);
- if (copy) {
- block_job_ratelimit_processed_bytes(&s->common, n);
+ if (ret < 0) {
+ return ret;
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 3/8] block: refactor error handling of commit_iteration
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2025-04-29 11:33 ` [PULL 2/8] block: move commit_run loop to separate function Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 3/5] block/reqlist: allow adding overlapping requests Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Vincent Vanlaer, Vladimir Sementsov-Ogievskiy
From: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Signed-off-by: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Message-Id: <20241026163010.2865002-4-libvirt-e6954efa@volkihar.be>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/commit.c | 62 ++++++++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 27 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 3ee0ade7df..0fbf684728 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -129,51 +129,59 @@ static void commit_clean(Job *job)
}
static int commit_iteration(CommitBlockJob *s, int64_t offset,
- int64_t *n, void *buf)
+ int64_t *requested_bytes, void *buf)
{
+ int64_t bytes = *requested_bytes;
int ret = 0;
- bool copy;
bool error_in_source = true;
/* Copy if allocated above the base */
WITH_GRAPH_RDLOCK_GUARD() {
ret = bdrv_co_common_block_status_above(blk_bs(s->top),
s->base_overlay, true, true, offset, COMMIT_BUFFER_SIZE,
- n, NULL, NULL, NULL);
+ &bytes, NULL, NULL, NULL);
}
- copy = (ret >= 0 && ret & BDRV_BLOCK_ALLOCATED);
- trace_commit_one_iteration(s, offset, *n, ret);
- if (copy) {
- assert(*n < SIZE_MAX);
+ trace_commit_one_iteration(s, offset, bytes, ret);
- ret = blk_co_pread(s->top, offset, *n, buf, 0);
- if (ret >= 0) {
- ret = blk_co_pwrite(s->base, offset, *n, buf, 0);
- if (ret < 0) {
- error_in_source = false;
- }
- }
- }
if (ret < 0) {
- BlockErrorAction action = block_job_error_action(&s->common,
- s->on_error,
- error_in_source,
- -ret);
- if (action == BLOCK_ERROR_ACTION_REPORT) {
- return ret;
- } else {
- *n = 0;
- return 0;
+ goto fail;
+ }
+
+ if (ret & BDRV_BLOCK_ALLOCATED) {
+ assert(bytes < SIZE_MAX);
+
+ ret = blk_co_pread(s->top, offset, bytes, buf, 0);
+ if (ret < 0) {
+ goto fail;
}
+
+ ret = blk_co_pwrite(s->base, offset, bytes, buf, 0);
+ if (ret < 0) {
+ error_in_source = false;
+ goto fail;
+ }
+
+ block_job_ratelimit_processed_bytes(&s->common, bytes);
}
+
/* Publish progress */
- job_progress_update(&s->common.job, *n);
- if (copy) {
- block_job_ratelimit_processed_bytes(&s->common, *n);
+ job_progress_update(&s->common.job, bytes);
+
+ *requested_bytes = bytes;
+
+ return 0;
+
+fail:
+ BlockErrorAction action = block_job_error_action(&s->common, s->on_error,
+ error_in_source, -ret);
+ if (action == BLOCK_ERROR_ACTION_REPORT) {
+ return ret;
}
+ *requested_bytes = 0;
+
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 3/5] block/reqlist: allow adding overlapping requests
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2025-04-29 11:33 ` [PULL 3/8] block: refactor error handling of commit_iteration Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 4/8] block: allow commit to unmap zero blocks Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Fiona Ebner, qemu-stable,
Vladimir Sementsov-Ogievskiy
From: Fiona Ebner <f.ebner@proxmox.com>
Allow overlapping request by removing the assert that made it
impossible. There are only two callers:
1. block_copy_task_create()
It already asserts the very same condition before calling
reqlist_init_req().
2. cbw_snapshot_read_lock()
There is no need to have read requests be non-overlapping in
copy-before-write when used for snapshot-access. In fact, there was no
protection against two callers of cbw_snapshot_read_lock() calling
reqlist_init_req() with overlapping ranges and this could lead to an
assertion failure [1].
In particular, with the reproducer script below [0], two
cbw_co_snapshot_block_status() callers could race, with the second
calling reqlist_init_req() before the first one finishes and removes
its conflicting request.
[0]:
> #!/bin/bash -e
> dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=1024
> ./qemu-img create /tmp/fleecing.raw -f raw 1G
> (
> ./qemu-system-x86_64 --qmp stdio \
> --blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw \
> --blockdev raw,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.raw \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
> {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
> {"execute": "nbd-server-start", "arguments": {"addr": { "type": "unix", "data": { "path": "/tmp/nbd.socket" } } } }
> {"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "snap0", "type": "nbd", "name": "exp0"}}
> EOF
> ) &
> sleep 5
> while true; do
> ./qemu-nbd -d /dev/nbd0
> ./qemu-nbd -c /dev/nbd0 nbd:unix:/tmp/nbd.socket:exportname=exp0 -f raw -r
> nbdinfo --map 'nbd+unix:///exp0?socket=/tmp/nbd.socket'
> done
[1]:
> #5 0x000071e5f0088eb2 in __GI___assert_fail (...) at ./assert/assert.c:101
> #6 0x0000615285438017 in reqlist_init_req (...) at ../block/reqlist.c:23
> #7 0x00006152853e2d98 in cbw_snapshot_read_lock (...) at ../block/copy-before-write.c:237
> #8 0x00006152853e3068 in cbw_co_snapshot_block_status (...) at ../block/copy-before-write.c:304
> #9 0x00006152853f4d22 in bdrv_co_snapshot_block_status (...) at ../block/io.c:3726
> #10 0x000061528543a63e in snapshot_access_co_block_status (...) at ../block/snapshot-access.c:48
> #11 0x00006152853f1a0a in bdrv_co_do_block_status (...) at ../block/io.c:2474
> #12 0x00006152853f2016 in bdrv_co_common_block_status_above (...) at ../block/io.c:2652
> #13 0x00006152853f22cf in bdrv_co_block_status_above (...) at ../block/io.c:2732
> #14 0x00006152853d9a86 in blk_co_block_status_above (...) at ../block/block-backend.c:1473
> #15 0x000061528538da6c in blockstatus_to_extents (...) at ../nbd/server.c:2374
> #16 0x000061528538deb1 in nbd_co_send_block_status (...) at ../nbd/server.c:2481
> #17 0x000061528538f424 in nbd_handle_request (...) at ../nbd/server.c:2978
> #18 0x000061528538f906 in nbd_trip (...) at ../nbd/server.c:3121
> #19 0x00006152855a7caf in coroutine_trampoline (...) at ../util/coroutine-ucontext.c:175
Cc: qemu-stable@nongnu.org
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-Id: <20240712140716.517911-1-f.ebner@proxmox.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/copy-before-write.c | 3 ++-
block/reqlist.c | 2 --
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index e835987e52..81afeff1c7 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -66,7 +66,8 @@ typedef struct BDRVCopyBeforeWriteState {
/*
* @frozen_read_reqs: current read requests for fleecing user in bs->file
- * node. These areas must not be rewritten by guest.
+ * node. These areas must not be rewritten by guest. There can be multiple
+ * overlapping read requests.
*/
BlockReqList frozen_read_reqs;
diff --git a/block/reqlist.c b/block/reqlist.c
index 08cb57cfa4..098e807378 100644
--- a/block/reqlist.c
+++ b/block/reqlist.c
@@ -20,8 +20,6 @@
void reqlist_init_req(BlockReqList *reqs, BlockReq *req, int64_t offset,
int64_t bytes)
{
- assert(!reqlist_find_conflict(reqs, offset, bytes));
-
*req = (BlockReq) {
.offset = offset,
.bytes = bytes,
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 4/8] block: allow commit to unmap zero blocks
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2025-04-29 11:33 ` [PULL 3/5] block/reqlist: allow adding overlapping requests Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 4/5] block: Remove unused aio_task_pool_empty Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Vincent Vanlaer, Vladimir Sementsov-Ogievskiy
From: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Non-active block commits do not discard blocks only containing zeros,
causing images to lose sparseness after the commit. This commit fixes
that by writing zero blocks using blk_co_pwrite_zeroes rather than
writing them out as any other arbitrary data.
Signed-off-by: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20241026163010.2865002-5-libvirt-e6954efa@volkihar.be>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/commit.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 0fbf684728..4938714e0f 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -149,19 +149,39 @@ static int commit_iteration(CommitBlockJob *s, int64_t offset,
}
if (ret & BDRV_BLOCK_ALLOCATED) {
- assert(bytes < SIZE_MAX);
+ if (ret & BDRV_BLOCK_ZERO) {
+ /*
+ * If the top (sub)clusters are smaller than the base
+ * (sub)clusters, this will not unmap unless the underlying device
+ * does some tracking of these requests. Ideally, we would find
+ * the maximal extent of the zero clusters.
+ */
+ ret = blk_co_pwrite_zeroes(s->base, offset, bytes,
+ BDRV_REQ_MAY_UNMAP);
+ if (ret < 0) {
+ error_in_source = false;
+ goto fail;
+ }
+ } else {
+ assert(bytes < SIZE_MAX);
- ret = blk_co_pread(s->top, offset, bytes, buf, 0);
- if (ret < 0) {
- goto fail;
- }
+ ret = blk_co_pread(s->top, offset, bytes, buf, 0);
+ if (ret < 0) {
+ goto fail;
+ }
- ret = blk_co_pwrite(s->base, offset, bytes, buf, 0);
- if (ret < 0) {
- error_in_source = false;
- goto fail;
+ ret = blk_co_pwrite(s->base, offset, bytes, buf, 0);
+ if (ret < 0) {
+ error_in_source = false;
+ goto fail;
+ }
}
+ /*
+ * Whether zeroes actually end up on disk depends on the details of
+ * the underlying driver. Therefore, this might rate limit more than
+ * is necessary.
+ */
block_job_ratelimit_processed_bytes(&s->common, bytes);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 4/5] block: Remove unused aio_task_pool_empty
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2025-04-29 11:33 ` [PULL 4/8] block: allow commit to unmap zero blocks Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 5/8] block: add test non-active commit with zeroed data Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Dr. David Alan Gilbert, Vladimir Sementsov-Ogievskiy
From: "Dr. David Alan Gilbert" <dave@treblig.org>
aio_task_pool_empty has been unused since it was added in
6e9b225f73 ("block: introduce aio task pool")
Remove it.
Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
Message-Id: <20240917002007.330689-1-dave@treblig.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/aio_task.c | 5 -----
include/block/aio_task.h | 2 --
2 files changed, 7 deletions(-)
diff --git a/block/aio_task.c b/block/aio_task.c
index 9bd17ea2c1..bb5c05f455 100644
--- a/block/aio_task.c
+++ b/block/aio_task.c
@@ -119,8 +119,3 @@ int aio_task_pool_status(AioTaskPool *pool)
return pool->status;
}
-
-bool aio_task_pool_empty(AioTaskPool *pool)
-{
- return pool->busy_tasks == 0;
-}
diff --git a/include/block/aio_task.h b/include/block/aio_task.h
index 18a9c41f4e..c81d637617 100644
--- a/include/block/aio_task.h
+++ b/include/block/aio_task.h
@@ -40,8 +40,6 @@ void aio_task_pool_free(AioTaskPool *);
/* error code of failed task or 0 if all is OK */
int aio_task_pool_status(AioTaskPool *pool);
-bool aio_task_pool_empty(AioTaskPool *pool);
-
/* User provides filled @task, however task->pool will be set automatically */
void coroutine_fn aio_task_pool_start_task(AioTaskPool *pool, AioTask *task);
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 5/8] block: add test non-active commit with zeroed data
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2025-04-29 11:33 ` [PULL 4/5] block: Remove unused aio_task_pool_empty Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 5/5] util/co-shared-resource: Remove unused co_try_get_from_shres Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Vincent Vanlaer, Vladimir Sementsov-Ogievskiy
From: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Signed-off-by: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20241026163010.2865002-6-libvirt-e6954efa@volkihar.be>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
tests/qemu-iotests/tests/commit-zero-blocks | 96 +++++++++++++++++++
.../qemu-iotests/tests/commit-zero-blocks.out | 54 +++++++++++
2 files changed, 150 insertions(+)
create mode 100755 tests/qemu-iotests/tests/commit-zero-blocks
create mode 100644 tests/qemu-iotests/tests/commit-zero-blocks.out
diff --git a/tests/qemu-iotests/tests/commit-zero-blocks b/tests/qemu-iotests/tests/commit-zero-blocks
new file mode 100755
index 0000000000..de00273e72
--- /dev/null
+++ b/tests/qemu-iotests/tests/commit-zero-blocks
@@ -0,0 +1,96 @@
+#!/usr/bin/env bash
+# group: rw quick
+#
+# Test for commit of discarded blocks
+#
+# This tests committing a live snapshot where some of the blocks that
+# are present in the base image are discarded in the intermediate image.
+# This intends to check that these blocks are also discarded in the base
+# image after the commit.
+#
+# Copyright (C) 2024 Vincent Vanlaer.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# creator
+owner=libvirt-e6954efa@volkihar.be
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_qemu
+ _rm_test_img "${TEST_IMG}.base"
+ _rm_test_img "${TEST_IMG}.mid"
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+cd ..
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto file
+
+size="1M"
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+TEST_IMG="$TEST_IMG.mid" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT $size
+_make_test_img -b "${TEST_IMG}.mid" -F $IMGFMT $size
+
+$QEMU_IO -c "write -P 0x01 64k 128k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "discard 64k 64k" "$TEST_IMG.mid" | _filter_qemu_io
+
+echo
+echo "=== Base image info before commit ==="
+TEST_IMG="${TEST_IMG}.base" _img_info | _filter_img_info
+$QEMU_IMG map --output=json "$TEST_IMG.base" | _filter_qemu_img_map
+
+echo
+echo "=== Middle image info before commit ==="
+TEST_IMG="${TEST_IMG}.mid" _img_info | _filter_img_info
+$QEMU_IMG map --output=json "$TEST_IMG.mid" | _filter_qemu_img_map
+
+echo
+echo === Running QEMU Live Commit Test ===
+echo
+
+qemu_comm_method="qmp"
+_launch_qemu -drive file="${TEST_IMG}",if=virtio,id=test
+h=$QEMU_HANDLE
+
+_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return"
+
+_send_qemu_cmd $h "{ 'execute': 'block-commit',
+ 'arguments': { 'device': 'test',
+ 'top': '"${TEST_IMG}.mid"',
+ 'base': '"${TEST_IMG}.base"'} }" '"status": "null"'
+
+_cleanup_qemu
+
+echo
+echo "=== Base image info after commit ==="
+TEST_IMG="${TEST_IMG}.base" _img_info | _filter_img_info
+$QEMU_IMG map --output=json "$TEST_IMG.base" | _filter_qemu_img_map
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/tests/commit-zero-blocks.out b/tests/qemu-iotests/tests/commit-zero-blocks.out
new file mode 100644
index 0000000000..85bdc46aaf
--- /dev/null
+++ b/tests/qemu-iotests/tests/commit-zero-blocks.out
@@ -0,0 +1,54 @@
+QA output created by commit-zero-blocks
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/t.IMGFMT.mid', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.mid backing_fmt=IMGFMT
+wrote 131072/131072 bytes at offset 65536
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Base image info before commit ===
+image: TEST_DIR/t.IMGFMT.base
+file format: IMGFMT
+virtual size: 1 MiB (1048576 bytes)
+[{ "start": 0, "length": 65536, "depth": 0, "present": false, "zero": true, "data": false, "compressed": false},
+{ "start": 65536, "length": 131072, "depth": 0, "present": true, "zero": false, "data": true, "compressed": false, "offset": OFFSET},
+{ "start": 196608, "length": 851968, "depth": 0, "present": false, "zero": true, "data": false, "compressed": false}]
+
+=== Middle image info before commit ===
+image: TEST_DIR/t.IMGFMT.mid
+file format: IMGFMT
+virtual size: 1 MiB (1048576 bytes)
+backing file: TEST_DIR/t.IMGFMT.base
+backing file format: IMGFMT
+[{ "start": 0, "length": 65536, "depth": 1, "present": false, "zero": true, "data": false, "compressed": false},
+{ "start": 65536, "length": 65536, "depth": 0, "present": true, "zero": true, "data": false, "compressed": false},
+{ "start": 131072, "length": 65536, "depth": 1, "present": true, "zero": false, "data": true, "compressed": false, "offset": OFFSET},
+{ "start": 196608, "length": 851968, "depth": 1, "present": false, "zero": true, "data": false, "compressed": false}]
+
+=== Running QEMU Live Commit Test ===
+
+{ 'execute': 'qmp_capabilities' }
+{"return": {}}
+{ 'execute': 'block-commit',
+ 'arguments': { 'device': 'test',
+ 'top': 'TEST_DIR/t.IMGFMT.mid',
+ 'base': 'TEST_DIR/t.IMGFMT.base'} }
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "test"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "test"}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "test"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "test"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "test", "len": 1048576, "offset": 1048576, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "test"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "test"}}
+
+=== Base image info after commit ===
+image: TEST_DIR/t.IMGFMT.base
+file format: IMGFMT
+virtual size: 1 MiB (1048576 bytes)
+[{ "start": 0, "length": 65536, "depth": 0, "present": false, "zero": true, "data": false, "compressed": false},
+{ "start": 65536, "length": 65536, "depth": 0, "present": true, "zero": true, "data": false, "compressed": false},
+{ "start": 131072, "length": 65536, "depth": 0, "present": true, "zero": false, "data": true, "compressed": false, "offset": OFFSET},
+{ "start": 196608, "length": 851968, "depth": 0, "present": false, "zero": true, "data": false, "compressed": false}]
+*** done
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 5/5] util/co-shared-resource: Remove unused co_try_get_from_shres
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2025-04-29 11:33 ` [PULL 5/8] block: add test non-active commit with zeroed data Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 6/8] qapi: synchronize jobs and block-jobs documentation Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Dr. David Alan Gilbert, Vladimir Sementsov-Ogievskiy
From: "Dr. David Alan Gilbert" <dave@treblig.org>
co_try_get_from_shres hasn't been used since it was added in
55fa54a789 ("co-shared-resource: protect with a mutex")
(Everyone uses the _locked version)
Remove it.
Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
Message-Id: <20240918124220.27871-1-dave@treblig.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
include/qemu/co-shared-resource.h | 7 -------
util/qemu-co-shared-resource.c | 6 ------
2 files changed, 13 deletions(-)
diff --git a/include/qemu/co-shared-resource.h b/include/qemu/co-shared-resource.h
index 78ca5850f8..41be1a8131 100644
--- a/include/qemu/co-shared-resource.h
+++ b/include/qemu/co-shared-resource.h
@@ -44,13 +44,6 @@ SharedResource *shres_create(uint64_t total);
*/
void shres_destroy(SharedResource *s);
-/*
- * Try to allocate an amount of @n. Return true on success, and false
- * if there is too little left of the collective resource to fulfill
- * the request.
- */
-bool co_try_get_from_shres(SharedResource *s, uint64_t n);
-
/*
* Allocate an amount of @n, and, if necessary, yield until
* that becomes possible.
diff --git a/util/qemu-co-shared-resource.c b/util/qemu-co-shared-resource.c
index a66cc07e75..752eb5a1c5 100644
--- a/util/qemu-co-shared-resource.c
+++ b/util/qemu-co-shared-resource.c
@@ -66,12 +66,6 @@ static bool co_try_get_from_shres_locked(SharedResource *s, uint64_t n)
return false;
}
-bool co_try_get_from_shres(SharedResource *s, uint64_t n)
-{
- QEMU_LOCK_GUARD(&s->lock);
- return co_try_get_from_shres_locked(s, n);
-}
-
void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n)
{
assert(n <= s->total);
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 6/8] qapi: synchronize jobs and block-jobs documentation
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2025-04-29 11:33 ` [PULL 5/5] util/co-shared-resource: Remove unused co_try_get_from_shres Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 7/8] qapi/block-core: deprecate some block-job- APIs Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Vladimir Sementsov-Ogievskiy, Markus Armbruster
Actualize documentation and synchronize it for commands which actually
call the same functions internally.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20250409084232.28201-2-vsementsov@yandex-team.ru>
---
qapi/block-core.json | 61 ++++++++++++++++++++++++++------------------
qapi/job.json | 30 ++++++++++++++++++++--
2 files changed, 64 insertions(+), 27 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b1937780e1..6beab0dc12 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2956,13 +2956,14 @@
#
# Pause an active background block operation.
#
-# This command returns immediately after marking the active background
-# block operation for pausing. It is an error to call this command if
-# no operation is in progress or if the job is already paused.
+# This command returns immediately after marking the active job for
+# pausing. Pausing an already paused job is an error.
+#
+# The job will pause as soon as possible, which means transitioning
+# into the PAUSED state if it was RUNNING, or into STANDBY if it was
+# READY. The corresponding JOB_STATUS_CHANGE event will be emitted.
#
-# The operation will pause as soon as possible. No event is emitted
-# when the operation is actually paused. Cancelling a paused job
-# automatically resumes it.
+# Cancelling a paused job automatically resumes it.
#
# @device: The job identifier. This used to be a device name (hence
# the name of the parameter), but since QEMU 2.7 it can have other
@@ -2982,9 +2983,8 @@
#
# Resume an active background block operation.
#
-# This command returns immediately after resuming a paused background
-# block operation. It is an error to call this command if no
-# operation is in progress or if the job is not paused.
+# This command returns immediately after resuming a paused job.
+# Resuming an already running job is an error.
#
# This command also clears the error status of the job.
#
@@ -3004,10 +3004,15 @@
##
# @block-job-complete:
#
-# Manually trigger completion of an active background block operation.
-# This is supported for drive mirroring, where it also switches the
-# device to write to the target path only. The ability to complete is
-# signaled with a BLOCK_JOB_READY event.
+# Manually trigger completion of an active job in the READY or STANDBY
+# state. Completing the job in any other state is an error.
+#
+# This is supported only for drive mirroring, where it also switches
+# the device to write to the target path only. Note that drive
+# mirroring includes drive-mirror, blockdev-mirror and block-commit
+# job (only in case of "active commit", when the node being commited
+# is used by the guest). The ability to complete is signaled with a
+# BLOCK_JOB_READY event.
#
# This command completes an active background block operation
# synchronously. The ordering of this command's return with the
@@ -3017,8 +3022,6 @@
# rerror/werror arguments that were specified when starting the
# operation.
#
-# A cancelled or paused job cannot be completed.
-#
# @device: The job identifier. This used to be a device name (hence
# the name of the parameter), but since QEMU 2.7 it can have other
# values.
@@ -3035,10 +3038,13 @@
##
# @block-job-dismiss:
#
-# For jobs that have already concluded, remove them from the
-# block-job-query list. This command only needs to be run for jobs
-# which were started with QEMU 2.12+ job lifetime management
-# semantics.
+# Deletes a job that is in the CONCLUDED state. This command only
+# needs to be run explicitly for jobs that don't have automatic
+# dismiss enabled. In turn, automatic dismiss may be enabled only
+# for jobs that have @auto-dismiss option, which are drive-backup,
+# blockdev-backup, drive-mirror, blockdev-mirror, block-commit and
+# block-stream. @auto-dismiss is enabled by default for these
+# jobs.
#
# This command will refuse to operate on any job that has not yet
# reached its terminal state, JOB_STATUS_CONCLUDED. For jobs that
@@ -3055,12 +3061,17 @@
##
# @block-job-finalize:
#
-# Once a job that has manual=true reaches the pending state, it can be
-# instructed to finalize any graph changes and do any necessary
-# cleanup via this command. For jobs in a transaction, instructing
-# one job to finalize will force ALL jobs in the transaction to
-# finalize, so it is only necessary to instruct a single member job to
-# finalize.
+# Instructs all jobs in a transaction (or a single job if it is not
+# part of any transaction) to finalize any graph changes and do any
+# necessary cleanup. This command requires that all involved jobs are
+# in the PENDING state.
+#
+# For jobs in a transaction, instructing one job to finalize will
+# force ALL jobs in the transaction to finalize, so it is only
+# necessary to instruct a single member job to finalize.
+#
+# The command is applicable only to jobs which have @auto-finalize option
+# and only when this option is set to false.
#
# @id: The job identifier.
#
diff --git a/qapi/job.json b/qapi/job.json
index cfc3beedd2..b03f80bc84 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -156,6 +156,9 @@
# This command returns immediately after resuming a paused job.
# Resuming an already running job is an error.
#
+# This command also clears the error status for block-jobs (stream,
+# commit, mirror, backup).
+#
# @id: The job identifier.
#
# Since: 3.0
@@ -184,7 +187,23 @@
##
# @job-complete:
#
-# Manually trigger completion of an active job in the READY state.
+# Manually trigger completion of an active job in the READY or STANDBY
+# state. Completing the job in any other state is an error.
+#
+# This is supported only for drive mirroring, where it also switches
+# the device to write to the target path only. Note that drive
+# mirroring includes drive-mirror, blockdev-mirror and block-commit
+# job (only in case of "active commit", when the node being commited
+# is used by the guest). The ability to complete is signaled with a
+# BLOCK_JOB_READY event.
+#
+# This command completes an active background block operation
+# synchronously. The ordering of this command's return with the
+# BLOCK_JOB_COMPLETED event is not defined. Note that if an I/O error
+# occurs during the processing of this command: 1) the command itself
+# will fail; 2) the error will be processed according to the
+# rerror/werror arguments that were specified when starting the
+# operation.
#
# @id: The job identifier.
#
@@ -197,7 +216,11 @@
#
# Deletes a job that is in the CONCLUDED state. This command only
# needs to be run explicitly for jobs that don't have automatic
-# dismiss enabled.
+# dismiss enabled. In turn, automatic dismiss may be enabled only
+# for jobs that have @auto-dismiss option, which are drive-backup,
+# blockdev-backup, drive-mirror, blockdev-mirror, block-commit and
+# block-stream. @auto-dismiss is enabled by default for these
+# jobs.
#
# This command will refuse to operate on any job that has not yet
# reached its terminal state, JOB_STATUS_CONCLUDED. For jobs that
@@ -222,6 +245,9 @@
# force ALL jobs in the transaction to finalize, so it is only
# necessary to instruct a single member job to finalize.
#
+# The command is applicable only to jobs which have @auto-finalize option
+# and only when this option is set to false.
+#
# @id: The identifier of any job in the transaction, or of a job that
# is not part of any transaction.
#
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 7/8] qapi/block-core: deprecate some block-job- APIs
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2025-04-29 11:33 ` [PULL 6/8] qapi: synchronize jobs and block-jobs documentation Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:33 ` [PULL 8/8] blockdev-backup: Add error handling option for copy-before-write jobs Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Vladimir Sementsov-Ogievskiy, Peter Krempa,
Eric Blake, Markus Armbruster
For change, pause, resume, complete, dismiss and finalize actions
corresponding job- and block-job commands are almost equal. The
difference is in find_block_job_locked() vs find_job_locked()
functions. What's different?
1. find_block_job_locked() checks whether the found job is a block-job.
This is OK when moving to more generic API, no needs to document this
change.
2. find_block_job_locked() reports DeviceNotActive on failure, when
find_job_locked() reports GenericError. So, let's document this
difference in deprecated.txt. Still, for dismiss and finalize errors
are not documented at all, so be silent in deprecated.txt as well.
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20250409084232.28201-3-vsementsov@yandex-team.ru>
---
docs/about/deprecated.rst | 31 +++++++++++++++++++++++++++++++
qapi/block-core.json | 30 ++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 05381441a9..b0d3ae6b32 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -148,6 +148,37 @@ options are removed in favor of using explicit ``blockdev-create`` and
``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for
details.
+``block-job-pause`` (since 10.1)
+''''''''''''''''''''''''''''''''
+
+Use ``job-pause`` instead. The only difference is that ``job-pause``
+always reports GenericError on failure when ``block-job-pause`` reports
+DeviceNotActive when block-job is not found.
+
+``block-job-resume`` (since 10.1)
+'''''''''''''''''''''''''''''''''
+
+Use ``job-resume`` instead. The only difference is that ``job-resume``
+always reports GenericError on failure when ``block-job-resume`` reports
+DeviceNotActive when block-job is not found.
+
+``block-job-complete`` (since 10.1)
+'''''''''''''''''''''''''''''''''''
+
+Use ``job-complete`` instead. The only difference is that ``job-complete``
+always reports GenericError on failure when ``block-job-complete`` reports
+DeviceNotActive when block-job is not found.
+
+``block-job-dismiss`` (since 10.1)
+''''''''''''''''''''''''''''''''''
+
+Use ``job-dismiss`` instead.
+
+``block-job-finalize`` (since 10.1)
+'''''''''''''''''''''''''''''''''''
+
+Use ``job-finalize`` instead.
+
``query-migrationthreads`` (since 9.2)
''''''''''''''''''''''''''''''''''''''
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6beab0dc12..22061227ca 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2969,6 +2969,11 @@
# the name of the parameter), but since QEMU 2.7 it can have other
# values.
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use @job-pause
+# instead.
+#
# Errors:
# - If no background operation is active on this device,
# DeviceNotActive
@@ -2976,6 +2981,7 @@
# Since: 1.3
##
{ 'command': 'block-job-pause', 'data': { 'device': 'str' },
+ 'features': ['deprecated'],
'allow-preconfig': true }
##
@@ -2992,6 +2998,11 @@
# the name of the parameter), but since QEMU 2.7 it can have other
# values.
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use @job-resume
+# instead.
+#
# Errors:
# - If no background operation is active on this device,
# DeviceNotActive
@@ -2999,6 +3010,7 @@
# Since: 1.3
##
{ 'command': 'block-job-resume', 'data': { 'device': 'str' },
+ 'features': ['deprecated'],
'allow-preconfig': true }
##
@@ -3026,6 +3038,11 @@
# the name of the parameter), but since QEMU 2.7 it can have other
# values.
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use @job-complete
+# instead.
+#
# Errors:
# - If no background operation is active on this device,
# DeviceNotActive
@@ -3033,6 +3050,7 @@
# Since: 1.3
##
{ 'command': 'block-job-complete', 'data': { 'device': 'str' },
+ 'features': ['deprecated'],
'allow-preconfig': true }
##
@@ -3053,9 +3071,15 @@
#
# @id: The job identifier.
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use @job-dismiss
+# instead.
+#
# Since: 2.12
##
{ 'command': 'block-job-dismiss', 'data': { 'id': 'str' },
+ 'features': ['deprecated'],
'allow-preconfig': true }
##
@@ -3075,9 +3099,15 @@
#
# @id: The job identifier.
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use @job-finalize
+# instead.
+#
# Since: 2.12
##
{ 'command': 'block-job-finalize', 'data': { 'id': 'str' },
+ 'features': ['deprecated'],
'allow-preconfig': true }
##
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 8/8] blockdev-backup: Add error handling option for copy-before-write jobs
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (11 preceding siblings ...)
2025-04-29 11:33 ` [PULL 7/8] qapi/block-core: deprecate some block-job- APIs Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:33 ` Vladimir Sementsov-Ogievskiy
2025-04-29 11:38 ` [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
2025-11-14 23:00 ` Michael Tokarev
14 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:33 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Raman Dzehtsiar, Raman Dzehtsiar, Markus Armbruster,
Vladimir Sementsov-Ogievskiy
From: Raman Dzehtsiar <raman.dzehtsiar@gmail.com>
This patch extends the blockdev-backup QMP command to allow users to specify
how to behave when IO errors occur during copy-before-write operations.
Previously, the behavior was fixed and could not be controlled by the user.
The new 'on-cbw-error' option can be set to one of two values:
- 'break-guest-write': Forwards the IO error to the guest and triggers
the on-source-error policy. This preserves snapshot integrity at the
expense of guest IO operations.
- 'break-snapshot': Allows the guest OS to continue running normally,
but invalidates the snapshot and aborts related jobs. This prioritizes
guest operation over backup consistency.
This enhancement provides more flexibility for backup operations in different
environments where requirements for guest availability versus backup
consistency may vary.
The default behavior remains unchanged to maintain backward compatibility.
Signed-off-by: Raman Dzehtsiar <Raman.Dzehtsiar@gmail.com>
Message-ID: <20250414090025.828660-1-Raman.Dzehtsiar@gmail.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/backup.c | 3 +-
block/copy-before-write.c | 2 +
block/copy-before-write.h | 1 +
block/replication.c | 4 +-
blockdev.c | 6 ++
include/block/block_int-global-state.h | 2 +
qapi/block-core.json | 4 +
tests/qemu-iotests/tests/copy-before-write | 90 +++++++++++++++++++
.../qemu-iotests/tests/copy-before-write.out | 4 +-
9 files changed, 112 insertions(+), 4 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 79652bf57b..0151e84395 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -361,6 +361,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BackupPerf *perf,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
+ OnCbwError on_cbw_error,
int creation_flags,
BlockCompletionFunc *cb, void *opaque,
JobTxn *txn, Error **errp)
@@ -458,7 +459,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
}
cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
- perf->min_cluster_size, &bcs, errp);
+ perf->min_cluster_size, &bcs, on_cbw_error, errp);
if (!cbw) {
goto error;
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index fd470f5f92..00af0b18ac 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -551,6 +551,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
bool discard_source,
uint64_t min_cluster_size,
BlockCopyState **bcs,
+ OnCbwError on_cbw_error,
Error **errp)
{
BDRVCopyBeforeWriteState *state;
@@ -568,6 +569,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
}
qdict_put_str(opts, "file", bdrv_get_node_name(source));
qdict_put_str(opts, "target", bdrv_get_node_name(target));
+ qdict_put_str(opts, "on-cbw-error", OnCbwError_str(on_cbw_error));
if (min_cluster_size > INT64_MAX) {
error_setg(errp, "min-cluster-size too large: %" PRIu64 " > %" PRIi64,
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 2a5d4ba693..eb93364e85 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -42,6 +42,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
bool discard_source,
uint64_t min_cluster_size,
BlockCopyState **bcs,
+ OnCbwError on_cbw_error,
Error **errp);
void bdrv_cbw_drop(BlockDriverState *bs);
diff --git a/block/replication.c b/block/replication.c
index d6625c51fe..07f274de9e 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -583,7 +583,9 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
0, MIRROR_SYNC_MODE_NONE, NULL, 0, false, false,
NULL, &perf,
BLOCKDEV_ON_ERROR_REPORT,
- BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
+ BLOCKDEV_ON_ERROR_REPORT,
+ ON_CBW_ERROR_BREAK_GUEST_WRITE,
+ JOB_INTERNAL,
backup_job_completed, bs, NULL, &local_err);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/blockdev.c b/blockdev.c
index 1d1f27cfff..818ec42511 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2641,6 +2641,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
BdrvDirtyBitmap *bmap = NULL;
BackupPerf perf = { .max_workers = 64 };
int job_flags = JOB_DEFAULT;
+ OnCbwError on_cbw_error = ON_CBW_ERROR_BREAK_GUEST_WRITE;
if (!backup->has_speed) {
backup->speed = 0;
@@ -2745,6 +2746,10 @@ static BlockJob *do_backup_common(BackupCommon *backup,
job_flags |= JOB_MANUAL_DISMISS;
}
+ if (backup->has_on_cbw_error) {
+ on_cbw_error = backup->on_cbw_error;
+ }
+
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
backup->sync, bmap, backup->bitmap_mode,
backup->compress, backup->discard_source,
@@ -2752,6 +2757,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
&perf,
backup->on_source_error,
backup->on_target_error,
+ on_cbw_error,
job_flags, NULL, NULL, txn, errp);
return job;
}
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index eb2d92a226..0d93783763 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -179,6 +179,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
* all ".has_*" fields are ignored.
* @on_source_error: The action to take upon error reading from the source.
* @on_target_error: The action to take upon error writing to the target.
+ * @on_cbw_error: The action to take upon error in copy-before-write operations.
* @creation_flags: Flags that control the behavior of the Job lifetime.
* See @BlockJobCreateFlags
* @cb: Completion function for the job.
@@ -198,6 +199,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BackupPerf *perf,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
+ OnCbwError on_cbw_error,
int creation_flags,
BlockCompletionFunc *cb, void *opaque,
JobTxn *txn, Error **errp);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 22061227ca..91c70e24a7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1602,6 +1602,9 @@
# default 'report' (no limitations, since this applies to a
# different block device than @device).
#
+# @on-cbw-error: policy defining behavior on I/O errors in
+# copy-before-write jobs; defaults to break-guest-write. (Since 10.1)
+#
# @auto-finalize: When false, this job will wait in a PENDING state
# after it has finished its work, waiting for @block-job-finalize
# before making any block graph changes. When true, this job will
@@ -1641,6 +1644,7 @@
'*compress': 'bool',
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError',
+ '*on-cbw-error': 'OnCbwError',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool',
'*filter-node-name': 'str',
'*discard-source': 'bool',
diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write
index 498c558008..23d70c7fe7 100755
--- a/tests/qemu-iotests/tests/copy-before-write
+++ b/tests/qemu-iotests/tests/copy-before-write
@@ -99,6 +99,66 @@ class TestCbwError(iotests.QMPTestCase):
log = iotests.filter_qemu_io(log)
return log
+ def do_cbw_error_via_blockdev_backup(self, on_cbw_error=None):
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'source',
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': source_img
+ }
+ })
+
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'target',
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'blkdebug',
+ 'image': {
+ 'driver': 'file',
+ 'filename': temp_img
+ },
+ 'inject-error': [
+ {
+ 'event': 'write_aio',
+ 'errno': 5,
+ 'immediately': False,
+ 'once': True
+ }
+ ]
+ }
+ })
+
+ blockdev_backup_options = {
+ 'device': 'source',
+ 'target': 'target',
+ 'sync': 'none',
+ 'job-id': 'job-id',
+ 'filter-node-name': 'cbw'
+ }
+
+ if on_cbw_error:
+ blockdev_backup_options['on-cbw-error'] = on_cbw_error
+
+ self.vm.cmd('blockdev-backup', blockdev_backup_options)
+
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'access',
+ 'driver': 'snapshot-access',
+ 'file': 'cbw'
+ })
+
+ result = self.vm.qmp('human-monitor-command', command_line='qemu-io cbw "write 0 1M"')
+ self.assert_qmp(result, 'return', '')
+
+ result = self.vm.qmp('human-monitor-command', command_line='qemu-io access "read 0 1M"')
+ self.assert_qmp(result, 'return', '')
+
+ self.vm.shutdown()
+ log = self.vm.get_log()
+ log = iotests.filter_qemu_io(log)
+ return log
+
def test_break_snapshot_on_cbw_error(self):
"""break-snapshot behavior:
Guest write succeed, but further snapshot-read fails, as snapshot is
@@ -123,6 +183,36 @@ read failed: Permission denied
write failed: Input/output error
read 1048576/1048576 bytes at offset 0
1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+""")
+
+ def test_break_snapshot_policy_forwarding(self):
+ """Ensure CBW filter accepts break-snapshot policy specified in blockdev-backup QMP command.
+ """
+ log = self.do_cbw_error_via_blockdev_backup('break-snapshot')
+ self.assertEqual(log, """\
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read failed: Permission denied
+""")
+
+ def test_break_guest_write_policy_forwarding(self):
+ """Ensure CBW filter accepts break-guest-write policy specified in blockdev-backup QMP command.
+ """
+ log = self.do_cbw_error_via_blockdev_backup('break-guest-write')
+ self.assertEqual(log, """\
+write failed: Input/output error
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+""")
+
+ def test_default_on_cbw_error_policy_forwarding(self):
+ """Ensure break-guest-write policy is used by default when on-cbw-error is not explicitly specified.
+ """
+ log = self.do_cbw_error_via_blockdev_backup()
+ self.assertEqual(log, """\
+write failed: Input/output error
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
""")
def do_cbw_timeout(self, on_cbw_error):
diff --git a/tests/qemu-iotests/tests/copy-before-write.out b/tests/qemu-iotests/tests/copy-before-write.out
index 89968f35d7..2f7d3902f2 100644
--- a/tests/qemu-iotests/tests/copy-before-write.out
+++ b/tests/qemu-iotests/tests/copy-before-write.out
@@ -1,5 +1,5 @@
-....
+.......
----------------------------------------------------------------------
-Ran 4 tests
+Ran 7 tests
OK
--
2.48.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PULL 0/8] block-job patches 2025-04-29
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (12 preceding siblings ...)
2025-04-29 11:33 ` [PULL 8/8] blockdev-backup: Add error handling option for copy-before-write jobs Vladimir Sementsov-Ogievskiy
@ 2025-04-29 11:38 ` Vladimir Sementsov-Ogievskiy
2025-04-30 17:36 ` Stefan Hajnoczi
2025-11-14 23:00 ` Michael Tokarev
14 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-04-29 11:38 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel
Oops, accidentally extra old patches were sent, sorry for the noice. Please ignore patches */5, they are already in master.
The cover-letter message and the tag pull-block-jobs-2025-04-29 are correct.
On 29.04.25 14:33, Vladimir Sementsov-Ogievskiy wrote:
> The following changes since commit 73d29ea2417b58ca55fba1aa468ba38e3607b583:
>
> Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging (2025-04-27 12:47:23 -0400)
>
> are available in the Git repository at:
>
> https://gitlab.com/vsementsov/qemu.git tags/pull-block-jobs-2025-04-29
>
> for you to fetch changes up to 300ef7b9d16067f3d8d85f5c36006c6d9637e07f:
>
> blockdev-backup: Add error handling option for copy-before-write jobs (2025-04-29 12:55:28 +0300)
>
> ----------------------------------------------------------------
> block-job patches
>
> - deprecate some old block-job- APIs
> - on-cbw-error option for backup
> - more efficient zero handling in block commit
>
> ----------------------------------------------------------------
> Raman Dzehtsiar (1):
> blockdev-backup: Add error handling option for copy-before-write jobs
>
> Vincent Vanlaer (5):
> block: get type of block allocation in commit_run
> block: move commit_run loop to separate function
> block: refactor error handling of commit_iteration
> block: allow commit to unmap zero blocks
> block: add test non-active commit with zeroed data
>
> Vladimir Sementsov-Ogievskiy (2):
> qapi: synchronize jobs and block-jobs documentation
> qapi/block-core: deprecate some block-job- APIs
>
> block/backup.c | 3 +-
> block/commit.c | 117 +++++++++++++++++-------
> block/copy-before-write.c | 2 +
> block/copy-before-write.h | 1 +
> block/replication.c | 4 +-
> blockdev.c | 6 ++
> docs/about/deprecated.rst | 31 +++++++
> include/block/block_int-global-state.h | 2 +
> qapi/block-core.json | 95 ++++++++++++++-----
> qapi/job.json | 30 +++++-
> tests/qemu-iotests/tests/commit-zero-blocks | 96 +++++++++++++++++++
> tests/qemu-iotests/tests/commit-zero-blocks.out | 54 +++++++++++
> tests/qemu-iotests/tests/copy-before-write | 90 ++++++++++++++++++
> tests/qemu-iotests/tests/copy-before-write.out | 4 +-
> 14 files changed, 470 insertions(+), 65 deletions(-)
> create mode 100755 tests/qemu-iotests/tests/commit-zero-blocks
> create mode 100644 tests/qemu-iotests/tests/commit-zero-blocks.out
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PULL 0/8] block-job patches 2025-04-29
2025-04-29 11:33 [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
` (13 preceding siblings ...)
2025-04-29 11:38 ` [PULL 0/8] block-job patches 2025-04-29 Vladimir Sementsov-Ogievskiy
@ 2025-11-14 23:00 ` Michael Tokarev
2025-11-21 9:39 ` Vladimir Sementsov-Ogievskiy
14 siblings, 1 reply; 18+ messages in thread
From: Michael Tokarev @ 2025-11-14 23:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, Vincent Vanlaer
Cc: qemu-devel, qemu-stable
On 4/29/25 14:33, Vladimir Sementsov-Ogievskiy wrote:
...> block-job patches
>
> - deprecate some old block-job- APIs
> - on-cbw-error option for backup
> - more efficient zero handling in block commit
>
> ----------------------------------------------------------------
> Raman Dzehtsiar (1):
> blockdev-backup: Add error handling option for copy-before-write jobs
>
> Vincent Vanlaer (5):
> block: get type of block allocation in commit_run
> block: move commit_run loop to separate function
> block: refactor error handling of commit_iteration
> block: allow commit to unmap zero blocks
> block: add test non-active commit with zeroed data
There's a bug report in debian, for 10.0.x series of qemu, here:
https://bugs.debian.org/1120389 . This looks like it's about
this very series, in particular, commit v10.0.0-369-g6f3199f996
"block: allow commit to unmap zero blocks". Does it look similar? :)
(I haven't yet verified if this commit actually fixed the issue
reported in the debian bug).
But if it it, it looks like it should be a good idea to pick this
series up for 10.0.x stable series too, what do you think?
(it all applies cleanly and seems to work too).
Thanks,
/mjt
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PULL 0/8] block-job patches 2025-04-29
2025-11-14 23:00 ` Michael Tokarev
@ 2025-11-21 9:39 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-11-21 9:39 UTC (permalink / raw)
To: Michael Tokarev, qemu-block, Vincent Vanlaer; +Cc: qemu-devel, qemu-stable
On 15.11.25 02:00, Michael Tokarev wrote:
> On 4/29/25 14:33, Vladimir Sementsov-Ogievskiy wrote:
> ...> block-job patches
>>
>> - deprecate some old block-job- APIs
>> - on-cbw-error option for backup
>> - more efficient zero handling in block commit
>>
>> ----------------------------------------------------------------
>> Raman Dzehtsiar (1):
>> blockdev-backup: Add error handling option for copy-before-write jobs
>>
>> Vincent Vanlaer (5):
>> block: get type of block allocation in commit_run
>> block: move commit_run loop to separate function
>> block: refactor error handling of commit_iteration
>> block: allow commit to unmap zero blocks
>> block: add test non-active commit with zeroed data
>
> There's a bug report in debian, for 10.0.x series of qemu, here:
> https://bugs.debian.org/1120389 . This looks like it's about
> this very series, in particular, commit v10.0.0-369-g6f3199f996
> "block: allow commit to unmap zero blocks". Does it look similar? :)
> (I haven't yet verified if this commit actually fixed the issue
> reported in the debian bug).
>
> But if it it, it looks like it should be a good idea to pick this
> series up for 10.0.x stable series too, what do you think?
> (it all applies cleanly and seems to work too).
>
Agree, looks like the series may fix https://bugs.debian.org/1120389,
but of course, better to check it explicitly.
Picking it into 10.0.* is not bad I think.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread