[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] block: mirror - split out part of mirror_ru
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] block: mirror - split out part of mirror_run() |
Date: |
Mon, 28 Sep 2015 16:47:36 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 28.09.2015 um 05:29 hat Jeff Cody geschrieben:
> This is code relocation, to pull the part of mirror_run() that
> calls mirror_iteration out into a separate function.
>
> Signed-off-by: Jeff Cody <address@hidden>
> ---
> block/mirror.c | 206
> ++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 110 insertions(+), 96 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 8b3e89b..405e5c4 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -318,6 +318,115 @@ static uint64_t coroutine_fn
> mirror_iteration(MirrorBlockJob *s)
> return delay_ns;
> }
>
> +static int mirror_do_iteration(MirrorBlockJob *s, uint64_t last_pause_ns)
> +{
> + int ret;
> +
> + BlockDriverState *bs = s->common.bs;
> +
> + for (;;) {
> + uint64_t delay_ns = 0;
> + int64_t cnt;
> + bool should_complete;
> +
> + if (s->ret < 0) {
> + ret = s->ret;
> + goto immediate_exit;
> + }
> +
> + cnt = bdrv_get_dirty_count(s->dirty_bitmap);
> + /* s->common.offset contains the number of bytes already processed so
> + * far, cnt is the number of dirty sectors remaining and
> + * s->sectors_in_flight is the number of sectors currently being
> + * processed; together those are the current total operation length
> */
> + s->common.len = s->common.offset +
> + (cnt + s->sectors_in_flight) * BDRV_SECTOR_SIZE;
> +
> + /* 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.
> + */
> + if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - last_pause_ns <
> 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)) {
> + trace_mirror_yield(s, s->in_flight, s->buf_free_count, cnt);
> + s->waiting_for_io = true;
> + qemu_coroutine_yield();
> + s->waiting_for_io = false;
> + continue;
> + } else if (cnt != 0) {
> + delay_ns = mirror_iteration(s);
> + }
> + }
> +
> + should_complete = false;
> + if (s->in_flight == 0 && cnt == 0) {
> + trace_mirror_before_flush(s);
> + ret = bdrv_flush(s->target);
> + if (ret < 0) {
> + if (mirror_error_action(s, false, -ret) ==
> + BLOCK_ERROR_ACTION_REPORT) {
> + goto immediate_exit;
> + }
> + } else {
> + /* We're out of the streaming phase. From now on, if the job
> + * is cancelled we will actually complete all pending I/O and
> + * report completion. This way, block-job-cancel will leave
> + * the target in a consistent state.
> + */
> + if (!s->synced) {
> + block_job_event_ready(&s->common);
> + s->synced = true;
> + }
> +
> + should_complete = s->should_complete ||
> + block_job_is_cancelled(&s->common);
> + cnt = bdrv_get_dirty_count(s->dirty_bitmap);
> + }
> + }
> +
> + if (cnt == 0 && should_complete) {
> + /* The dirty bitmap is not updated while operations are pending.
> + * If we're about to exit, wait for pending operations before
> + * calling bdrv_get_dirty_count(bs), or we may exit while the
> + * source has dirty data to copy!
> + *
> + * Note that I/O can be submitted by the guest while
> + * mirror_populate runs.
> + */
> + trace_mirror_before_drain(s, cnt);
> + bdrv_drain(bs);
> + cnt = bdrv_get_dirty_count(s->dirty_bitmap);
> + }
> +
> + ret = 0;
> + trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
> + if (!s->synced) {
> + block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns);
> + if (block_job_is_cancelled(&s->common)) {
> + break;
> + }
> + } else if (!should_complete) {
> + delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
> + block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns);
> + } else if (cnt == 0) {
> + /* The two disks are in sync. Exit and report successful
> + * completion.
> + */
> + assert(QLIST_EMPTY(&bs->tracked_requests));
> + s->common.cancelled = false;
> + break;
> + }
> + last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> + }
> +
> +immediate_exit:
> + return ret;
> +
> +}
Did you leave this additional empty line there intentionally?
With or without it removed, and also with or without Paolo's suggestion
regarding goto vs. return applied:
Reviewed-by: Kevin Wolf <address@hidden>
- [Qemu-devel] [PATCH 0/3] block: mirror - Write zeroes for unallocated sectors if no zero init, Jeff Cody, 2015/09/27
- [Qemu-devel] [PATCH 1/3] block: allow creation of detached dirty bitmaps, Jeff Cody, 2015/09/27
- [Qemu-devel] [PATCH 2/3] block: mirror - split out part of mirror_run(), Jeff Cody, 2015/09/27
- [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Jeff Cody, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Kevin Wolf, 2015/09/29
- Re: [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Kevin Wolf, 2015/09/29