On 16.07.19 02:01, John Snow wrote: > Presently, If sync=TOP is selected, we mark the entire bitmap as dirty. > In the write notifier handler, we dutifully copy out such regions. > > Fix this in three parts: > > 1. Mark the bitmap as being initialized before the first yield. > 2. After the first yield but before the backup loop, interrogate the > allocation status asynchronously and initialize the bitmap. > 3. Teach the write notifier to interrogate allocation status if it is > invoked during bitmap initialization. > > As an effect of this patch, the job progress for TOP backups > now behaves like this: > > - total progress starts at bdrv_length. > - As allocation status is interrogated, total progress decreases. > - As blocks are copied, current progress increases. > > Taken together, the floor and ceiling move to meet each other. > > Signed-off-by: John Snow <js...@redhat.com> > --- > block/backup.c | 78 ++++++++++++++++++++++++++++++++++++++++------ > block/trace-events | 1 + > 2 files changed, 70 insertions(+), 9 deletions(-)
Looks good to me but for a seemingly unrelated change: > diff --git a/block/backup.c b/block/backup.c > index b407d57954..e28fd23f6a 100644 > --- a/block/backup.c > +++ b/block/backup.c [...] > @@ -507,10 +565,12 @@ static int coroutine_fn backup_run(Job *job, Error > **errp) > * notify callback service CoW requests. */ > job_yield(job); > } > + ret = -ECANCELED; This one. This doesn’t look like it belongs in this patch, and I’m not even sure it’s correct. Being cancelled is the normal state for sync=none, so I suppose it is correct to just return 0 then. Max > } else { > ret = backup_loop(s); > } > > + out: > notifier_with_return_remove(&s->before_write); > > /* wait until pending backup_do_cow() calls have completed */
signature.asc
Description: OpenPGP digital signature