From 6d49d3a859b0f19226dbb0df5e7f50267b42f45c Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Fri, 21 Feb 2020 11:25:19 +0000 Subject: luks: extract qcrypto_block_calculate_payload_offset() The qcow2 .bdrv_measure() code calculates the crypto payload offset. This logic really belongs in crypto/block.c where it can be reused by other image formats. The "luks" block driver will need this same logic in order to implement .bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset() function now. Signed-off-by: Stefan Hajnoczi Reviewed-by: Max Reitz Message-Id: <20200221112522.1497712-2-stefanha@redhat.com> Signed-off-by: Max Reitz --- include/crypto/block.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'include') diff --git a/include/crypto/block.h b/include/crypto/block.h index d49d2c2da9..c77ccaf9c0 100644 --- a/include/crypto/block.h +++ b/include/crypto/block.h @@ -145,6 +145,26 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options, Error **errp); +/** + * qcrypto_block_calculate_payload_offset: + * @create_opts: the encryption options + * @optprefix: name prefix for options + * @len: output for number of header bytes before payload + * @errp: pointer to a NULL-initialized error object + * + * Calculate the number of header bytes before the payload in an encrypted + * storage volume. The header is an area before the payload that is reserved + * for encryption metadata. + * + * Returns: true on success, false on error + */ +bool +qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts, + const char *optprefix, + size_t *len, + Error **errp); + + /** * qcrypto_block_get_info: * @block: the block encryption object @@ -269,5 +289,7 @@ uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block); void qcrypto_block_free(QCryptoBlock *block); G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlock, qcrypto_block_free) +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlockCreateOptions, + qapi_free_QCryptoBlockCreateOptions) #endif /* QCRYPTO_BLOCK_H */ -- cgit v1.2.3-55-g7522 From 01fe1ca945345d3dc420d70c69488143dc0451b1 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 11 Mar 2020 13:29:56 +0300 Subject: job: refactor progress to separate object We need it in separate to pass to the block-copy object in the next commit. Cc: qemu-stable@nongnu.org Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich Reviewed-by: Max Reitz Message-Id: <20200311103004.7649-2-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- blockjob.c | 16 ++++++------ include/qemu/job.h | 11 ++------ include/qemu/progress_meter.h | 58 +++++++++++++++++++++++++++++++++++++++++++ job-qmp.c | 4 +-- job.c | 6 ++--- qemu-img.c | 6 ++--- 6 files changed, 76 insertions(+), 25 deletions(-) create mode 100644 include/qemu/progress_meter.h (limited to 'include') diff --git a/blockjob.c b/blockjob.c index 5d63b1e89d..fc850312c1 100644 --- a/blockjob.c +++ b/blockjob.c @@ -299,8 +299,8 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp) info->device = g_strdup(job->job.id); info->busy = atomic_read(&job->job.busy); info->paused = job->job.pause_count > 0; - info->offset = job->job.progress_current; - info->len = job->job.progress_total; + info->offset = job->job.progress.current; + info->len = job->job.progress.total; info->speed = job->speed; info->io_status = job->iostatus; info->ready = job_is_ready(&job->job), @@ -330,8 +330,8 @@ static void block_job_event_cancelled(Notifier *n, void *opaque) qapi_event_send_block_job_cancelled(job_type(&job->job), job->job.id, - job->job.progress_total, - job->job.progress_current, + job->job.progress.total, + job->job.progress.current, job->speed); } @@ -350,8 +350,8 @@ static void block_job_event_completed(Notifier *n, void *opaque) qapi_event_send_block_job_completed(job_type(&job->job), job->job.id, - job->job.progress_total, - job->job.progress_current, + job->job.progress.total, + job->job.progress.current, job->speed, !!msg, msg); @@ -379,8 +379,8 @@ static void block_job_event_ready(Notifier *n, void *opaque) qapi_event_send_block_job_ready(job_type(&job->job), job->job.id, - job->job.progress_total, - job->job.progress_current, + job->job.progress.total, + job->job.progress.current, job->speed); } diff --git a/include/qemu/job.h b/include/qemu/job.h index bd59cd8944..32aabb1c60 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -28,6 +28,7 @@ #include "qapi/qapi-types-job.h" #include "qemu/queue.h" +#include "qemu/progress_meter.h" #include "qemu/coroutine.h" #include "block/aio.h" @@ -117,15 +118,7 @@ typedef struct Job { /** True if this job should automatically dismiss itself */ bool auto_dismiss; - /** - * Current progress. The unit is arbitrary as long as the ratio between - * progress_current and progress_total represents the estimated percentage - * of work already done. - */ - int64_t progress_current; - - /** Estimated progress_current value at the completion of the job */ - int64_t progress_total; + ProgressMeter progress; /** * Return code from @run and/or @prepare callback(s). diff --git a/include/qemu/progress_meter.h b/include/qemu/progress_meter.h new file mode 100644 index 0000000000..9a23ff071c --- /dev/null +++ b/include/qemu/progress_meter.h @@ -0,0 +1,58 @@ +/* + * Helper functionality for some process progress tracking. + * + * Copyright (c) 2011 IBM Corp. + * Copyright (c) 2012, 2018 Red Hat, Inc. + * Copyright (c) 2020 Virtuozzo International GmbH + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef QEMU_PROGRESS_METER_H +#define QEMU_PROGRESS_METER_H + +typedef struct ProgressMeter { + /** + * Current progress. The unit is arbitrary as long as the ratio between + * current and total represents the estimated percentage + * of work already done. + */ + uint64_t current; + + /** Estimated current value at the completion of the process */ + uint64_t total; +} ProgressMeter; + +static inline void progress_work_done(ProgressMeter *pm, uint64_t done) +{ + pm->current += done; +} + +static inline void progress_set_remaining(ProgressMeter *pm, uint64_t remaining) +{ + pm->total = pm->current + remaining; +} + +static inline void progress_increase_remaining(ProgressMeter *pm, + uint64_t delta) +{ + pm->total += delta; +} + +#endif /* QEMU_PROGRESS_METER_H */ diff --git a/job-qmp.c b/job-qmp.c index fbfed25a00..fecc939ebd 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -143,8 +143,8 @@ static JobInfo *job_query_single(Job *job, Error **errp) .id = g_strdup(job->id), .type = job_type(job), .status = job->status, - .current_progress = job->progress_current, - .total_progress = job->progress_total, + .current_progress = job->progress.current, + .total_progress = job->progress.total, .has_error = !!job->err, .error = job->err ? \ g_strdup(error_get_pretty(job->err)) : NULL, diff --git a/job.c b/job.c index 04409b40aa..134a07b92e 100644 --- a/job.c +++ b/job.c @@ -369,17 +369,17 @@ void job_unref(Job *job) void job_progress_update(Job *job, uint64_t done) { - job->progress_current += done; + progress_work_done(&job->progress, done); } void job_progress_set_remaining(Job *job, uint64_t remaining) { - job->progress_total = job->progress_current + remaining; + progress_set_remaining(&job->progress, remaining); } void job_progress_increase_remaining(Job *job, uint64_t delta) { - job->progress_total += delta; + progress_increase_remaining(&job->progress, delta); } void job_event_cancelled(Job *job) diff --git a/qemu-img.c b/qemu-img.c index 7b7087dd60..afddf33f08 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -884,9 +884,9 @@ static void run_block_job(BlockJob *job, Error **errp) do { float progress = 0.0f; aio_poll(aio_context, true); - if (job->job.progress_total) { - progress = (float)job->job.progress_current / - job->job.progress_total * 100.f; + if (job->job.progress.total) { + progress = (float)job->job.progress.current / + job->job.progress.total * 100.f; } qemu_progress_print(progress, 0); } while (!job_is_ready(&job->job) && !job_is_completed(&job->job)); -- cgit v1.2.3-55-g7522 From d0ebeca14a585f352938062ef8ddde47fe4d39f9 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 11 Mar 2020 13:29:57 +0300 Subject: block/block-copy: fix progress calculation Assume we have two regions, A and B, and region B is in-flight now, region A is not yet touched, but it is unallocated and should be skipped. Correspondingly, as progress we have total = A + B current = 0 If we reset unallocated region A and call progress_reset_callback, it will calculate 0 bytes dirty in the bitmap and call job_progress_set_remaining, which will set total = current + 0 = 0 + 0 = 0 So, B bytes are actually removed from total accounting. When job finishes we'll have total = 0 current = B , which doesn't sound good. This is because we didn't considered in-flight bytes, actually when calculating remaining, we should have set (in_flight + dirty_bytes) as remaining, not only dirty_bytes. To fix it, let's refactor progress calculation, moving it to block-copy itself instead of fixing callback. And, of course, track in_flight bytes count. We still have to keep one callback, to maintain backup job bytes_read calculation, but it will go on soon, when we turn the whole backup process into one block_copy call. Cc: qemu-stable@nongnu.org Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich Message-Id: <20200311103004.7649-3-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- block/backup.c | 13 ++----------- block/block-copy.c | 16 ++++++++++++---- include/block/block-copy.h | 15 +++++---------- 3 files changed, 19 insertions(+), 25 deletions(-) (limited to 'include') diff --git a/block/backup.c b/block/backup.c index 1383e219f5..8694e0394b 100644 --- a/block/backup.c +++ b/block/backup.c @@ -57,15 +57,6 @@ static void backup_progress_bytes_callback(int64_t bytes, void *opaque) BackupBlockJob *s = opaque; s->bytes_read += bytes; - job_progress_update(&s->common.job, bytes); -} - -static void backup_progress_reset_callback(void *opaque) -{ - BackupBlockJob *s = opaque; - uint64_t estimate = bdrv_get_dirty_count(s->bcs->copy_bitmap); - - job_progress_set_remaining(&s->common.job, estimate); } static int coroutine_fn backup_do_cow(BackupBlockJob *job, @@ -464,8 +455,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, job->cluster_size = cluster_size; job->len = len; - block_copy_set_callbacks(bcs, backup_progress_bytes_callback, - backup_progress_reset_callback, job); + block_copy_set_progress_callback(bcs, backup_progress_bytes_callback, job); + block_copy_set_progress_meter(bcs, &job->common.job.progress); /* 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 79798a1567..e2d7b3b887 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -127,17 +127,20 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, return s; } -void block_copy_set_callbacks( +void block_copy_set_progress_callback( BlockCopyState *s, ProgressBytesCallbackFunc progress_bytes_callback, - ProgressResetCallbackFunc progress_reset_callback, void *progress_opaque) { s->progress_bytes_callback = progress_bytes_callback; - s->progress_reset_callback = progress_reset_callback; s->progress_opaque = progress_opaque; } +void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm) +{ + s->progress = pm; +} + /* * block_copy_do_copy * @@ -269,7 +272,9 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s, if (!ret) { bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); - s->progress_reset_callback(s->progress_opaque); + progress_set_remaining(s->progress, + bdrv_get_dirty_count(s->copy_bitmap) + + s->in_flight_bytes); } *count = bytes; @@ -331,15 +336,18 @@ int coroutine_fn block_copy(BlockCopyState *s, trace_block_copy_process(s, start); bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start); + s->in_flight_bytes += chunk_end - start; co_get_from_shres(s->mem, chunk_end - start); ret = block_copy_do_copy(s, start, chunk_end, error_is_read); co_put_to_shres(s->mem, chunk_end - start); + s->in_flight_bytes -= chunk_end - start; if (ret < 0) { bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start); break; } + progress_work_done(s->progress, chunk_end - start); s->progress_bytes_callback(chunk_end - start, s->progress_opaque); start = chunk_end; ret = 0; diff --git a/include/block/block-copy.h b/include/block/block-copy.h index 0a161724d7..9def00068c 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -26,7 +26,6 @@ typedef struct BlockCopyInFlightReq { } BlockCopyInFlightReq; typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque); -typedef void (*ProgressResetCallbackFunc)(void *opaque); typedef struct BlockCopyState { /* * BdrvChild objects are not owned or managed by block-copy. They are @@ -36,6 +35,7 @@ typedef struct BlockCopyState { BdrvChild *source; BdrvChild *target; BdrvDirtyBitmap *copy_bitmap; + int64_t in_flight_bytes; int64_t cluster_size; bool use_copy_range; int64_t copy_size; @@ -60,15 +60,9 @@ typedef struct BlockCopyState { */ bool skip_unallocated; + ProgressMeter *progress; /* progress_bytes_callback: called when some copying progress is done. */ ProgressBytesCallbackFunc progress_bytes_callback; - - /* - * progress_reset_callback: called when some bytes reset from copy_bitmap - * (see @skip_unallocated above). The callee is assumed to recalculate how - * many bytes remain based on the dirty bit count of copy_bitmap. - */ - ProgressResetCallbackFunc progress_reset_callback; void *progress_opaque; SharedResource *mem; @@ -79,12 +73,13 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, BdrvRequestFlags write_flags, Error **errp); -void block_copy_set_callbacks( +void block_copy_set_progress_callback( BlockCopyState *s, ProgressBytesCallbackFunc progress_bytes_callback, - ProgressResetCallbackFunc progress_reset_callback, void *progress_opaque); +void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm); + void block_copy_state_free(BlockCopyState *s); int64_t block_copy_reset_unallocated(BlockCopyState *s, -- cgit v1.2.3-55-g7522 From dafaf13593de240724a210e72da66f9d162735c3 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 11 Mar 2020 13:30:01 +0300 Subject: block/block-copy: refactor interfaces to use bytes instead of end We have a lot of "chunk_end - start" invocations, let's switch to bytes/cur_bytes scheme instead. While being here, improve check on block_copy_do_copy parameters to not overflow when calculating nbytes and use int64_t for bytes in block_copy for consistency. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich Reviewed-by: Max Reitz Message-Id: <20200311103004.7649-7-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- block/block-copy.c | 78 ++++++++++++++++++++++++---------------------- include/block/block-copy.h | 6 ++-- 2 files changed, 44 insertions(+), 40 deletions(-) (limited to 'include') diff --git a/block/block-copy.c b/block/block-copy.c index 251d415a2c..4c947e548b 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -26,12 +26,12 @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s, int64_t start, - int64_t end) + int64_t bytes) { BlockCopyInFlightReq *req; QLIST_FOREACH(req, &s->inflight_reqs, list) { - if (end > req->start_byte && start < req->end_byte) { + if (start + bytes > req->start && start < req->start + req->bytes) { return req; } } @@ -41,21 +41,21 @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s, static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s, int64_t start, - int64_t end) + int64_t bytes) { BlockCopyInFlightReq *req; - while ((req = find_conflicting_inflight_req(s, start, end))) { + while ((req = find_conflicting_inflight_req(s, start, bytes))) { qemu_co_queue_wait(&req->wait_queue, NULL); } } static void block_copy_inflight_req_begin(BlockCopyState *s, BlockCopyInFlightReq *req, - int64_t start, int64_t end) + int64_t start, int64_t bytes) { - req->start_byte = start; - req->end_byte = end; + req->start = start; + req->bytes = bytes; qemu_co_queue_init(&req->wait_queue); QLIST_INSERT_HEAD(&s->inflight_reqs, req, list); } @@ -153,24 +153,28 @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm) /* * block_copy_do_copy * - * Do copy of cluser-aligned chunk. @end is allowed to exceed s->len only to - * cover last cluster when s->len is not aligned to clusters. + * Do copy of cluster-aligned chunk. Requested region is allowed to exceed + * s->len only to cover last cluster when s->len is not aligned to clusters. * * No sync here: nor bitmap neighter intersecting requests handling, only copy. * * Returns 0 on success. */ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, - int64_t start, int64_t end, + int64_t start, int64_t bytes, bool zeroes, bool *error_is_read) { int ret; - int nbytes = MIN(end, s->len) - start; + int64_t nbytes = MIN(start + bytes, s->len) - start; void *bounce_buffer = NULL; + assert(start >= 0 && bytes > 0 && INT64_MAX - start >= bytes); assert(QEMU_IS_ALIGNED(start, s->cluster_size)); - assert(QEMU_IS_ALIGNED(end, s->cluster_size)); - assert(end < s->len || end == QEMU_ALIGN_UP(s->len, s->cluster_size)); + assert(QEMU_IS_ALIGNED(bytes, s->cluster_size)); + assert(start < s->len); + assert(start + bytes <= s->len || + start + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size)); + assert(nbytes < INT_MAX); if (zeroes) { ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags & @@ -354,11 +358,10 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s, } int coroutine_fn block_copy(BlockCopyState *s, - int64_t start, uint64_t bytes, + int64_t start, int64_t bytes, bool *error_is_read) { int ret = 0; - int64_t end = bytes + start; /* bytes */ BlockCopyInFlightReq req; /* @@ -369,32 +372,32 @@ int coroutine_fn block_copy(BlockCopyState *s, bdrv_get_aio_context(s->target->bs)); assert(QEMU_IS_ALIGNED(start, s->cluster_size)); - assert(QEMU_IS_ALIGNED(end, s->cluster_size)); + assert(QEMU_IS_ALIGNED(bytes, s->cluster_size)); block_copy_wait_inflight_reqs(s, start, bytes); - block_copy_inflight_req_begin(s, &req, start, end); + block_copy_inflight_req_begin(s, &req, start, bytes); - while (start < end) { - int64_t next_zero, chunk_end, status_bytes; + while (bytes) { + int64_t next_zero, cur_bytes, status_bytes; if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) { trace_block_copy_skip(s, start); start += s->cluster_size; + bytes -= s->cluster_size; continue; /* already copied */ } - chunk_end = MIN(end, start + s->copy_size); + cur_bytes = MIN(bytes, s->copy_size); next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start, - chunk_end - start); + cur_bytes); if (next_zero >= 0) { assert(next_zero > start); /* start is dirty */ - assert(next_zero < chunk_end); /* no need to do MIN() */ - chunk_end = next_zero; + assert(next_zero < start + cur_bytes); /* no need to do MIN() */ + cur_bytes = next_zero - start; } - ret = block_copy_block_status(s, start, chunk_end - start, - &status_bytes); + ret = block_copy_block_status(s, start, cur_bytes, &status_bytes); if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) { bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes); progress_set_remaining(s->progress, @@ -402,30 +405,31 @@ int coroutine_fn block_copy(BlockCopyState *s, s->in_flight_bytes); trace_block_copy_skip_range(s, start, status_bytes); start += status_bytes; + bytes -= status_bytes; continue; } - chunk_end = MIN(chunk_end, start + status_bytes); + cur_bytes = MIN(cur_bytes, status_bytes); trace_block_copy_process(s, start); - bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start); - s->in_flight_bytes += chunk_end - start; + bdrv_reset_dirty_bitmap(s->copy_bitmap, start, cur_bytes); + s->in_flight_bytes += cur_bytes; - co_get_from_shres(s->mem, chunk_end - start); - ret = block_copy_do_copy(s, start, chunk_end, ret & BDRV_BLOCK_ZERO, + co_get_from_shres(s->mem, cur_bytes); + ret = block_copy_do_copy(s, start, cur_bytes, ret & BDRV_BLOCK_ZERO, error_is_read); - co_put_to_shres(s->mem, chunk_end - start); - s->in_flight_bytes -= chunk_end - start; + co_put_to_shres(s->mem, cur_bytes); + s->in_flight_bytes -= cur_bytes; if (ret < 0) { - bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start); + bdrv_set_dirty_bitmap(s->copy_bitmap, start, cur_bytes); break; } - progress_work_done(s->progress, chunk_end - start); - s->progress_bytes_callback(chunk_end - start, s->progress_opaque); - start = chunk_end; - ret = 0; + progress_work_done(s->progress, cur_bytes); + s->progress_bytes_callback(cur_bytes, s->progress_opaque); + start += cur_bytes; + bytes -= cur_bytes; } block_copy_inflight_req_end(&req); diff --git a/include/block/block-copy.h b/include/block/block-copy.h index 9def00068c..7fd36e528b 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -19,8 +19,8 @@ #include "qemu/co-shared-resource.h" typedef struct BlockCopyInFlightReq { - int64_t start_byte; - int64_t end_byte; + int64_t start; + int64_t bytes; QLIST_ENTRY(BlockCopyInFlightReq) list; CoQueue wait_queue; /* coroutines blocked on this request */ } BlockCopyInFlightReq; @@ -85,7 +85,7 @@ void block_copy_state_free(BlockCopyState *s); int64_t block_copy_reset_unallocated(BlockCopyState *s, int64_t offset, int64_t *count); -int coroutine_fn block_copy(BlockCopyState *s, int64_t start, uint64_t bytes, +int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes, bool *error_is_read); #endif /* BLOCK_COPY_H */ -- cgit v1.2.3-55-g7522 From 8719091f9d591595ea6876d25de9c0956951c513 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 11 Mar 2020 13:30:02 +0300 Subject: block/block-copy: rename start to offset in interfaces offset/bytes pair is more usual naming in block layer, let's use it. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich Reviewed-by: Max Reitz Message-Id: <20200311103004.7649-8-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- block/block-copy.c | 82 +++++++++++++++++++++++----------------------- include/block/block-copy.h | 4 +-- 2 files changed, 43 insertions(+), 43 deletions(-) (limited to 'include') diff --git a/block/block-copy.c b/block/block-copy.c index 4c947e548b..8b91fa0b06 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -25,13 +25,13 @@ #define BLOCK_COPY_MAX_MEM (128 * MiB) static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s, - int64_t start, + int64_t offset, int64_t bytes) { BlockCopyInFlightReq *req; QLIST_FOREACH(req, &s->inflight_reqs, list) { - if (start + bytes > req->start && start < req->start + req->bytes) { + if (offset + bytes > req->offset && offset < req->offset + req->bytes) { return req; } } @@ -40,21 +40,21 @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s, } static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s, - int64_t start, + int64_t offset, int64_t bytes) { BlockCopyInFlightReq *req; - while ((req = find_conflicting_inflight_req(s, start, bytes))) { + while ((req = find_conflicting_inflight_req(s, offset, bytes))) { qemu_co_queue_wait(&req->wait_queue, NULL); } } static void block_copy_inflight_req_begin(BlockCopyState *s, BlockCopyInFlightReq *req, - int64_t start, int64_t bytes) + int64_t offset, int64_t bytes) { - req->start = start; + req->offset = offset; req->bytes = bytes; qemu_co_queue_init(&req->wait_queue); QLIST_INSERT_HEAD(&s->inflight_reqs, req, list); @@ -161,26 +161,26 @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm) * Returns 0 on success. */ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, - int64_t start, int64_t bytes, + int64_t offset, int64_t bytes, bool zeroes, bool *error_is_read) { int ret; - int64_t nbytes = MIN(start + bytes, s->len) - start; + int64_t nbytes = MIN(offset + bytes, s->len) - offset; void *bounce_buffer = NULL; - assert(start >= 0 && bytes > 0 && INT64_MAX - start >= bytes); - assert(QEMU_IS_ALIGNED(start, s->cluster_size)); + assert(offset >= 0 && bytes > 0 && INT64_MAX - offset >= bytes); + assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(bytes, s->cluster_size)); - assert(start < s->len); - assert(start + bytes <= s->len || - start + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size)); + assert(offset < s->len); + assert(offset + bytes <= s->len || + offset + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size)); assert(nbytes < INT_MAX); if (zeroes) { - ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags & + ret = bdrv_co_pwrite_zeroes(s->target, offset, nbytes, s->write_flags & ~BDRV_REQ_WRITE_COMPRESSED); if (ret < 0) { - trace_block_copy_write_zeroes_fail(s, start, ret); + trace_block_copy_write_zeroes_fail(s, offset, ret); if (error_is_read) { *error_is_read = false; } @@ -189,10 +189,10 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, } if (s->use_copy_range) { - ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes, + ret = bdrv_co_copy_range(s->source, offset, s->target, offset, nbytes, 0, s->write_flags); if (ret < 0) { - trace_block_copy_copy_range_fail(s, start, ret); + trace_block_copy_copy_range_fail(s, offset, ret); s->use_copy_range = false; s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER); /* Fallback to read+write with allocated buffer */ @@ -228,19 +228,19 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, bounce_buffer = qemu_blockalign(s->source->bs, nbytes); - ret = bdrv_co_pread(s->source, start, nbytes, bounce_buffer, 0); + ret = bdrv_co_pread(s->source, offset, nbytes, bounce_buffer, 0); if (ret < 0) { - trace_block_copy_read_fail(s, start, ret); + trace_block_copy_read_fail(s, offset, ret); if (error_is_read) { *error_is_read = true; } goto out; } - ret = bdrv_co_pwrite(s->target, start, nbytes, bounce_buffer, + ret = bdrv_co_pwrite(s->target, offset, nbytes, bounce_buffer, s->write_flags); if (ret < 0) { - trace_block_copy_write_fail(s, start, ret); + trace_block_copy_write_fail(s, offset, ret); if (error_is_read) { *error_is_read = false; } @@ -358,7 +358,7 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s, } int coroutine_fn block_copy(BlockCopyState *s, - int64_t start, int64_t bytes, + int64_t offset, int64_t bytes, bool *error_is_read) { int ret = 0; @@ -371,64 +371,64 @@ int coroutine_fn block_copy(BlockCopyState *s, assert(bdrv_get_aio_context(s->source->bs) == bdrv_get_aio_context(s->target->bs)); - assert(QEMU_IS_ALIGNED(start, s->cluster_size)); + assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(bytes, s->cluster_size)); - block_copy_wait_inflight_reqs(s, start, bytes); - block_copy_inflight_req_begin(s, &req, start, bytes); + block_copy_wait_inflight_reqs(s, offset, bytes); + block_copy_inflight_req_begin(s, &req, offset, bytes); while (bytes) { int64_t next_zero, cur_bytes, status_bytes; - if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) { - trace_block_copy_skip(s, start); - start += s->cluster_size; + if (!bdrv_dirty_bitmap_get(s->copy_bitmap, offset)) { + trace_block_copy_skip(s, offset); + offset += s->cluster_size; bytes -= s->cluster_size; continue; /* already copied */ } cur_bytes = MIN(bytes, s->copy_size); - next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start, + next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset, cur_bytes); if (next_zero >= 0) { - assert(next_zero > start); /* start is dirty */ - assert(next_zero < start + cur_bytes); /* no need to do MIN() */ - cur_bytes = next_zero - start; + assert(next_zero > offset); /* offset is dirty */ + assert(next_zero < offset + cur_bytes); /* no need to do MIN() */ + cur_bytes = next_zero - offset; } - ret = block_copy_block_status(s, start, cur_bytes, &status_bytes); + ret = block_copy_block_status(s, offset, cur_bytes, &status_bytes); if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) { - bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes); + bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, status_bytes); progress_set_remaining(s->progress, bdrv_get_dirty_count(s->copy_bitmap) + s->in_flight_bytes); - trace_block_copy_skip_range(s, start, status_bytes); - start += status_bytes; + trace_block_copy_skip_range(s, offset, status_bytes); + offset += status_bytes; bytes -= status_bytes; continue; } cur_bytes = MIN(cur_bytes, status_bytes); - trace_block_copy_process(s, start); + trace_block_copy_process(s, offset); - bdrv_reset_dirty_bitmap(s->copy_bitmap, start, cur_bytes); + bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, cur_bytes); s->in_flight_bytes += cur_bytes; co_get_from_shres(s->mem, cur_bytes); - ret = block_copy_do_copy(s, start, cur_bytes, ret & BDRV_BLOCK_ZERO, + ret = block_copy_do_copy(s, offset, cur_bytes, ret & BDRV_BLOCK_ZERO, error_is_read); co_put_to_shres(s->mem, cur_bytes); s->in_flight_bytes -= cur_bytes; if (ret < 0) { - bdrv_set_dirty_bitmap(s->copy_bitmap, start, cur_bytes); + bdrv_set_dirty_bitmap(s->copy_bitmap, offset, cur_bytes); break; } progress_work_done(s->progress, cur_bytes); s->progress_bytes_callback(cur_bytes, s->progress_opaque); - start += cur_bytes; + offset += cur_bytes; bytes -= cur_bytes; } diff --git a/include/block/block-copy.h b/include/block/block-copy.h index 7fd36e528b..b76efb736f 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -19,7 +19,7 @@ #include "qemu/co-shared-resource.h" typedef struct BlockCopyInFlightReq { - int64_t start; + int64_t offset; int64_t bytes; QLIST_ENTRY(BlockCopyInFlightReq) list; CoQueue wait_queue; /* coroutines blocked on this request */ @@ -85,7 +85,7 @@ void block_copy_state_free(BlockCopyState *s); int64_t block_copy_reset_unallocated(BlockCopyState *s, int64_t offset, int64_t *count); -int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes, +int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes, bool *error_is_read); #endif /* BLOCK_COPY_H */ -- cgit v1.2.3-55-g7522 From 397f4e9d83e9c0000905f0a988ba1aeda162571c Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 11 Mar 2020 13:30:04 +0300 Subject: block/block-copy: hide structure definitions Hide structure definitions and add explicit API instead, to keep an eye on the scope of the shared fields. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich Reviewed-by: Max Reitz Message-Id: <20200311103004.7649-10-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- block/backup-top.c | 6 +++-- block/backup.c | 25 ++++++++++---------- block/block-copy.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++ include/block/block-copy.h | 52 ++++------------------------------------ 4 files changed, 80 insertions(+), 62 deletions(-) (limited to 'include') diff --git a/block/backup-top.c b/block/backup-top.c index 1bfb360bd3..3b50c06e2c 100644 --- a/block/backup-top.c +++ b/block/backup-top.c @@ -38,6 +38,7 @@ typedef struct BDRVBackupTopState { BlockCopyState *bcs; BdrvChild *target; bool active; + int64_t cluster_size; } BDRVBackupTopState; static coroutine_fn int backup_top_co_preadv( @@ -57,8 +58,8 @@ static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset, return 0; } - off = QEMU_ALIGN_DOWN(offset, s->bcs->cluster_size); - end = QEMU_ALIGN_UP(offset + bytes, s->bcs->cluster_size); + off = QEMU_ALIGN_DOWN(offset, s->cluster_size); + end = QEMU_ALIGN_UP(offset + bytes, s->cluster_size); return block_copy(s->bcs, off, end - off, NULL); } @@ -238,6 +239,7 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source, goto fail; } + state->cluster_size = cluster_size; state->bcs = block_copy_state_new(top->backing, state->target, cluster_size, write_flags, &local_err); if (local_err) { diff --git a/block/backup.c b/block/backup.c index 8694e0394b..7430ca5883 100644 --- a/block/backup.c +++ b/block/backup.c @@ -102,7 +102,7 @@ static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret) if (ret < 0 && job->bitmap_mode == BITMAP_SYNC_MODE_ALWAYS) { /* If we failed and synced, merge in the bits we didn't copy: */ - bdrv_dirty_bitmap_merge_internal(bm, job->bcs->copy_bitmap, + bdrv_dirty_bitmap_merge_internal(bm, block_copy_dirty_bitmap(job->bcs), NULL, true); } } @@ -145,7 +145,8 @@ void backup_do_checkpoint(BlockJob *job, Error **errp) return; } - bdrv_set_dirty_bitmap(backup_job->bcs->copy_bitmap, 0, backup_job->len); + bdrv_set_dirty_bitmap(block_copy_dirty_bitmap(backup_job->bcs), 0, + backup_job->len); } static BlockErrorAction backup_error_action(BackupBlockJob *job, @@ -190,7 +191,7 @@ static int coroutine_fn backup_loop(BackupBlockJob *job) BdrvDirtyBitmapIter *bdbi; int ret = 0; - bdbi = bdrv_dirty_iter_new(job->bcs->copy_bitmap); + 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)) { @@ -210,14 +211,14 @@ static int coroutine_fn backup_loop(BackupBlockJob *job) return ret; } -static void backup_init_copy_bitmap(BackupBlockJob *job) +static void backup_init_bcs_bitmap(BackupBlockJob *job) { bool ret; uint64_t estimate; + BdrvDirtyBitmap *bcs_bitmap = block_copy_dirty_bitmap(job->bcs); if (job->sync_mode == MIRROR_SYNC_MODE_BITMAP) { - ret = bdrv_dirty_bitmap_merge_internal(job->bcs->copy_bitmap, - job->sync_bitmap, + ret = bdrv_dirty_bitmap_merge_internal(bcs_bitmap, job->sync_bitmap, NULL, true); assert(ret); } else { @@ -226,12 +227,12 @@ static void backup_init_copy_bitmap(BackupBlockJob *job) * We can't hog the coroutine to initialize this thoroughly. * Set a flag and resume work when we are able to yield safely. */ - job->bcs->skip_unallocated = true; + block_copy_set_skip_unallocated(job->bcs, true); } - bdrv_set_dirty_bitmap(job->bcs->copy_bitmap, 0, job->len); + bdrv_set_dirty_bitmap(bcs_bitmap, 0, job->len); } - estimate = bdrv_get_dirty_count(job->bcs->copy_bitmap); + estimate = bdrv_get_dirty_count(bcs_bitmap); job_progress_set_remaining(&job->common.job, estimate); } @@ -240,7 +241,7 @@ static int coroutine_fn backup_run(Job *job, Error **errp) BackupBlockJob *s = container_of(job, BackupBlockJob, common.job); int ret = 0; - backup_init_copy_bitmap(s); + backup_init_bcs_bitmap(s); if (s->sync_mode == MIRROR_SYNC_MODE_TOP) { int64_t offset = 0; @@ -259,12 +260,12 @@ static int coroutine_fn backup_run(Job *job, Error **errp) offset += count; } - s->bcs->skip_unallocated = false; + block_copy_set_skip_unallocated(s->bcs, false); } if (s->sync_mode == MIRROR_SYNC_MODE_NONE) { /* - * All bits are set in copy_bitmap to allow any cluster to be copied. + * All bits are set in bcs bitmap to allow any cluster to be copied. * This does not actually require them to be copied. */ while (!job_is_cancelled(job)) { diff --git a/block/block-copy.c b/block/block-copy.c index 44a64a94c8..05227e18bf 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -24,6 +24,55 @@ #define BLOCK_COPY_MAX_BUFFER (1 * MiB) #define BLOCK_COPY_MAX_MEM (128 * MiB) +typedef struct BlockCopyInFlightReq { + int64_t offset; + int64_t bytes; + QLIST_ENTRY(BlockCopyInFlightReq) list; + CoQueue wait_queue; /* coroutines blocked on this request */ +} BlockCopyInFlightReq; + +typedef struct BlockCopyState { + /* + * BdrvChild objects are not owned or managed by block-copy. They are + * provided by block-copy user and user is responsible for appropriate + * permissions on these children. + */ + BdrvChild *source; + BdrvChild *target; + BdrvDirtyBitmap *copy_bitmap; + int64_t in_flight_bytes; + int64_t cluster_size; + bool use_copy_range; + int64_t copy_size; + uint64_t len; + QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs; + + BdrvRequestFlags write_flags; + + /* + * skip_unallocated: + * + * Used by sync=top jobs, which first scan the source node for unallocated + * areas and clear them in the copy_bitmap. During this process, the bitmap + * is thus not fully initialized: It may still have bits set for areas that + * are unallocated and should actually not be copied. + * + * This is indicated by skip_unallocated. + * + * In this case, block_copy() will query the source’s allocation status, + * skip unallocated regions, clear them in the copy_bitmap, and invoke + * block_copy_reset_unallocated() every time it does. + */ + bool skip_unallocated; + + ProgressMeter *progress; + /* progress_bytes_callback: called when some copying progress is done. */ + ProgressBytesCallbackFunc progress_bytes_callback; + void *progress_opaque; + + SharedResource *mem; +} BlockCopyState; + static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s, int64_t offset, int64_t bytes) @@ -517,3 +566,13 @@ int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes, return ret; } + +BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s) +{ + return s->copy_bitmap; +} + +void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip) +{ + s->skip_unallocated = skip; +} diff --git a/include/block/block-copy.h b/include/block/block-copy.h index b76efb736f..aac85e1488 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -18,55 +18,8 @@ #include "block/block.h" #include "qemu/co-shared-resource.h" -typedef struct BlockCopyInFlightReq { - int64_t offset; - int64_t bytes; - QLIST_ENTRY(BlockCopyInFlightReq) list; - CoQueue wait_queue; /* coroutines blocked on this request */ -} BlockCopyInFlightReq; - typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque); -typedef struct BlockCopyState { - /* - * BdrvChild objects are not owned or managed by block-copy. They are - * provided by block-copy user and user is responsible for appropriate - * permissions on these children. - */ - BdrvChild *source; - BdrvChild *target; - BdrvDirtyBitmap *copy_bitmap; - int64_t in_flight_bytes; - int64_t cluster_size; - bool use_copy_range; - int64_t copy_size; - uint64_t len; - QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs; - - BdrvRequestFlags write_flags; - - /* - * skip_unallocated: - * - * Used by sync=top jobs, which first scan the source node for unallocated - * areas and clear them in the copy_bitmap. During this process, the bitmap - * is thus not fully initialized: It may still have bits set for areas that - * are unallocated and should actually not be copied. - * - * This is indicated by skip_unallocated. - * - * In this case, block_copy() will query the source’s allocation status, - * skip unallocated regions, clear them in the copy_bitmap, and invoke - * block_copy_reset_unallocated() every time it does. - */ - bool skip_unallocated; - - ProgressMeter *progress; - /* progress_bytes_callback: called when some copying progress is done. */ - ProgressBytesCallbackFunc progress_bytes_callback; - void *progress_opaque; - - SharedResource *mem; -} BlockCopyState; +typedef struct BlockCopyState BlockCopyState; BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, int64_t cluster_size, @@ -88,4 +41,7 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s, int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes, bool *error_is_read); +BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s); +void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip); + #endif /* BLOCK_COPY_H */ -- cgit v1.2.3-55-g7522