* [PATCH v4 1/5] block/copy-before-write: fix permission
2024-03-13 15:28 [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
@ 2024-03-13 15:28 ` Vladimir Sementsov-Ogievskiy
2024-03-13 15:28 ` [PATCH v4 2/5] block/copy-before-write: support unligned snapshot-discard Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-03-13 15:28 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, eblake, xiechanglong.d, wencongyang2, hreitz,
kwolf, vsementsov, jsnow, f.ebner
In case when source node does not have any parents, the condition still
works as required: backup job do create the parent by
block_job_create -> block_job_add_bdrv -> bdrv_root_attach_child
Still, in this case checking @perm variable doesn't work, as backup job
creates the root blk with empty permissions (as it rely on CBW filter
to require correct permissions and don't want to create extra
conflicts).
So, we should not check @perm.
The hack may be dropped entirely when transactional insertion of
filter (when we don't try to recalculate permissions in intermediate
state, when filter does conflict with original parent of the source
node) merged (old big series
"[PATCH v5 00/45] Transactional block-graph modifying API"[1] and it's
current in-flight part is "[PATCH v8 0/7] blockdev-replace"[2])
[1] https://patchew.org/QEMU/20220330212902.590099-1-vsementsov@openvz.org/
[2] https://patchew.org/QEMU/20231017184444.932733-1-vsementsov@yandex-team.ru/
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
---
block/copy-before-write.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 8aba27a71d..3e3af30c08 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -364,9 +364,13 @@ cbw_child_perm(BlockDriverState *bs, BdrvChild *c, BdrvChildRole role,
perm, shared, nperm, nshared);
if (!QLIST_EMPTY(&bs->parents)) {
- if (perm & BLK_PERM_WRITE) {
- *nperm = *nperm | BLK_PERM_CONSISTENT_READ;
- }
+ /*
+ * Note, that source child may be shared with backup job. Backup job
+ * does create own blk parent on copy-before-write node, so this
+ * works even if source node does not have any parents before backup
+ * start
+ */
+ *nperm = *nperm | BLK_PERM_CONSISTENT_READ;
*nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 2/5] block/copy-before-write: support unligned snapshot-discard
2024-03-13 15:28 [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
2024-03-13 15:28 ` [PATCH v4 1/5] block/copy-before-write: fix permission Vladimir Sementsov-Ogievskiy
@ 2024-03-13 15:28 ` Vladimir Sementsov-Ogievskiy
2024-03-13 15:28 ` [PATCH v4 3/5] block/copy-before-write: create block_copy bitmap in filter node Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-03-13 15:28 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, eblake, xiechanglong.d, wencongyang2, hreitz,
kwolf, vsementsov, jsnow, f.ebner
First thing that crashes on unligned access here is
bdrv_reset_dirty_bitmap(). Correct way is to align-down the
snapshot-discard request.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
---
block/copy-before-write.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 3e3af30c08..6d89af0b29 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -325,14 +325,24 @@ static int coroutine_fn GRAPH_RDLOCK
cbw_co_pdiscard_snapshot(BlockDriverState *bs, int64_t offset, int64_t bytes)
{
BDRVCopyBeforeWriteState *s = bs->opaque;
+ uint32_t cluster_size = block_copy_cluster_size(s->bcs);
+ int64_t aligned_offset = QEMU_ALIGN_UP(offset, cluster_size);
+ int64_t aligned_end = QEMU_ALIGN_DOWN(offset + bytes, cluster_size);
+ int64_t aligned_bytes;
+
+ if (aligned_end <= aligned_offset) {
+ return 0;
+ }
+ aligned_bytes = aligned_end - aligned_offset;
WITH_QEMU_LOCK_GUARD(&s->lock) {
- bdrv_reset_dirty_bitmap(s->access_bitmap, offset, bytes);
+ bdrv_reset_dirty_bitmap(s->access_bitmap, aligned_offset,
+ aligned_bytes);
}
- block_copy_reset(s->bcs, offset, bytes);
+ block_copy_reset(s->bcs, aligned_offset, aligned_bytes);
- return bdrv_co_pdiscard(s->target, offset, bytes);
+ return bdrv_co_pdiscard(s->target, aligned_offset, aligned_bytes);
}
static void GRAPH_RDLOCK cbw_refresh_filename(BlockDriverState *bs)
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 3/5] block/copy-before-write: create block_copy bitmap in filter node
2024-03-13 15:28 [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
2024-03-13 15:28 ` [PATCH v4 1/5] block/copy-before-write: fix permission Vladimir Sementsov-Ogievskiy
2024-03-13 15:28 ` [PATCH v4 2/5] block/copy-before-write: support unligned snapshot-discard Vladimir Sementsov-Ogievskiy
@ 2024-03-13 15:28 ` Vladimir Sementsov-Ogievskiy
2024-03-13 15:28 ` [PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-03-13 15:28 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, eblake, xiechanglong.d, wencongyang2, hreitz,
kwolf, vsementsov, jsnow, f.ebner
Currently block_copy creates copy_bitmap in source node. But that is in
bad relation with .independent_close=true of copy-before-write filter:
source node may be detached and removed before .bdrv_close() handler
called, which should call block_copy_state_free(), which in turn should
remove copy_bitmap.
That's all not ideal: it would be better if internal bitmap of
block-copy object is not attached to any node. But that is not possible
now.
The simplest solution is just create copy_bitmap in filter node, where
anyway two other bitmaps are created.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
---
block/block-copy.c | 3 +-
block/copy-before-write.c | 2 +-
include/block/block-copy.h | 1 +
tests/qemu-iotests/257.out | 112 ++++++++++++++++++-------------------
4 files changed, 60 insertions(+), 58 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 9ee3dd7ef5..8fca2c3698 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -351,6 +351,7 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
}
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
+ BlockDriverState *copy_bitmap_bs,
const BdrvDirtyBitmap *bitmap,
Error **errp)
{
@@ -367,7 +368,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
return NULL;
}
- copy_bitmap = bdrv_create_dirty_bitmap(source->bs, cluster_size, NULL,
+ copy_bitmap = bdrv_create_dirty_bitmap(copy_bitmap_bs, cluster_size, NULL,
errp);
if (!copy_bitmap) {
return NULL;
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 6d89af0b29..ed2c228da7 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -468,7 +468,7 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
bs->file->bs->supported_zero_flags);
- s->bcs = block_copy_state_new(bs->file, s->target, bitmap, errp);
+ s->bcs = block_copy_state_new(bs->file, s->target, bs, bitmap, 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 0700953ab8..8b41643bfa 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -25,6 +25,7 @@ typedef struct BlockCopyState BlockCopyState;
typedef struct BlockCopyCallState BlockCopyCallState;
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
+ BlockDriverState *copy_bitmap_bs,
const BdrvDirtyBitmap *bitmap,
Error **errp);
diff --git a/tests/qemu-iotests/257.out b/tests/qemu-iotests/257.out
index aa76131ca9..c33dd7f3a9 100644
--- a/tests/qemu-iotests/257.out
+++ b/tests/qemu-iotests/257.out
@@ -120,16 +120,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -596,16 +596,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -865,16 +865,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -1341,16 +1341,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -1610,16 +1610,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -2086,16 +2086,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -2355,16 +2355,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -2831,16 +2831,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -3100,16 +3100,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -3576,16 +3576,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -3845,16 +3845,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -4321,16 +4321,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -4590,16 +4590,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
@@ -5066,16 +5066,16 @@ write -P0x67 0x3fe0000 0x20000
"granularity": 65536,
"persistent": false,
"recording": false
- }
- ],
- "drive0": [
+ },
{
"busy": false,
"count": 0,
"granularity": 65536,
"persistent": false,
"recording": false
- },
+ }
+ ],
+ "drive0": [
{
"busy": false,
"count": 458752,
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter
2024-03-13 15:28 [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2024-03-13 15:28 ` [PATCH v4 3/5] block/copy-before-write: create block_copy bitmap in filter node Vladimir Sementsov-Ogievskiy
@ 2024-03-13 15:28 ` Vladimir Sementsov-Ogievskiy
2024-03-13 16:08 ` Markus Armbruster
2024-03-13 15:28 ` [PATCH v4 5/5] iotests: add backup-discard-source Vladimir Sementsov-Ogievskiy
2024-04-05 13:46 ` [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
5 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-03-13 15:28 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, eblake, xiechanglong.d, wencongyang2, hreitz,
kwolf, vsementsov, jsnow, f.ebner
Add a parameter that enables discard-after-copy. That is mostly useful
in "push backup with fleecing" scheme, when source is snapshot-access
format driver node, based on copy-before-write filter snapshot-access
API:
[guest] [snapshot-access] ~~ blockdev-backup ~~> [backup target]
| |
| root | file
v v
[copy-before-write]
| |
| file | target
v v
[active disk] [temp.img]
In this case discard-after-copy does two things:
- discard data in temp.img to save disk space
- avoid further copy-before-write operation in discarded area
Note that we have to declare WRITE permission on source in
copy-before-write filter, for discard to work. Still we can't take it
unconditionally, as it will break normal backup from RO source. So, we
have to add a parameter and pass it thorough bdrv_open flags.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
---
block/backup.c | 5 +++--
block/block-copy.c | 9 +++++++++
block/copy-before-write.c | 15 +++++++++++++--
block/copy-before-write.h | 1 +
block/replication.c | 4 ++--
blockdev.c | 2 +-
include/block/block-common.h | 2 ++
include/block/block-copy.h | 1 +
include/block/block_int-global-state.h | 2 +-
qapi/block-core.json | 4 ++++
10 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index ec29d6b810..3dd2e229d2 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -356,7 +356,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BlockDriverState *target, int64_t speed,
MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
BitmapSyncMode bitmap_mode,
- bool compress,
+ bool compress, bool discard_source,
const char *filter_node_name,
BackupPerf *perf,
BlockdevOnError on_source_error,
@@ -457,7 +457,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
goto error;
}
- cbw = bdrv_cbw_append(bs, target, filter_node_name, &bcs, errp);
+ cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
+ &bcs, errp);
if (!cbw) {
goto error;
}
diff --git a/block/block-copy.c b/block/block-copy.c
index 8fca2c3698..7e3b378528 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -137,6 +137,7 @@ typedef struct BlockCopyState {
CoMutex lock;
int64_t in_flight_bytes;
BlockCopyMethod method;
+ bool discard_source;
BlockReqList reqs;
QLIST_HEAD(, BlockCopyCallState) calls;
/*
@@ -353,6 +354,7 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
BlockDriverState *copy_bitmap_bs,
const BdrvDirtyBitmap *bitmap,
+ bool discard_source,
Error **errp)
{
ERRP_GUARD();
@@ -418,6 +420,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
cluster_size),
};
+ s->discard_source = discard_source;
block_copy_set_copy_opts(s, false, false);
ratelimit_init(&s->rate_limit);
@@ -589,6 +592,12 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
co_put_to_shres(s->mem, t->req.bytes);
block_copy_task_end(t, ret);
+ if (s->discard_source && ret == 0) {
+ int64_t nbytes =
+ MIN(t->req.offset + t->req.bytes, s->len) - t->req.offset;
+ bdrv_co_pdiscard(s->source, t->req.offset, nbytes);
+ }
+
return ret;
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index ed2c228da7..cd65524e26 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -44,6 +44,7 @@ typedef struct BDRVCopyBeforeWriteState {
BdrvChild *target;
OnCbwError on_cbw_error;
uint32_t cbw_timeout_ns;
+ bool discard_source;
/*
* @lock: protects access to @access_bitmap, @done_bitmap and
@@ -357,6 +358,8 @@ cbw_child_perm(BlockDriverState *bs, BdrvChild *c, BdrvChildRole role,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared)
{
+ BDRVCopyBeforeWriteState *s = bs->opaque;
+
if (!(role & BDRV_CHILD_FILTERED)) {
/*
* Target child
@@ -381,6 +384,10 @@ cbw_child_perm(BlockDriverState *bs, BdrvChild *c, BdrvChildRole role,
* start
*/
*nperm = *nperm | BLK_PERM_CONSISTENT_READ;
+ if (s->discard_source) {
+ *nperm = *nperm | BLK_PERM_WRITE;
+ }
+
*nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
}
}
@@ -468,7 +475,9 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
bs->file->bs->supported_zero_flags);
- s->bcs = block_copy_state_new(bs->file, s->target, bs, bitmap, errp);
+ 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);
if (!s->bcs) {
error_prepend(errp, "Cannot create block-copy-state: ");
return -EINVAL;
@@ -535,12 +544,14 @@ static BlockDriver bdrv_cbw_filter = {
BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
+ bool discard_source,
BlockCopyState **bcs,
Error **errp)
{
BDRVCopyBeforeWriteState *state;
BlockDriverState *top;
QDict *opts;
+ int flags = BDRV_O_RDWR | (discard_source ? BDRV_O_CBW_DISCARD_SOURCE : 0);
assert(source->total_sectors == target->total_sectors);
GLOBAL_STATE_CODE();
@@ -553,7 +564,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));
- top = bdrv_insert_node(source, opts, BDRV_O_RDWR, errp);
+ 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 6e72bb25e9..01af0cd3c4 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -39,6 +39,7 @@
BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
+ bool discard_source,
BlockCopyState **bcs,
Error **errp);
void bdrv_cbw_drop(BlockDriverState *bs);
diff --git a/block/replication.c b/block/replication.c
index ca6bd0a720..0415a5e8b7 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -582,8 +582,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
s->backup_job = backup_job_create(
NULL, s->secondary_disk->bs, s->hidden_disk->bs,
- 0, MIRROR_SYNC_MODE_NONE, NULL, 0, false, NULL,
- &perf,
+ 0, MIRROR_SYNC_MODE_NONE, NULL, 0, false, false,
+ NULL, &perf,
BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
backup_job_completed, bs, NULL, &local_err);
diff --git a/blockdev.c b/blockdev.c
index d8fb3399f5..67c1feeceb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2725,7 +2725,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
backup->sync, bmap, backup->bitmap_mode,
- backup->compress,
+ backup->compress, backup->discard_source,
backup->filter_node_name,
&perf,
backup->on_source_error,
diff --git a/include/block/block-common.h b/include/block/block-common.h
index a846023a09..338fe5ff7a 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -243,6 +243,8 @@ typedef enum {
read-write fails */
#define BDRV_O_IO_URING 0x40000 /* use io_uring instead of the thread pool */
+#define BDRV_O_CBW_DISCARD_SOURCE 0x80000 /* for copy-before-write filter */
+
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 8b41643bfa..bdc703bacd 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -27,6 +27,7 @@ typedef struct BlockCopyCallState BlockCopyCallState;
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
BlockDriverState *copy_bitmap_bs,
const BdrvDirtyBitmap *bitmap,
+ bool discard_source,
Error **errp);
/* Function should be called prior any actual copy request */
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index d2201e27f4..eb2d92a226 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -193,7 +193,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
MirrorSyncMode sync_mode,
BdrvDirtyBitmap *sync_bitmap,
BitmapSyncMode bitmap_mode,
- bool compress,
+ bool compress, bool discard_source,
const char *filter_node_name,
BackupPerf *perf,
BlockdevOnError on_source_error,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1874f880a8..2ef52ae9a7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1610,6 +1610,9 @@
# node specified by @drive. If this option is not given, a node
# name is autogenerated. (Since: 4.2)
#
+# @discard-source: Discard blocks on source which are already copied
+# to the target. (Since 9.1)
+#
# @x-perf: Performance options. (Since 6.0)
#
# Features:
@@ -1631,6 +1634,7 @@
'*on-target-error': 'BlockdevOnError',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool',
'*filter-node-name': 'str',
+ '*discard-source': 'bool',
'*x-perf': { 'type': 'BackupPerf',
'features': [ 'unstable' ] } } }
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter
2024-03-13 15:28 ` [PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter Vladimir Sementsov-Ogievskiy
@ 2024-03-13 16:08 ` Markus Armbruster
2024-03-14 13:44 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 13+ messages in thread
From: Markus Armbruster @ 2024-03-13 16:08 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-block, qemu-devel, eblake, xiechanglong.d, wencongyang2,
hreitz, kwolf, jsnow, f.ebner
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> Add a parameter that enables discard-after-copy. That is mostly useful
> in "push backup with fleecing" scheme, when source is snapshot-access
> format driver node, based on copy-before-write filter snapshot-access
> API:
>
> [guest] [snapshot-access] ~~ blockdev-backup ~~> [backup target]
> | |
> | root | file
> v v
> [copy-before-write]
> | |
> | file | target
> v v
> [active disk] [temp.img]
>
> In this case discard-after-copy does two things:
>
> - discard data in temp.img to save disk space
> - avoid further copy-before-write operation in discarded area
>
> Note that we have to declare WRITE permission on source in
> copy-before-write filter, for discard to work. Still we can't take it
> unconditionally, as it will break normal backup from RO source. So, we
> have to add a parameter and pass it thorough bdrv_open flags.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 1874f880a8..2ef52ae9a7 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1610,6 +1610,9 @@
> # node specified by @drive. If this option is not given, a node
> # name is autogenerated. (Since: 4.2)
> #
> +# @discard-source: Discard blocks on source which are already copied
"have been copied"?
> +# to the target. (Since 9.1)
> +#
> # @x-perf: Performance options. (Since 6.0)
> #
> # Features:
> @@ -1631,6 +1634,7 @@
> '*on-target-error': 'BlockdevOnError',
> '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
> '*filter-node-name': 'str',
> + '*discard-source': 'bool',
> '*x-perf': { 'type': 'BackupPerf',
> 'features': [ 'unstable' ] } } }
QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter
2024-03-13 16:08 ` Markus Armbruster
@ 2024-03-14 13:44 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-03-14 13:44 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-block, qemu-devel, eblake, xiechanglong.d, wencongyang2,
hreitz, kwolf, jsnow, f.ebner
On 13.03.24 19:08, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>
>> Add a parameter that enables discard-after-copy. That is mostly useful
>> in "push backup with fleecing" scheme, when source is snapshot-access
>> format driver node, based on copy-before-write filter snapshot-access
>> API:
>>
>> [guest] [snapshot-access] ~~ blockdev-backup ~~> [backup target]
>> | |
>> | root | file
>> v v
>> [copy-before-write]
>> | |
>> | file | target
>> v v
>> [active disk] [temp.img]
>>
>> In this case discard-after-copy does two things:
>>
>> - discard data in temp.img to save disk space
>> - avoid further copy-before-write operation in discarded area
>>
>> Note that we have to declare WRITE permission on source in
>> copy-before-write filter, for discard to work. Still we can't take it
>> unconditionally, as it will break normal backup from RO source. So, we
>> have to add a parameter and pass it thorough bdrv_open flags.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
>> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
>
> [...]
>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 1874f880a8..2ef52ae9a7 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1610,6 +1610,9 @@
>> # node specified by @drive. If this option is not given, a node
>> # name is autogenerated. (Since: 4.2)
>> #
>> +# @discard-source: Discard blocks on source which are already copied
>
> "have been copied"?
Oh, right
>
>> +# to the target. (Since 9.1)
>> +#
>> # @x-perf: Performance options. (Since 6.0)
>> #
>> # Features:
>> @@ -1631,6 +1634,7 @@
>> '*on-target-error': 'BlockdevOnError',
>> '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
>> '*filter-node-name': 'str',
>> + '*discard-source': 'bool',
>> '*x-perf': { 'type': 'BackupPerf',
>> 'features': [ 'unstable' ] } } }
>
> QAPI schema
> Acked-by: Markus Armbruster <armbru@redhat.com>
>
Thanks!
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 5/5] iotests: add backup-discard-source
2024-03-13 15:28 [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2024-03-13 15:28 ` [PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter Vladimir Sementsov-Ogievskiy
@ 2024-03-13 15:28 ` Vladimir Sementsov-Ogievskiy
2024-06-11 17:49 ` Kevin Wolf
2024-04-05 13:46 ` [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
5 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-03-13 15:28 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, eblake, xiechanglong.d, wencongyang2, hreitz,
kwolf, vsementsov, jsnow, f.ebner
Add test for a new backup option: discard-source.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
---
.../qemu-iotests/tests/backup-discard-source | 152 ++++++++++++++++++
.../tests/backup-discard-source.out | 5 +
2 files changed, 157 insertions(+)
create mode 100755 tests/qemu-iotests/tests/backup-discard-source
create mode 100644 tests/qemu-iotests/tests/backup-discard-source.out
diff --git a/tests/qemu-iotests/tests/backup-discard-source b/tests/qemu-iotests/tests/backup-discard-source
new file mode 100755
index 0000000000..2391b12acd
--- /dev/null
+++ b/tests/qemu-iotests/tests/backup-discard-source
@@ -0,0 +1,152 @@
+#!/usr/bin/env python3
+#
+# Test backup discard-source parameter
+#
+# Copyright (c) Virtuozzo International GmbH.
+# Copyright (c) Yandex
+#
+# 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/>.
+#
+
+import os
+
+import iotests
+from iotests import qemu_img_create, qemu_img_map, qemu_io
+
+
+temp_img = os.path.join(iotests.test_dir, 'temp')
+source_img = os.path.join(iotests.test_dir, 'source')
+target_img = os.path.join(iotests.test_dir, 'target')
+size = '1M'
+
+
+def get_actual_size(vm, node_name):
+ nodes = vm.cmd('query-named-block-nodes', flat=True)
+ node = next(n for n in nodes if n['node-name'] == node_name)
+ return node['image']['actual-size']
+
+
+class TestBackup(iotests.QMPTestCase):
+ def setUp(self):
+ qemu_img_create('-f', iotests.imgfmt, source_img, size)
+ qemu_img_create('-f', iotests.imgfmt, temp_img, size)
+ qemu_img_create('-f', iotests.imgfmt, target_img, size)
+ qemu_io('-c', 'write 0 1M', source_img)
+
+ self.vm = iotests.VM()
+ self.vm.launch()
+
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'cbw',
+ 'driver': 'copy-before-write',
+ 'file': {
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': source_img,
+ }
+ },
+ 'target': {
+ 'driver': iotests.imgfmt,
+ 'discard': 'unmap',
+ 'node-name': 'temp',
+ 'file': {
+ 'driver': 'file',
+ 'filename': temp_img
+ }
+ }
+ })
+
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'access',
+ 'discard': 'unmap',
+ 'driver': 'snapshot-access',
+ 'file': 'cbw'
+ })
+
+ self.vm.cmd('blockdev-add', {
+ 'driver': iotests.imgfmt,
+ 'node-name': 'target',
+ 'file': {
+ 'driver': 'file',
+ 'filename': target_img
+ }
+ })
+
+ self.assertLess(get_actual_size(self.vm, 'temp'), 512 * 1024)
+
+ def tearDown(self):
+ # That should fail, because region is discarded
+ self.vm.hmp_qemu_io('access', 'read 0 1M')
+
+ self.vm.shutdown()
+
+ self.assertTrue('read failed: Permission denied' in self.vm.get_log())
+
+ # Final check that temp image is empty
+ mapping = qemu_img_map(temp_img)
+ self.assertEqual(len(mapping), 1)
+ self.assertEqual(mapping[0]['start'], 0)
+ self.assertEqual(mapping[0]['length'], 1024 * 1024)
+ self.assertEqual(mapping[0]['data'], False)
+
+ os.remove(temp_img)
+ os.remove(source_img)
+ os.remove(target_img)
+
+ def do_backup(self):
+ self.vm.cmd('blockdev-backup', device='access',
+ sync='full', target='target',
+ job_id='backup0',
+ discard_source=True)
+
+ self.vm.event_wait(name='BLOCK_JOB_COMPLETED')
+
+ def test_discard_written(self):
+ """
+ 1. Guest writes
+ 2. copy-before-write operation, data is stored to temp
+ 3. start backup(discard_source=True), check that data is
+ removed from temp
+ """
+ # Trigger copy-before-write operation
+ result = self.vm.hmp_qemu_io('cbw', 'write 0 1M')
+ self.assert_qmp(result, 'return', '')
+
+ # Check that data is written to temporary image
+ self.assertGreater(get_actual_size(self.vm, 'temp'), 1024 * 1024)
+
+ self.do_backup()
+
+ def test_discard_cbw(self):
+ """
+ 1. do backup(discard_source=True), which should inform
+ copy-before-write that data is not needed anymore
+ 2. Guest writes
+ 3. Check that copy-before-write operation is not done
+ """
+ self.do_backup()
+
+ # Try trigger copy-before-write operation
+ result = self.vm.hmp_qemu_io('cbw', 'write 0 1M')
+ self.assert_qmp(result, 'return', '')
+
+ # Check that data is not written to temporary image, as region
+ # is discarded from copy-before-write process
+ self.assertLess(get_actual_size(self.vm, 'temp'), 512 * 1024)
+
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2'],
+ supported_protocols=['file'])
diff --git a/tests/qemu-iotests/tests/backup-discard-source.out b/tests/qemu-iotests/tests/backup-discard-source.out
new file mode 100644
index 0000000000..fbc63e62f8
--- /dev/null
+++ b/tests/qemu-iotests/tests/backup-discard-source.out
@@ -0,0 +1,5 @@
+..
+----------------------------------------------------------------------
+Ran 2 tests
+
+OK
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 5/5] iotests: add backup-discard-source
2024-03-13 15:28 ` [PATCH v4 5/5] iotests: add backup-discard-source Vladimir Sementsov-Ogievskiy
@ 2024-06-11 17:49 ` Kevin Wolf
2024-06-12 19:21 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2024-06-11 17:49 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-block, qemu-devel, armbru, eblake, xiechanglong.d,
wencongyang2, hreitz, jsnow, f.ebner
Am 13.03.2024 um 16:28 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add test for a new backup option: discard-source.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
This test fails for me, and it already does so after this commit that
introduced it. I haven't checked what get_actual_size(), but I'm running
on XFS, so its preallocation could be causing this. We generally avoid
checking the number of allocated blocks in image files for this reason.
Kevin
backup-discard-source fail [19:45:49] [19:45:50] 0.8s failed, exit status 1
--- /home/kwolf/source/qemu/tests/qemu-iotests/tests/backup-discard-source.out
+++ /home/kwolf/source/qemu/build-clang/scratch/qcow2-file-backup-discard-source/backup-discard-source.out.bad
@@ -1,5 +1,14 @@
-..
+F.
+======================================================================
+FAIL: test_discard_cbw (__main__.TestBackup.test_discard_cbw)
+1. do backup(discard_source=True), which should inform
+----------------------------------------------------------------------
+Traceback (most recent call last):
+ File "/home/kwolf/source/qemu/tests/qemu-iotests/tests/backup-discard-source", line 147, in test_discard_cbw
+ self.assertLess(get_actual_size(self.vm, 'temp'), 512 * 1024)
+AssertionError: 1249280 not less than 524288
+
----------------------------------------------------------------------
Ran 2 tests
-OK
+FAILED (failures=1)
Failures: backup-discard-source
Failed 1 of 1 iotests
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/5] iotests: add backup-discard-source
2024-06-11 17:49 ` Kevin Wolf
@ 2024-06-12 19:21 ` Vladimir Sementsov-Ogievskiy
2024-06-13 8:02 ` Kevin Wolf
0 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-06-12 19:21 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-block, qemu-devel, armbru, eblake, xiechanglong.d,
wencongyang2, hreitz, jsnow, f.ebner
On 11.06.24 20:49, Kevin Wolf wrote:
> Am 13.03.2024 um 16:28 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> Add test for a new backup option: discard-source.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
>> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
>
> This test fails for me, and it already does so after this commit that
> introduced it. I haven't checked what get_actual_size(), but I'm running
> on XFS, so its preallocation could be causing this. We generally avoid
> checking the number of allocated blocks in image files for this reason.
>
Hmm right, I see that relying on allocated size is bad thing. Better is to check block status, to see how many qcow2 clusters are allocated.
Do we have some qmp command to get such information? The simplest way I see is to add dirty to temp block-node, and then check its dirty count through query-named-block-nodes
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/5] iotests: add backup-discard-source
2024-06-12 19:21 ` Vladimir Sementsov-Ogievskiy
@ 2024-06-13 8:02 ` Kevin Wolf
2024-06-20 14:15 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2024-06-13 8:02 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-block, qemu-devel, armbru, eblake, xiechanglong.d,
wencongyang2, hreitz, jsnow, f.ebner
Am 12.06.2024 um 21:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
> On 11.06.24 20:49, Kevin Wolf wrote:
> > Am 13.03.2024 um 16:28 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > Add test for a new backup option: discard-source.
> > >
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > > Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> > > Tested-by: Fiona Ebner <f.ebner@proxmox.com>
> >
> > This test fails for me, and it already does so after this commit that
> > introduced it. I haven't checked what get_actual_size(), but I'm running
> > on XFS, so its preallocation could be causing this. We generally avoid
> > checking the number of allocated blocks in image files for this reason.
> >
>
> Hmm right, I see that relying on allocated size is bad thing. Better
> is to check block status, to see how many qcow2 clusters are
> allocated.
>
> Do we have some qmp command to get such information? The simplest way
> I see is to add dirty to temp block-node, and then check its dirty
> count through query-named-block-nodes
Hm, does it have to be QMP in a running QEMU process? I'm not sure what
the best way would be there.
Otherwise, my approach would be 'qemu-img check' or even 'qemu-img map',
depending on what you want to verify with it.
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/5] iotests: add backup-discard-source
2024-06-13 8:02 ` Kevin Wolf
@ 2024-06-20 14:15 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-06-20 14:15 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-block, qemu-devel, armbru, eblake, xiechanglong.d,
wencongyang2, hreitz, jsnow, f.ebner
On 13.06.24 11:02, Kevin Wolf wrote:
> Am 12.06.2024 um 21:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> On 11.06.24 20:49, Kevin Wolf wrote:
>>> Am 13.03.2024 um 16:28 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> Add test for a new backup option: discard-source.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
>>>> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
>>>
>>> This test fails for me, and it already does so after this commit that
>>> introduced it. I haven't checked what get_actual_size(), but I'm running
>>> on XFS, so its preallocation could be causing this. We generally avoid
>>> checking the number of allocated blocks in image files for this reason.
>>>
>>
>> Hmm right, I see that relying on allocated size is bad thing. Better
>> is to check block status, to see how many qcow2 clusters are
>> allocated.
>>
>> Do we have some qmp command to get such information? The simplest way
>> I see is to add dirty to temp block-node, and then check its dirty
>> count through query-named-block-nodes
>
> Hm, does it have to be QMP in a running QEMU process?
hmm, yes, seems in test_discard_written() we do want to examine running process. I'll try to go with bitmap
> I'm not sure what
> the best way would be there.
>
> Otherwise, my approach would be 'qemu-img check' or even 'qemu-img map',
> depending on what you want to verify with it.
>
> Kevin
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/5] backup: discard-source parameter
2024-03-13 15:28 [PATCH v4 0/5] backup: discard-source parameter Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2024-03-13 15:28 ` [PATCH v4 5/5] iotests: add backup-discard-source Vladimir Sementsov-Ogievskiy
@ 2024-04-05 13:46 ` Vladimir Sementsov-Ogievskiy
5 siblings, 0 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-04-05 13:46 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, eblake, xiechanglong.d, wencongyang2, hreitz,
kwolf, jsnow, f.ebner
On 13.03.24 18:28, Vladimir Sementsov-Ogievskiy wrote:
> Hi all! The main patch is 04, please look at it for description and
> diagram.
>
> v4: add t-b by Fiona
> add r-b by Fiona to 02-05 (patch 01 still lack an r-b)
> 05: fix copyrights and subject in the test
> 04: since 9.0 --> since 9.1 (we missed a soft freeze for 9.0)
>
> Vladimir Sementsov-Ogievskiy (5):
> block/copy-before-write: fix permission
> block/copy-before-write: support unligned snapshot-discard
> block/copy-before-write: create block_copy bitmap in filter node
> qapi: blockdev-backup: add discard-source parameter
> iotests: add backup-discard-source
>
> block/backup.c | 5 +-
> block/block-copy.c | 12 +-
> block/copy-before-write.c | 39 ++++-
> block/copy-before-write.h | 1 +
> block/replication.c | 4 +-
> blockdev.c | 2 +-
> include/block/block-common.h | 2 +
> include/block/block-copy.h | 2 +
> include/block/block_int-global-state.h | 2 +-
> qapi/block-core.json | 4 +
> tests/qemu-iotests/257.out | 112 ++++++-------
> .../qemu-iotests/tests/backup-discard-source | 152 ++++++++++++++++++
> .../tests/backup-discard-source.out | 5 +
> 13 files changed, 272 insertions(+), 70 deletions(-)
> create mode 100755 tests/qemu-iotests/tests/backup-discard-source
> create mode 100644 tests/qemu-iotests/tests/backup-discard-source.out
>
Thanks for review, applied to my block branch.
(r-b to 01 is still appreciated, I will not pull this until 9.1 tree opened)
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 13+ messages in thread