From 05df8a6a2b4e36e8d69de2130e616d5ac28e8837 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 18 Jan 2018 18:08:22 +0100 Subject: blockjob: Wrappers for progress counter access Block job drivers are not expected to mess with the internals of the BlockJob object, so provide wrapper functions for one of the cases where they still do it: Updating the progress counter. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: John Snow --- block/commit.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'block/commit.c') diff --git a/block/commit.c b/block/commit.c index 1432baeef4..50b191c980 100644 --- a/block/commit.c +++ b/block/commit.c @@ -146,21 +146,21 @@ static void coroutine_fn commit_run(void *opaque) int64_t n = 0; /* bytes */ void *buf = NULL; int bytes_written = 0; - int64_t base_len; + int64_t len, base_len; - ret = s->common.len = blk_getlength(s->top); - - if (s->common.len < 0) { + ret = len = blk_getlength(s->top); + if (len < 0) { goto out; } + block_job_progress_set_remaining(&s->common, len); ret = base_len = blk_getlength(s->base); if (base_len < 0) { goto out; } - if (base_len < s->common.len) { - ret = blk_truncate(s->base, s->common.len, PREALLOC_MODE_OFF, NULL); + if (base_len < len) { + ret = blk_truncate(s->base, len, PREALLOC_MODE_OFF, NULL); if (ret) { goto out; } @@ -168,7 +168,7 @@ static void coroutine_fn commit_run(void *opaque) buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE); - for (offset = 0; offset < s->common.len; offset += n) { + for (offset = 0; offset < len; offset += n) { bool copy; /* Note that even when no rate limit is applied we need to yield @@ -198,7 +198,7 @@ static void coroutine_fn commit_run(void *opaque) } } /* Publish progress */ - s->common.offset += n; + block_job_progress_update(&s->common, n); if (copy && s->common.speed) { delay_ns = ratelimit_calculate_delay(&s->limit, n); -- cgit 1.4.1 From f05fee508f538ca262d2ab19bcd8772196efe848 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 18 Jan 2018 20:20:24 +0100 Subject: blockjob: Move RateLimit to BlockJob Every block job has a RateLimit, and they all do the exact same thing with it, so it should be common infrastructure. Move the struct field for a start. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: John Snow --- block/backup.c | 5 ++--- block/commit.c | 5 ++--- block/mirror.c | 6 +++--- block/stream.c | 5 ++--- include/block/blockjob.h | 4 ++++ 5 files changed, 13 insertions(+), 12 deletions(-) (limited to 'block/commit.c') diff --git a/block/backup.c b/block/backup.c index 5d95805472..7585c4391e 100644 --- a/block/backup.c +++ b/block/backup.c @@ -35,7 +35,6 @@ typedef struct BackupBlockJob { /* bitmap for sync=incremental */ BdrvDirtyBitmap *sync_bitmap; MirrorSyncMode sync_mode; - RateLimit limit; BlockdevOnError on_source_error; BlockdevOnError on_target_error; CoRwlock flush_rwlock; @@ -199,7 +198,7 @@ static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER, "speed"); return; } - ratelimit_set_speed(&s->limit, speed, SLICE_TIME); + ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); } static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret) @@ -346,7 +345,7 @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job) * (without, VM does not reboot) */ if (job->common.speed) { - uint64_t delay_ns = ratelimit_calculate_delay(&job->limit, + uint64_t delay_ns = ratelimit_calculate_delay(&job->common.limit, job->bytes_read); job->bytes_read = 0; block_job_sleep_ns(&job->common, delay_ns); diff --git a/block/commit.c b/block/commit.c index 50b191c980..beec5d0ad6 100644 --- a/block/commit.c +++ b/block/commit.c @@ -35,7 +35,6 @@ enum { typedef struct CommitBlockJob { BlockJob common; - RateLimit limit; BlockDriverState *commit_top_bs; BlockBackend *top; BlockBackend *base; @@ -201,7 +200,7 @@ static void coroutine_fn commit_run(void *opaque) block_job_progress_update(&s->common, n); if (copy && s->common.speed) { - delay_ns = ratelimit_calculate_delay(&s->limit, n); + delay_ns = ratelimit_calculate_delay(&s->common.limit, n); } else { delay_ns = 0; } @@ -225,7 +224,7 @@ static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER, "speed"); return; } - ratelimit_set_speed(&s->limit, speed, SLICE_TIME); + ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); } static const BlockJobDriver commit_job_driver = { diff --git a/block/mirror.c b/block/mirror.c index 56a7ce2f55..702c139368 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -36,7 +36,6 @@ typedef struct MirrorBuffer { typedef struct MirrorBlockJob { BlockJob common; - RateLimit limit; BlockBackend *target; BlockDriverState *mirror_top_bs; BlockDriverState *source; @@ -450,7 +449,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) offset += io_bytes; nb_chunks -= DIV_ROUND_UP(io_bytes, s->granularity); if (s->common.speed) { - delay_ns = ratelimit_calculate_delay(&s->limit, io_bytes_acct); + delay_ns = ratelimit_calculate_delay(&s->common.limit, + io_bytes_acct); } } return delay_ns; @@ -916,7 +916,7 @@ static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER, "speed"); return; } - ratelimit_set_speed(&s->limit, speed, SLICE_TIME); + ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); } static void mirror_complete(BlockJob *job, Error **errp) diff --git a/block/stream.c b/block/stream.c index 8369852bda..a1d4768c2e 100644 --- a/block/stream.c +++ b/block/stream.c @@ -33,7 +33,6 @@ enum { typedef struct StreamBlockJob { BlockJob common; - RateLimit limit; BlockDriverState *base; BlockdevOnError on_error; char *backing_file_str; @@ -189,7 +188,7 @@ static void coroutine_fn stream_run(void *opaque) /* Publish progress */ block_job_progress_update(&s->common, n); if (copy && s->common.speed) { - delay_ns = ratelimit_calculate_delay(&s->limit, n); + delay_ns = ratelimit_calculate_delay(&s->common.limit, n); } else { delay_ns = 0; } @@ -219,7 +218,7 @@ static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER, "speed"); return; } - ratelimit_set_speed(&s->limit, speed, SLICE_TIME); + ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); } static const BlockJobDriver stream_job_driver = { diff --git a/include/block/blockjob.h b/include/block/blockjob.h index a2cc52233b..22bf418209 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -27,6 +27,7 @@ #define BLOCKJOB_H #include "block/block.h" +#include "qemu/ratelimit.h" typedef struct BlockJobDriver BlockJobDriver; typedef struct BlockJobTxn BlockJobTxn; @@ -118,6 +119,9 @@ typedef struct BlockJob { /** Speed that was set with @block_job_set_speed. */ int64_t speed; + /** Rate limiting data structure for implementing @speed. */ + RateLimit limit; + /** The completion function that will be called when the job completes. */ BlockCompletionFunc *cb; -- cgit 1.4.1 From 18bb69287ea522ab696e1bea818b93e5eaa85745 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 18 Jan 2018 20:25:40 +0100 Subject: blockjob: Implement block_job_set_speed() centrally All block job drivers support .set_speed and all of them duplicate the same code to implement it. Move that code to blockjob.c and remove the now useless callback. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: John Snow --- block/backup.c | 13 ------------- block/commit.c | 14 -------------- block/mirror.c | 26 ++++++-------------------- block/stream.c | 14 -------------- blockjob.c | 12 ++++-------- include/block/blockjob.h | 2 ++ include/block/blockjob_int.h | 3 --- 7 files changed, 12 insertions(+), 72 deletions(-) (limited to 'block/commit.c') diff --git a/block/backup.c b/block/backup.c index 7585c4391e..8468fd9f94 100644 --- a/block/backup.c +++ b/block/backup.c @@ -27,7 +27,6 @@ #include "qemu/error-report.h" #define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16) -#define SLICE_TIME 100000000ULL /* ns */ typedef struct BackupBlockJob { BlockJob common; @@ -190,17 +189,6 @@ static int coroutine_fn backup_before_write_notify( return backup_do_cow(job, req->offset, req->bytes, NULL, true); } -static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp) -{ - BackupBlockJob *s = container_of(job, BackupBlockJob, common); - - if (speed < 0) { - error_setg(errp, QERR_INVALID_PARAMETER, "speed"); - return; - } - ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); -} - static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret) { BdrvDirtyBitmap *bm; @@ -540,7 +528,6 @@ static const BlockJobDriver backup_job_driver = { .instance_size = sizeof(BackupBlockJob), .job_type = BLOCK_JOB_TYPE_BACKUP, .start = backup_run, - .set_speed = backup_set_speed, .commit = backup_commit, .abort = backup_abort, .clean = backup_clean, diff --git a/block/commit.c b/block/commit.c index beec5d0ad6..46cbeaec3e 100644 --- a/block/commit.c +++ b/block/commit.c @@ -31,8 +31,6 @@ enum { COMMIT_BUFFER_SIZE = 512 * 1024, /* in bytes */ }; -#define SLICE_TIME 100000000ULL /* ns */ - typedef struct CommitBlockJob { BlockJob common; BlockDriverState *commit_top_bs; @@ -216,21 +214,9 @@ out: block_job_defer_to_main_loop(&s->common, commit_complete, data); } -static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp) -{ - CommitBlockJob *s = container_of(job, CommitBlockJob, common); - - if (speed < 0) { - error_setg(errp, QERR_INVALID_PARAMETER, "speed"); - return; - } - ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); -} - static const BlockJobDriver commit_job_driver = { .instance_size = sizeof(CommitBlockJob), .job_type = BLOCK_JOB_TYPE_COMMIT, - .set_speed = commit_set_speed, .start = commit_run, }; diff --git a/block/mirror.c b/block/mirror.c index 702c139368..6955d68804 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -22,7 +22,6 @@ #include "qemu/ratelimit.h" #include "qemu/bitmap.h" -#define SLICE_TIME 100000000ULL /* ns */ #define MAX_IN_FLIGHT 16 #define MAX_IO_BYTES (1 << 20) /* 1 Mb */ #define DEFAULT_MIRROR_BUF_SIZE (MAX_IN_FLIGHT * MAX_IO_BYTES) @@ -596,7 +595,7 @@ static void mirror_throttle(MirrorBlockJob *s) { int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); - if (now - s->last_pause_ns > SLICE_TIME) { + if (now - s->last_pause_ns > BLOCK_JOB_SLICE_TIME) { s->last_pause_ns = now; block_job_sleep_ns(&s->common, 0); } else { @@ -799,11 +798,10 @@ static void coroutine_fn mirror_run(void *opaque) /* Note that even when no rate limit is applied we need to yield * periodically with no pending I/O so that bdrv_drain_all() returns. - * We do so every SLICE_TIME nanoseconds, or when there is an error, - * or when the source is clean, whichever comes first. - */ + * We do so every BLKOCK_JOB_SLICE_TIME nanoseconds, or when there is + * an error, or when the source is clean, whichever comes first. */ delta = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - s->last_pause_ns; - if (delta < SLICE_TIME && + if (delta < BLOCK_JOB_SLICE_TIME && s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) { if (s->in_flight >= MAX_IN_FLIGHT || s->buf_free_count == 0 || (cnt == 0 && s->in_flight > 0)) { @@ -869,7 +867,8 @@ static void coroutine_fn mirror_run(void *opaque) ret = 0; if (s->synced && !should_complete) { - delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0); + delay_ns = (s->in_flight == 0 && + cnt == 0 ? BLOCK_JOB_SLICE_TIME : 0); } trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); block_job_sleep_ns(&s->common, delay_ns); @@ -908,17 +907,6 @@ immediate_exit: block_job_defer_to_main_loop(&s->common, mirror_exit, data); } -static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp) -{ - MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); - - if (speed < 0) { - error_setg(errp, QERR_INVALID_PARAMETER, "speed"); - return; - } - ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); -} - static void mirror_complete(BlockJob *job, Error **errp) { MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); @@ -1003,7 +991,6 @@ static void mirror_drain(BlockJob *job) static const BlockJobDriver mirror_job_driver = { .instance_size = sizeof(MirrorBlockJob), .job_type = BLOCK_JOB_TYPE_MIRROR, - .set_speed = mirror_set_speed, .start = mirror_run, .complete = mirror_complete, .pause = mirror_pause, @@ -1014,7 +1001,6 @@ static const BlockJobDriver mirror_job_driver = { static const BlockJobDriver commit_active_job_driver = { .instance_size = sizeof(MirrorBlockJob), .job_type = BLOCK_JOB_TYPE_COMMIT, - .set_speed = mirror_set_speed, .start = mirror_run, .complete = mirror_complete, .pause = mirror_pause, diff --git a/block/stream.c b/block/stream.c index a1d4768c2e..797d7c4f21 100644 --- a/block/stream.c +++ b/block/stream.c @@ -29,8 +29,6 @@ enum { STREAM_BUFFER_SIZE = 512 * 1024, /* in bytes */ }; -#define SLICE_TIME 100000000ULL /* ns */ - typedef struct StreamBlockJob { BlockJob common; BlockDriverState *base; @@ -210,21 +208,9 @@ out: block_job_defer_to_main_loop(&s->common, stream_complete, data); } -static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp) -{ - StreamBlockJob *s = container_of(job, StreamBlockJob, common); - - if (speed < 0) { - error_setg(errp, QERR_INVALID_PARAMETER, "speed"); - return; - } - ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME); -} - static const BlockJobDriver stream_job_driver = { .instance_size = sizeof(StreamBlockJob), .job_type = BLOCK_JOB_TYPE_STREAM, - .set_speed = stream_set_speed, .start = stream_run, }; diff --git a/blockjob.c b/blockjob.c index ebc26a5245..2f4b768338 100644 --- a/blockjob.c +++ b/blockjob.c @@ -659,22 +659,18 @@ static bool block_job_timer_pending(BlockJob *job) void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) { - Error *local_err = NULL; int64_t old_speed = job->speed; - if (!job->driver->set_speed) { - error_setg(errp, QERR_UNSUPPORTED); - return; - } if (block_job_apply_verb(job, BLOCK_JOB_VERB_SET_SPEED, errp)) { return; } - job->driver->set_speed(job, speed, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (speed < 0) { + error_setg(errp, QERR_INVALID_PARAMETER, "speed"); return; } + ratelimit_set_speed(&job->limit, speed, BLOCK_JOB_SLICE_TIME); + job->speed = speed; if (speed && speed <= old_speed) { return; diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 22bf418209..82f52f4b14 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -29,6 +29,8 @@ #include "block/block.h" #include "qemu/ratelimit.h" +#define BLOCK_JOB_SLICE_TIME 100000000ULL /* ns */ + typedef struct BlockJobDriver BlockJobDriver; typedef struct BlockJobTxn BlockJobTxn; diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index d5a515de9b..ad510d5a0c 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -41,9 +41,6 @@ struct BlockJobDriver { /** String describing the operation, part of query-block-jobs QMP API */ BlockJobType job_type; - /** Optional callback for job types that support setting a speed limit */ - void (*set_speed)(BlockJob *job, int64_t speed, Error **errp); - /** Mandatory: Entrypoint for the Coroutine. */ CoroutineEntry *start; -- cgit 1.4.1 From dee81d5111ff0e24ac63ab0dbbd19e84c2f87904 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 18 Jan 2018 21:19:38 +0100 Subject: blockjob: Introduce block_job_ratelimit_get_delay() This gets us rid of more direct accesses to BlockJob fields from the job drivers. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: John Snow --- block/backup.c | 18 +++++++----------- block/commit.c | 4 ++-- block/mirror.c | 5 +---- block/stream.c | 4 ++-- blockjob.c | 9 +++++++++ include/block/blockjob_int.h | 8 ++++++++ 6 files changed, 29 insertions(+), 19 deletions(-) (limited to 'block/commit.c') diff --git a/block/backup.c b/block/backup.c index 8468fd9f94..cfdb6ecdf5 100644 --- a/block/backup.c +++ b/block/backup.c @@ -325,21 +325,17 @@ static void backup_complete(BlockJob *job, void *opaque) static bool coroutine_fn yield_and_check(BackupBlockJob *job) { + uint64_t delay_ns; + if (block_job_is_cancelled(&job->common)) { return true; } - /* we need to yield so that bdrv_drain_all() returns. - * (without, VM does not reboot) - */ - if (job->common.speed) { - uint64_t delay_ns = ratelimit_calculate_delay(&job->common.limit, - job->bytes_read); - job->bytes_read = 0; - block_job_sleep_ns(&job->common, delay_ns); - } else { - block_job_sleep_ns(&job->common, 0); - } + /* 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; + block_job_sleep_ns(&job->common, delay_ns); if (block_job_is_cancelled(&job->common)) { return true; diff --git a/block/commit.c b/block/commit.c index 46cbeaec3e..ba5df6aa0a 100644 --- a/block/commit.c +++ b/block/commit.c @@ -197,8 +197,8 @@ static void coroutine_fn commit_run(void *opaque) /* Publish progress */ block_job_progress_update(&s->common, n); - if (copy && s->common.speed) { - delay_ns = ratelimit_calculate_delay(&s->common.limit, n); + if (copy) { + delay_ns = block_job_ratelimit_get_delay(&s->common, n); } else { delay_ns = 0; } diff --git a/block/mirror.c b/block/mirror.c index 6955d68804..6aa38db114 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -447,10 +447,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) assert(io_bytes); offset += io_bytes; nb_chunks -= DIV_ROUND_UP(io_bytes, s->granularity); - if (s->common.speed) { - delay_ns = ratelimit_calculate_delay(&s->common.limit, - io_bytes_acct); - } + delay_ns = block_job_ratelimit_get_delay(&s->common, io_bytes_acct); } return delay_ns; } diff --git a/block/stream.c b/block/stream.c index 797d7c4f21..df9660d2fc 100644 --- a/block/stream.c +++ b/block/stream.c @@ -185,8 +185,8 @@ static void coroutine_fn stream_run(void *opaque) /* Publish progress */ block_job_progress_update(&s->common, n); - if (copy && s->common.speed) { - delay_ns = ratelimit_calculate_delay(&s->common.limit, n); + if (copy) { + delay_ns = block_job_ratelimit_get_delay(&s->common, n); } else { delay_ns = 0; } diff --git a/blockjob.c b/blockjob.c index 2f4b768338..04416f11cd 100644 --- a/blockjob.c +++ b/blockjob.c @@ -680,6 +680,15 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) block_job_enter_cond(job, block_job_timer_pending); } +int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n) +{ + if (!job->speed) { + return 0; + } + + return ratelimit_calculate_delay(&job->limit, n); +} + void block_job_complete(BlockJob *job, Error **errp) { /* Should not be reachable via external interface for internal jobs */ diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index ad510d5a0c..62ec964d09 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -165,6 +165,14 @@ void block_job_sleep_ns(BlockJob *job, int64_t ns); */ void block_job_yield(BlockJob *job); +/** + * block_job_ratelimit_get_delay: + * + * Calculate and return delay for the next request in ns. See the documentation + * of ratelimit_calculate_delay() for details. + */ +int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n); + /** * block_job_early_fail: * @bs: The block device. -- cgit 1.4.1