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

[Eric Blake]

On 07/07/2016 03: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(-)
> 

> +static int mirror_dirty_init(MirrorBlockJob *s)
> +{
> +    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);

You now have a new variable tracking one point in time...


>  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;

...and a different variable tracking another point in time.  Depending
on timing, you can end up doing nearly twice as much work between pause
points as intended as you transition out of the helper initialization
and back into this function.  I think the solution is to pass
last_pause_ns by reference as one of the parameters to the helper
initialization, rather than tracking two separate points in time.

Otherwise looks like a sane refactoring.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature