From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
Peter Maydell <peter.maydell@linaro.org>,
qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PULL 26/53] qapi: backup: add perf.use-copy-range parameter
Date: Tue, 26 Jan 2021 15:19:49 +0100 [thread overview]
Message-ID: <20210126142016.806073-27-mreitz@redhat.com> (raw)
In-Reply-To: <20210126142016.806073-1-mreitz@redhat.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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>
---
qapi/block-core.json | 17 ++++++++++++++++-
block/backup-top.h | 1 +
include/block/block-copy.h | 2 +-
include/block/block_int.h | 3 +++
block/backup-top.c | 4 +++-
block/backup.c | 6 +++++-
block/block-copy.c | 4 ++--
block/replication.c | 2 ++
blockdev.c | 8 ++++++++
9 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1d9dcd7d30..83f661d7f6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
diff --git a/block/backup-top.h b/block/backup-top.h
index e5cabfa197..b28b0031c4 100644
--- a/block/backup-top.h
+++ b/block/backup-top.h
@@ -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);
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index aac85e1488..6397505f30 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -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);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f4b844f310..d01fc23720 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -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,
diff --git a/block/backup-top.c b/block/backup-top.c
index fe6883cc97..789acf6965 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -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;
diff --git a/block/backup.c b/block/backup.c
index 9afa0bf3b4..4b07e9115d 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -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);
diff --git a/block/block-copy.c b/block/block-copy.c
index cd9bc47c8f..63398a171c 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -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);
}
diff --git a/block/replication.c b/block/replication.c
index 0c70215784..22ffc811ee 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -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);
diff --git a/blockdev.c b/blockdev.c
index 0540c621da..fc88dc03e1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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);
--
2.29.2
next prev parent reply other threads:[~2021-01-26 14:50 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-26 14:19 [PULL 00/53] Block patches Max Reitz
2021-01-26 14:19 ` [PULL 01/53] iotests: fix _check_o_direct Max Reitz
2021-01-26 14:19 ` [PULL 02/53] copy-on-read: support preadv/pwritev_part functions Max Reitz
2021-01-26 14:19 ` [PULL 03/53] block: add API function to insert a node Max Reitz
2021-01-26 14:19 ` [PULL 04/53] copy-on-read: add filter drop function Max Reitz
2021-01-26 14:19 ` [PULL 05/53] qapi: add filter-node-name to block-stream Max Reitz
2021-01-26 14:19 ` [PULL 06/53] qapi: copy-on-read filter: add 'bottom' option Max Reitz
2021-01-26 14:19 ` [PULL 07/53] iotests: add #310 to test bottom node in COR driver Max Reitz
2021-01-26 14:19 ` [PULL 08/53] block: include supported_read_flags into BDS structure Max Reitz
2021-01-26 14:19 ` [PULL 09/53] copy-on-read: skip non-guest reads if no copy needed Max Reitz
2021-01-26 14:19 ` [PULL 10/53] stream: rework backing-file changing Max Reitz
2021-01-26 14:19 ` [PULL 11/53] qapi: block-stream: add "bottom" argument Max Reitz
2021-01-26 14:19 ` [PULL 12/53] iotests: 30: prepare to COR filter insertion by stream job Max Reitz
2021-01-26 14:19 ` [PULL 13/53] block/stream: add s->target_bs Max Reitz
2021-01-26 14:19 ` [PULL 14/53] block: apply COR-filter to block-stream jobs Max Reitz
2021-01-28 18:38 ` Philippe Mathieu-Daudé
2021-01-29 5:26 ` Vladimir Sementsov-Ogievskiy
2021-01-29 8:23 ` Max Reitz
2021-01-29 9:23 ` Philippe Mathieu-Daudé
2021-01-26 14:19 ` [PULL 15/53] iotests.py: Assume a couple of variables as given Max Reitz
2021-01-26 14:19 ` [PULL 16/53] iotests/297: Rewrite in Python and extend reach Max Reitz
2021-01-26 14:19 ` [PULL 17/53] iotests: Move try_remove to iotests.py Max Reitz
2021-01-26 14:19 ` [PULL 18/53] iotests/129: Remove test images in tearDown() Max Reitz
2021-01-26 14:19 ` [PULL 19/53] iotests/129: Do not check @busy Max Reitz
2021-01-26 14:19 ` [PULL 20/53] iotests/129: Use throttle node Max Reitz
2021-01-26 14:19 ` [PULL 21/53] iotests/129: Actually test a commit job Max Reitz
2021-01-26 14:19 ` [PULL 22/53] iotests/129: Limit mirror job's buffer size Max Reitz
2021-01-26 14:19 ` [PULL 23/53] iotests/129: Clean up pylint and mypy complaints Max Reitz
2021-01-26 14:19 ` [PULL 24/53] iotests/300: " Max Reitz
2021-01-26 14:19 ` [PULL 25/53] coroutine-sigaltstack: Add SIGUSR2 mutex Max Reitz
2021-01-26 14:19 ` Max Reitz [this message]
2021-01-26 14:19 ` [PULL 27/53] block/block-copy: More explicit call_state Max Reitz
2021-01-26 14:19 ` [PULL 28/53] block/block-copy: implement block_copy_async Max Reitz
2021-01-26 14:19 ` [PULL 29/53] block/block-copy: add max_chunk and max_workers parameters Max Reitz
2021-01-26 14:19 ` [PULL 30/53] block/block-copy: add list of all call-states Max Reitz
2021-01-26 14:19 ` [PULL 31/53] block/block-copy: add ratelimit to block-copy Max Reitz
2021-01-26 14:19 ` [PULL 32/53] block/block-copy: add block_copy_cancel Max Reitz
2021-01-26 14:19 ` [PULL 33/53] blockjob: add set_speed to BlockJobDriver Max Reitz
2021-01-26 14:19 ` [PULL 34/53] job: call job_enter from job_pause Max Reitz
2021-01-26 14:19 ` [PULL 35/53] qapi: backup: add max-chunk and max-workers to x-perf struct Max Reitz
2021-01-26 14:19 ` [PULL 36/53] iotests: 56: prepare for backup over block-copy Max Reitz
2021-01-26 14:20 ` [PULL 37/53] iotests/129: Limit backup's max-chunk/max-workers Max Reitz
2021-01-26 14:20 ` [PULL 38/53] iotests: 185: prepare for backup over block-copy Max Reitz
2021-01-26 14:20 ` [PULL 39/53] iotests: 219: " Max Reitz
2021-01-26 14:20 ` [PULL 40/53] iotests: 257: " Max Reitz
2021-01-26 14:20 ` [PULL 41/53] block/block-copy: make progress_bytes_callback optional Max Reitz
2021-01-26 14:20 ` [PULL 42/53] block/backup: drop extra gotos from backup_run() Max Reitz
2021-01-26 14:20 ` [PULL 43/53] backup: move to block-copy Max Reitz
2021-01-26 14:20 ` [PULL 44/53] qapi: backup: disable copy_range by default Max Reitz
2021-01-26 14:20 ` [PULL 45/53] block/block-copy: drop unused block_copy_set_progress_callback() Max Reitz
2021-01-26 14:20 ` [PULL 46/53] block/block-copy: drop unused argument of block_copy() Max Reitz
2021-01-26 14:20 ` [PULL 47/53] simplebench/bench_block_job: use correct shebang line with python3 Max Reitz
2021-01-26 14:20 ` [PULL 48/53] simplebench: bench_block_job: add cmd_options argument Max Reitz
2021-01-26 14:20 ` [PULL 49/53] simplebench: add bench-backup.py Max Reitz
2021-01-26 14:20 ` [PULL 50/53] block: report errno when flock fcntl fails Max Reitz
2021-01-26 14:20 ` [PULL 51/53] iotests: Add test for the regression fixed in c8bf9a9169 Max Reitz
2021-01-26 14:20 ` [PULL 52/53] iotests/118: Drop 'change' test Max Reitz
2021-01-26 14:20 ` [PULL 53/53] iotests/178: Pass value to invalid option Max Reitz
2021-01-27 17:40 ` [PULL 00/53] Block patches Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210126142016.806073-27-mreitz@redhat.com \
--to=mreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).