Re: [PATCH 8/9] mirror: return the remaining dirty bytes upon query

[Fiona Ebner <f.ebner@proxmox.com>]

Am 01.03.23 um 17:31 schrieb Vladimir Sementsov-Ogievskiy:
> On 24.02.23 17:48, Fiona Ebner wrote:
>> This can be used by management applications starting with a job in
>> background mode to determine when the switch to active mode should
>> happen.
>>
>> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>   block/mirror.c       | 1 +
>>   qapi/block-core.json | 4 +++-
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 02b5bd8bd2..ac83309b82 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -1259,6 +1259,7 @@ static void mirror_query(BlockJob *job,
>> BlockJobInfo *info)
>>         info->u.mirror = (BlockJobInfoMirror) {
>>           .actively_synced = s->actively_synced,
>> +        .remaining_dirty = bdrv_get_dirty_count(s->dirty_bitmap),
> 
> Doesn't it duplicate info->len - info->offset in meaning?
> 

Essentially yes, apart from the in-flight bytes:
>         job_progress_set_remaining(&s->common.job,
>                                    s->bytes_in_flight + cnt +
>                                    s->active_write_bytes_in_flight);

Should I rather use that value (and rename it to e.g. data_remaining to
be more similar to data_sent from 9/9)?

But I'd argue the same way as in 9/9: it's not transparent to users what
offset and len mean for the mirror job, because their documentation is
for a generic block job. E.g. len is documented to be able to change in
both directions while the job runs.

Best Regards,
Fiona