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

[Vladimir Sementsov-Ogievskiy]

On 02.03.23 13:00, Fiona Ebner wrote:
> 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:

Is it worth reporting to user?

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

Still I'm not sure that we need new status values. I.e. if you need some new 
ones, you should explain the case and why existing information is not enough.

Especially when documentation of existing things is unclear, its better to 
start from improving it. And when we understand what len and offset means for 
mirror, it would probably be enough.


--
Best regards,
Vladimir