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 <[email protected]>
> Reviewed-by: Vladimir Sementsov-Ogievskiy<[email protected]>
> CC: Stefan Hajnoczi <[email protected]>
> CC: Fam Zheng <[email protected]>
> CC: Kevin Wolf <[email protected]>
> CC: Max Reitz <[email protected]>
> CC: Jeff Cody <[email protected]>
> CC: Eric Blake <[email protected]>
> ---
>  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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to