qapi: backup: add perf.use-copy-range parameter

Experiments show, that copy_range is not always making things faster.
So, to make experimentation simpler, let's add a parameter. Some more
perf parameters will be added soon, so here is a new struct.

For now, add new backup qmp parameter with x- prefix for the following
reasons:

 - We are going to add more performance parameters, some will be
   related to the whole block-copy process, some only to background
   copying in backup (ignored for copy-before-write operations).
 - On the other hand, we are going to use block-copy interface in other
   block jobs, which will need performance options as well.. And it
   should be the same structure or at least somehow related.

So, there are too much unclean things about how the interface and now
we need the new options mostly for testing. Let's keep them
experimental for a while.

In do_backup_common() new x-perf parameter handled in a way to
make further options addition simpler.

We add use-copy-range with default=true, and we'll change the default
in further patch, after moving backup to use block-copy.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210116214705.822267-2-vsementsov@virtuozzo.com>
[mreitz: s/5\.2/6.0/]
Signed-off-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
Vladimir Sementsov-Ogievskiy 2021-01-17 00:46:43 +03:00 committed by Max Reitz
parent f4be822519
commit 86c6a3b690
9 changed files with 41 additions and 6 deletions

View File

@ -186,6 +186,7 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
uint64_t cluster_size,
BackupPerf *perf,
BdrvRequestFlags write_flags,
BlockCopyState **bcs,
Error **errp)
@ -244,7 +245,8 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
state->cluster_size = cluster_size;
state->bcs = block_copy_state_new(top->backing, state->target,
cluster_size, write_flags, &local_err);
cluster_size, perf->use_copy_range,
write_flags, &local_err);
if (local_err) {
error_prepend(&local_err, "Cannot create block-copy-state: ");
goto fail;

View File

@ -33,6 +33,7 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
uint64_t cluster_size,
BackupPerf *perf,
BdrvRequestFlags write_flags,
BlockCopyState **bcs,
Error **errp);

View File

@ -46,6 +46,7 @@ typedef struct BackupBlockJob {
uint64_t len;
uint64_t bytes_read;
int64_t cluster_size;
BackupPerf perf;
BlockCopyState *bcs;
} BackupBlockJob;
@ -335,6 +336,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BitmapSyncMode bitmap_mode,
bool compress,
const char *filter_node_name,
BackupPerf *perf,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
int creation_flags,
@ -441,7 +443,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
(compress ? BDRV_REQ_WRITE_COMPRESSED : 0),
backup_top = bdrv_backup_top_append(bs, target, filter_node_name,
cluster_size, write_flags, &bcs, errp);
cluster_size, perf,
write_flags, &bcs, errp);
if (!backup_top) {
goto error;
}
@ -464,6 +467,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
job->bcs = bcs;
job->cluster_size = cluster_size;
job->len = len;
job->perf = *perf;
block_copy_set_progress_callback(bcs, backup_progress_bytes_callback, job);
block_copy_set_progress_meter(bcs, &job->common.job.progress);

View File

@ -218,7 +218,7 @@ static uint32_t block_copy_max_transfer(BdrvChild *source, BdrvChild *target)
}
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
int64_t cluster_size,
int64_t cluster_size, bool use_copy_range,
BdrvRequestFlags write_flags, Error **errp)
{
BlockCopyState *s;
@ -260,7 +260,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
* We enable copy-range, but keep small copy_size, until first
* successful copy_range (look at block_copy_do_copy).
*/
s->use_copy_range = true;
s->use_copy_range = use_copy_range;
s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
}

View File

@ -454,6 +454,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
int64_t active_length, hidden_length, disk_length;
AioContext *aio_context;
Error *local_err = NULL;
BackupPerf perf = { .use_copy_range = true };
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@ -558,6 +559,7 @@ 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,
BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
backup_job_completed, bs, NULL, &local_err);

View File

@ -2829,6 +2829,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
{
BlockJob *job = NULL;
BdrvDirtyBitmap *bmap = NULL;
BackupPerf perf = { .use_copy_range = true };
int job_flags = JOB_DEFAULT;
if (!backup->has_speed) {
@ -2853,6 +2854,12 @@ static BlockJob *do_backup_common(BackupCommon *backup,
backup->compress = false;
}
if (backup->x_perf) {
if (backup->x_perf->has_use_copy_range) {
perf.use_copy_range = backup->x_perf->use_copy_range;
}
}
if ((backup->sync == MIRROR_SYNC_MODE_BITMAP) ||
(backup->sync == MIRROR_SYNC_MODE_INCREMENTAL)) {
/* done before desugaring 'incremental' to print the right message */
@ -2926,6 +2933,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
backup->sync, bmap, backup->bitmap_mode,
backup->compress,
backup->filter_node_name,
&perf,
backup->on_source_error,
backup->on_target_error,
job_flags, NULL, NULL, txn, errp);

View File

@ -22,7 +22,7 @@ typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
typedef struct BlockCopyState BlockCopyState;
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
int64_t cluster_size,
int64_t cluster_size, bool use_copy_range,
BdrvRequestFlags write_flags,
Error **errp);

View File

@ -1266,6 +1266,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
* @sync_mode: What parts of the disk image should be copied to the destination.
* @sync_bitmap: The dirty bitmap if sync_mode is 'bitmap' or 'incremental'
* @bitmap_mode: The bitmap synchronization policy to use.
* @perf: Performance options. All actual fields assumed to be present,
* 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.
* @creation_flags: Flags that control the behavior of the Job lifetime.
@ -1284,6 +1286,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BitmapSyncMode bitmap_mode,
bool compress,
const char *filter_node_name,
BackupPerf *perf,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
int creation_flags,

View File

@ -1371,6 +1371,19 @@
{ 'struct': 'BlockdevSnapshot',
'data': { 'node': 'str', 'overlay': 'str' } }
##
# @BackupPerf:
#
# Optional parameters for backup. These parameters don't affect
# functionality, but may significantly affect performance.
#
# @use-copy-range: Use copy offloading. Default true.
#
# Since: 6.0
##
{ 'struct': 'BackupPerf',
'data': { '*use-copy-range': 'bool' }}
##
# @BackupCommon:
#
@ -1426,6 +1439,8 @@
# above node specified by @drive. If this option is not given,
# a node name is autogenerated. (Since: 4.2)
#
# @x-perf: Performance options. (Since 6.0)
#
# Note: @on-source-error and @on-target-error only affect background
# I/O. If an error occurs during a guest write request, the device's
# rerror/werror actions will be used.
@ -1440,7 +1455,7 @@
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool',
'*filter-node-name': 'str' } }
'*filter-node-name': 'str', '*x-perf': 'BackupPerf' } }
##
# @DriveBackup: