On 06.03.20 08:38, Vladimir Sementsov-Ogievskiy wrote: > Assume we have two regions, A and B, and region B is in-flight now, > region A is not yet touched, but it is unallocated and should be > skipped. > > Correspondingly, as progress we have > > total = A + B > current = 0 > > If we reset unallocated region A and call progress_reset_callback, > it will calculate 0 bytes dirty in the bitmap and call > job_progress_set_remaining, which will set > > total = current + 0 = 0 + 0 = 0 > > So, B bytes are actually removed from total accounting. When job > finishes we'll have > > total = 0 > current = B > > , which doesn't sound good. > > This is because we didn't considered in-flight bytes, actually when > calculating remaining, we should have set (in_flight + dirty_bytes) > as remaining, not only dirty_bytes. > > To fix it, let's refactor progress calculation, moving it to block-copy > itself instead of fixing callback. And, of course, track in_flight > bytes count. > > We still have to keep one callback, to maintain backup job bytes_read > calculation, but it will go on soon, when we turn the whole backup > process into one block_copy call. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Reviewed-by: Andrey Shinkevich <andrey.shinkev...@virtuozzo.com> > --- > include/block/block-copy.h | 14 +++++--------- > block/backup.c | 13 ++----------- > block/block-copy.c | 16 ++++++++++++---- > 3 files changed, 19 insertions(+), 24 deletions(-)
Looks good, but I suppose we should also drop the ProgressResetCallbackFunc type. Max
signature.asc
Description: OpenPGP digital signature
