[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v13 07/14] block/mirror: Improve progress report
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v13 07/14] block/mirror: Improve progress report |
Date: |
Thu, 23 Oct 2014 14:03:19 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 23.10.2014 um 13:09 hat Max Reitz geschrieben:
> On 2014-10-23 at 12:52, Kevin Wolf wrote:
> >Am 22.10.2014 um 14:51 hat Max Reitz geschrieben:
> >>Instead of taking the total length of the block device as the block
> >>job's length, use the number of dirty sectors. The progress is now the
> >>number of sectors mirrored to the target block device. Note that this
> >>may result in the job's length increasing during operation, which is
> >>however in fact desirable.
> >More importantly, because it might surprise management tools, is that
> >the progress (as in offset/len) can actually decrease now.
> >
> >I can't say whether that creates any problem, I'll rely on Eric's
> >Reviewed-by for that.
>
> Eric said, it would rather solve problems.
>
> >>Signed-off-by: Max Reitz <address@hidden>
> >>Reviewed-by: Eric Blake <address@hidden>
> >>---
> >> block/mirror.c | 34 ++++++++++++++++++++++------------
> >> 1 file changed, 22 insertions(+), 12 deletions(-)
> >>@@ -409,6 +416,12 @@ static void coroutine_fn mirror_run(void *opaque)
> >> }
> >> cnt = bdrv_get_dirty_count(bs, 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;
> >Isn't s->sectors_in_flight still contained in cnt? If I understand
> >correctly, sectors are only marked as clean at the same time as
> >s->sectors_in_flight is decremented again.
>
> As far as I see, bdrv_reset_dirty() is called in mirror_iteration()
> right before s->sectors_in_flight is incremented.
You're right, I confused it with the other bitmap operations in
mirror_iteration_done(). So:
Reviewed-by: Kevin Wolf <address@hidden>
[Qemu-devel] [PATCH v13 09/14] qemu-img: Empty image after commit, Max Reitz, 2014/10/22
[Qemu-devel] [PATCH v13 10/14] qemu-img: Enable progress output for commit, Max Reitz, 2014/10/22