This brings async request handling and block-status driven chunk sizes
to backup out of the box, which improves backup performance.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
include/block/block-copy.h | 9 +--
block/backup.c | 145 +++++++++++++++++++------------------
block/block-copy.c | 21 ++----
3 files changed, 83 insertions(+), 92 deletions(-)
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 370a194d3c..a3e11d3fa2 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -18,7 +18,6 @@
#include "block/block.h"
#include "qemu/co-shared-resource.h"
-typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
typedef void (*BlockCopyAsyncCallbackFunc)(int ret, bool error_is_read,
void *opaque);
typedef struct BlockCopyState BlockCopyState;
@@ -29,11 +28,6 @@ BlockCopyState *block_copy_state_new(BdrvChild *source,
BdrvChild *target,
BdrvRequestFlags write_flags,
Error **errp);
-void block_copy_set_progress_callback(
- BlockCopyState *s,
- ProgressBytesCallbackFunc progress_bytes_callback,
- void *progress_opaque);
-
void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm);
void block_copy_state_free(BlockCopyState *s);
@@ -57,7 +51,8 @@ BlockCopyCallState *block_copy_async(BlockCopyState *s,
int64_t offset, int64_t bytes,
bool ratelimit, int max_workers,
int64_t max_chunk,
- BlockCopyAsyncCallbackFunc cb);
+ BlockCopyAsyncCallbackFunc cb,
+ void *cb_opaque);
/*
* Set speed limit for block-copy instance. All block-copy operations related
to
diff --git a/block/backup.c b/block/backup.c
index ec2676abc2..59c00f5293 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -44,42 +44,19 @@ typedef struct BackupBlockJob {
BlockdevOnError on_source_error;
BlockdevOnError on_target_error;
uint64_t len;
- uint64_t bytes_read;
int64_t cluster_size;
int max_workers;
int64_t max_chunk;
BlockCopyState *bcs;
+
+ BlockCopyCallState *bcs_call;
+ int ret;
+ bool error_is_read;
} BackupBlockJob;
static const BlockJobDriver backup_job_driver;
-static void backup_progress_bytes_callback(int64_t bytes, void *opaque)
-{
- BackupBlockJob *s = opaque;
-
- s->bytes_read += bytes;
-}
-
-static int coroutine_fn backup_do_cow(BackupBlockJob *job,
- int64_t offset, uint64_t bytes,
- bool *error_is_read)
-{
- int ret = 0;
- int64_t start, end; /* bytes */
-
- start = QEMU_ALIGN_DOWN(offset, job->cluster_size);
- end = QEMU_ALIGN_UP(bytes + offset, job->cluster_size);
-
- trace_backup_do_cow_enter(job, start, offset, bytes);
-
- ret = block_copy(job->bcs, start, end - start, error_is_read);
-
- trace_backup_do_cow_return(job, offset, bytes, ret);
-
- return ret;
-}
-
static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
{
BdrvDirtyBitmap *bm;
@@ -159,54 +136,58 @@ static BlockErrorAction
backup_error_action(BackupBlockJob *job,
}
}
-static bool coroutine_fn yield_and_check(BackupBlockJob *job)
+static void coroutine_fn backup_block_copy_callback(int ret, bool
error_is_read,
+ void *opaque)
{
- uint64_t delay_ns;
-
- if (job_is_cancelled(&job->common.job)) {
- return true;
- }
-
- /*
- * We need to yield even for delay_ns = 0 so that bdrv_drain_all() can
- * return. Without a yield, the VM would not reboot.
- */
- delay_ns = block_job_ratelimit_get_delay(&job->common, job->bytes_read);
- job->bytes_read = 0;
- job_sleep_ns(&job->common.job, delay_ns);
-
- if (job_is_cancelled(&job->common.job)) {
- return true;
- }
+ BackupBlockJob *s = opaque;
- return false;
+ s->bcs_call = NULL;
+ s->ret = ret;
+ s->error_is_read = error_is_read;
+ job_enter(&s->common.job);
}
static int coroutine_fn backup_loop(BackupBlockJob *job)
{
- bool error_is_read;
- int64_t offset;
- BdrvDirtyBitmapIter *bdbi;
- int ret = 0;
+ while (true) { /* retry loop */
+ assert(!job->bcs_call);
+ job->bcs_call = block_copy_async(job->bcs, 0,
+ QEMU_ALIGN_UP(job->len,
+ job->cluster_size),
+ true, job->max_workers,
job->max_chunk,
+ backup_block_copy_callback, job);
- bdbi = bdrv_dirty_iter_new(block_copy_dirty_bitmap(job->bcs));
- while ((offset = bdrv_dirty_iter_next(bdbi)) != -1) {
- do {
- if (yield_and_check(job)) {
- goto out;
+ while (job->bcs_call && !job->common.job.cancelled) {
+ /* wait and handle pauses */
+
+ job_pause_point(&job->common.job);
+
+ if (job->bcs_call && !job->common.job.cancelled) {
+ job_yield(&job->common.job);
}
- ret = backup_do_cow(job, offset, job->cluster_size,
&error_is_read);
- if (ret < 0 && backup_error_action(job, error_is_read, -ret) ==
- BLOCK_ERROR_ACTION_REPORT)
- {
- goto out;
+ }
+
+ if (!job->bcs_call && job->ret == 0) {
+ /* Success */
+ return 0;
+ }
+
+ if (job->common.job.cancelled) {
+ if (job->bcs_call) {
+ block_copy_cancel(job->bcs_call);
}
- } while (ret < 0);
+ return 0;
+ }
+
+ if (!job->bcs_call && job->ret < 0 &&
+ (backup_error_action(job, job->error_is_read, -job->ret) ==
+ BLOCK_ERROR_ACTION_REPORT))
+ {
+ return job->ret;
+ }
}
- out:
- bdrv_dirty_iter_free(bdbi);
- return ret;
+ g_assert_not_reached();
}
static void backup_init_bcs_bitmap(BackupBlockJob *job)
@@ -246,9 +227,14 @@ static int coroutine_fn backup_run(Job *job, Error **errp)
int64_t count;
for (offset = 0; offset < s->len; ) {
- if (yield_and_check(s)) {
- ret = -ECANCELED;
- goto out;
+ if (job_is_cancelled(job)) {
+ return -ECANCELED;
+ }
+
+ job_pause_point(job);
+
+ if (job_is_cancelled(job)) {
+ return -ECANCELED;
}
ret = block_copy_reset_unallocated(s->bcs, offset, &count);
@@ -281,6 +267,25 @@ static int coroutine_fn backup_run(Job *job, Error **errp)
return ret;
}
+static void coroutine_fn backup_pause(Job *job)
+{
+ BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
+
+ if (s->bcs_call) {
+ block_copy_cancel(s->bcs_call);
+ }
+}
+
+static void coroutine_fn backup_set_speed(BlockJob *job, int64_t speed)
+{
+ BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+
+ if (s->bcs) {
+ /* In block_job_create we yet don't have bcs */
+ block_copy_set_speed(s->bcs, s->bcs_call, speed);
+ }
+}
+
static const BlockJobDriver backup_job_driver = {
.job_driver = {
.instance_size = sizeof(BackupBlockJob),
@@ -291,7 +296,9 @@ static const BlockJobDriver backup_job_driver = {
.commit = backup_commit,
.abort = backup_abort,
.clean = backup_clean,
- }
+ .pause = backup_pause,
+ },
+ .set_speed = backup_set_speed,
};
static int64_t backup_calculate_cluster_size(BlockDriverState *target,
@@ -487,8 +494,8 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
job->max_workers = max_workers;
job->max_chunk = max_chunk;
- block_copy_set_progress_callback(bcs, backup_progress_bytes_callback, job);
block_copy_set_progress_meter(bcs, &job->common.job.progress);
+ block_copy_set_speed(bcs, NULL, speed);
/* Required permissions are already taken by backup-top target */
block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
diff --git a/block/block-copy.c b/block/block-copy.c
index b551feb6c2..6a9d891b63 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -39,6 +39,7 @@ typedef struct BlockCopyCallState {
int64_t max_chunk;
bool ratelimit;
BlockCopyAsyncCallbackFunc cb;
+ void *cb_opaque;
/* State */
bool failed;
@@ -103,9 +104,6 @@ typedef struct BlockCopyState {
bool skip_unallocated;
ProgressMeter *progress;
- /* progress_bytes_callback: called when some copying progress is done. */
- ProgressBytesCallbackFunc progress_bytes_callback;
- void *progress_opaque;
SharedResource *mem;
@@ -287,15 +285,6 @@ BlockCopyState *block_copy_state_new(BdrvChild *source,
BdrvChild *target,
return s;
}
-void block_copy_set_progress_callback(
- BlockCopyState *s,
- ProgressBytesCallbackFunc progress_bytes_callback,
- void *progress_opaque)
-{
- s->progress_bytes_callback = progress_bytes_callback;
- s->progress_opaque = progress_opaque;
-}
-
void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
{
s->progress = pm;
@@ -443,7 +432,6 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
t->call_state->error_is_read = error_is_read;
} else {
progress_work_done(t->s->progress, t->bytes);
- t->s->progress_bytes_callback(t->bytes, t->s->progress_opaque);
}
co_put_to_shres(t->s->mem, t->bytes);
block_copy_task_end(t, ret);
@@ -712,8 +700,7 @@ static int coroutine_fn
block_copy_common(BlockCopyCallState *call_state)
} while (ret > 0 && !call_state->cancelled);
if (call_state->cb) {
- call_state->cb(ret, call_state->error_is_read,
- call_state->s->progress_opaque);
+ call_state->cb(ret, call_state->error_is_read, call_state->cb_opaque);
}
if (call_state->canceller) {
@@ -754,7 +741,8 @@ BlockCopyCallState *block_copy_async(BlockCopyState *s,
int64_t offset, int64_t bytes,
bool ratelimit, int max_workers,
int64_t max_chunk,
- BlockCopyAsyncCallbackFunc cb)
+ BlockCopyAsyncCallbackFunc cb,
+ void *cb_opaque)
{
BlockCopyCallState *call_state = g_new(BlockCopyCallState, 1);
Coroutine *co = qemu_coroutine_create(block_copy_async_co_entry,
@@ -766,6 +754,7 @@ BlockCopyCallState *block_copy_async(BlockCopyState *s,
.bytes = bytes,
.ratelimit = ratelimit,
.cb = cb,
+ .cb_opaque = cb_opaque,
.max_workers = max_workers ?: BLOCK_COPY_MAX_WORKERS,
.max_chunk = max_chunk,
};
--
2.21.0
