Re: [Qemu-block] [PATCH v2 3/7] mirror: create mirror_dirty_init helper for mirror_run

[John Snow]

On 07/07/2016 05:35 AM, Denis V. Lunev wrote:
> The code inside the helper will be extended in the next patch. mirror_run
> itself is overbloated at the moment.
> 
> Signed-off-by: Denis V. Lunev <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy<address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> CC: Fam Zheng <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Max Reitz <address@hidden>
> CC: Jeff Cody <address@hidden>
> CC: Eric Blake <address@hidden>
> ---
>  block/mirror.c | 87 
> ++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 51 insertions(+), 36 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 9850e54..9beda4b 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -512,19 +512,64 @@ static void mirror_exit(BlockJob *job, void *opaque)
>      bdrv_unref(src);
>  }
>  
> +static int mirror_dirty_init(MirrorBlockJob *s)

Does this need to be a coroutine_fn because it is executed within a
coroutine context?

> +{
> +    int64_t sector_num, end;
> +    BlockDriverState *base = s->base;
> +    BlockDriverState *bs = blk_bs(s->common.blk);
> +    BlockDriverState *target_bs = blk_bs(s->target);
> +    bool mark_all_dirty = base == NULL && !bdrv_has_zero_init(target_bs);
> +    uint64_t last_pause_ns;
> +    int ret, n;
> +
> +    end = s->bdev_length / BDRV_SECTOR_SIZE;
> +
> +    last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +
> +    /* First part, loop on the sectors and initialize the dirty bitmap.  */
> +    for (sector_num = 0; sector_num < end; ) {
> +        /* Just to make sure we are not exceeding int limit. */
> +        int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
> +                             end - sector_num);
> +        int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +
> +        if (now - last_pause_ns > SLICE_TIME) {
> +            last_pause_ns = now;
> +            block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
> +        } else {
> +            block_job_pause_point(&s->common);
> +        }
> +
> +        if (block_job_is_cancelled(&s->common)) {
> +            return 0;
> +        }
> +
> +        ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +
> +        assert(n > 0);
> +        if (ret == 1 || mark_all_dirty) {
> +            bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
> +        }
> +        sector_num += n;
> +    }
> +    return 0;
> +}
> +
>  static void coroutine_fn mirror_run(void *opaque)
>  {
>      MirrorBlockJob *s = opaque;
>      MirrorExitData *data;
>      BlockDriverState *bs = blk_bs(s->common.blk);
>      BlockDriverState *target_bs = blk_bs(s->target);
> -    int64_t sector_num, end, length;
> +    int64_t length;
>      uint64_t last_pause_ns;
>      BlockDriverInfo bdi;
>      char backing_filename[2]; /* we only need 2 characters because we are 
> only
>                                   checking for a NULL string */
>      int ret = 0;
> -    int n;
>      int target_cluster_size = BDRV_SECTOR_SIZE;
>  
>      if (block_job_is_cancelled(&s->common)) {
> @@ -566,7 +611,6 @@ static void coroutine_fn mirror_run(void *opaque)
>      s->target_cluster_sectors = target_cluster_size >> BDRV_SECTOR_BITS;
>      s->max_iov = MIN(bs->bl.max_iov, target_bs->bl.max_iov);
>  
> -    end = s->bdev_length / BDRV_SECTOR_SIZE;
>      s->buf = qemu_try_blockalign(bs, s->buf_size);
>      if (s->buf == NULL) {
>          ret = -ENOMEM;
> @@ -575,43 +619,14 @@ static void coroutine_fn mirror_run(void *opaque)
>  
>      mirror_free_init(s);
>  
> -    last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>      if (!s->is_none_mode) {
> -        /* First part, loop on the sectors and initialize the dirty bitmap.  
> */
> -        BlockDriverState *base = s->base;
> -        bool mark_all_dirty = s->base == NULL && 
> !bdrv_has_zero_init(target_bs);
> -
> -        for (sector_num = 0; sector_num < end; ) {
> -            /* Just to make sure we are not exceeding int limit. */
> -            int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
> -                                 end - sector_num);
> -            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> -
> -            if (now - last_pause_ns > SLICE_TIME) {
> -                last_pause_ns = now;
> -                block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
> -            } else {
> -                block_job_pause_point(&s->common);
> -            }
> -
> -            if (block_job_is_cancelled(&s->common)) {
> -                goto immediate_exit;
> -            }
> -
> -            ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, 
> &n);
> -
> -            if (ret < 0) {
> -                goto immediate_exit;
> -            }
> -
> -            assert(n > 0);
> -            if (ret == 1 || mark_all_dirty) {
> -                bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
> -            }
> -            sector_num += n;
> +        ret = mirror_dirty_init(s);
> +        if (ret < 0 || block_job_is_cancelled(&s->common)) {
> +            goto immediate_exit;
>          }
>      }
>  
> +    last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>      bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
>      for (;;) {
>          uint64_t delay_ns = 0;
> 

-- 
—js