Re: [Qemu-devel] [PATCH v13 07/14] block/mirror: Improve progress report

[Max Reitz <mreitz@redhat.com>]

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 <mreitz@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>   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.

Max