[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: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] block: mirror - split out part of mirror_run() |
Date: |
Mon, 28 Sep 2015 16:17:54 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 28/09/2015 05:29, Jeff Cody wrote:
> 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;
> + }
You might as well replace the gotos with returns (either "return ret;"
or "return s->ret;") and the breaks with "return 0;").
> +
> + 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;
> +
> +}
> +
> static void mirror_free_init(MirrorBlockJob *s)
> {
> int granularity = s->granularity;
> @@ -485,103 +594,8 @@ static void coroutine_fn mirror_run(void *opaque)
> }
>
> bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
> - 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);
> - }
> + ret = mirror_do_iteration(s, last_pause_ns);
>
> immediate_exit:
> if (s->in_flight > 0) {
>
- [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