[Qemu-devel] [PATCH 4/5] backup: simplify non-dirty bits progress processing

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

Set fake progress for non-dirty clusters in copy_bitmap initialization,
to. It simplifies code and allows further refactoring.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---

Motivation (some of these paragraphs may be needed in commit message...)

1. Current behavior is not good: setting fake progress leads to progress
hopping, so actually for sparse disk progress say nothing. We just move
all these hops to the beginning.

2. Big hop in the beginning is possible now too: if there is a hole at
the beginning of virtual disk.

3. Now, there are some excuses for old behavior in comparison to new one:
backup progress is linear, cluster by cluster. But in future cluster
copying will be done by several coroutine-workers, several requests in
parallel, so it will make no sense to publish fake progress by parts in
parallel with non-sequential requests.

4. Actually it's just a point of view: when we are actually skip clusters?
If go through disk sequentially, it's logical to say, that we skip clusters
between copied portions to the left and to the right of them. But even know
copying progress is not sequential because of write notifiers.
If copying is async, non-sequential, we can say in the very beginning, that
we skip these clusters and do not think about them later.

So, I don't want to maintain portions of fake progress in the new backup
architecture. I understand, that this patch will change user's view
of backup progress, but formally it doesn't change: progress hops are just
moved to the beginning. Progress was meaningless for incremental backup and
it remains meaningless.

But what should we do with progress, really? It's obvious, that we need at
least one more field for block job status, for example, "skipped", which
would be amount of bytes which are not really processed but skipped by the
block job. So, more real progress may be calculated (by management tool) like

(job.offset - job.skipped) / (job.len - job.skipped)

And this progress will be more precise if information about skipped bytes
is provided earlier, and current patch is close to this idea.

So, if you agree, I can make a patch for 'skipped' too, but it actually not
related to the series, so I hope that current patch will be merged anyway.
(I need it for backup refactoring, not for accurate progress)

 block/backup.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 07f4ae846b..52df7bb19e 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -369,7 +369,6 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
     int64_t offset;
     int64_t cluster;
     int64_t end;
-    int64_t last_cluster = -1;
     BdrvDirtyBitmapIter *dbi;
 
     granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
@@ -380,12 +379,6 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
     while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
         cluster = offset / job->cluster_size;
 
-        /* Fake progress updates for any clusters we skipped */
-        if (cluster != last_cluster + 1) {
-            job->common.offset += ((cluster - last_cluster - 1) *
-                                   job->cluster_size);
-        }
-
         for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
             do {
                 if (yield_and_check(job)) {
@@ -407,14 +400,6 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
         if (granularity < job->cluster_size) {
             bdrv_set_dirty_iter(dbi, cluster * job->cluster_size);
         }
-
-        last_cluster = cluster - 1;
-    }
-
-    /* Play some final catchup with the progress meter */
-    end = DIV_ROUND_UP(job->common.len, job->cluster_size);
-    if (last_cluster + 1 < end) {
-        job->common.offset += ((end - last_cluster - 1) * job->cluster_size);
     }
 
 out:
@@ -456,6 +441,9 @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
         bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
     }
 
+    job->common.offset = job->common.len -
+                         hbitmap_count(job->copy_bitmap) * job->cluster_size;
+
     bdrv_dirty_iter_free(dbi);
 }
 
-- 
2.11.1