Skip to content

Commit

Permalink
qapi: blockdev-backup: add discard-source parameter
Browse files Browse the repository at this point in the history
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>
Acked-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20240313152822.626493-5-vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
  • Loading branch information
Vladimir Sementsov-Ogievskiy committed Apr 29, 2024
1 parent c0aab14 commit e962cb8
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 8 deletions.
5 changes: 3 additions & 2 deletions block/backup.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down
9 changes: 9 additions & 0 deletions block/block-copy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/*
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
15 changes: 13 additions & 2 deletions block/copy-before-write.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions block/copy-before-write.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions block/replication.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion blockdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -2726,7 +2726,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,
Expand Down
2 changes: 2 additions & 0 deletions include/block/block-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
1 change: 1 addition & 0 deletions include/block/block-copy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 1 addition & 1 deletion include/block/block_int-global-state.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions qapi/block-core.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 have already been
# copied to the target. (Since 9.1)
#
# @x-perf: Performance options. (Since 6.0)
#
# Features:
Expand All @@ -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' ] } } }

Expand Down

0 comments on commit e962cb8

Please sign in to comment.