[Qemu-devel] [PATCH v3 7/7] block/backup: merge duplicated logic into backup_do_cow

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

backup_cow_with_offload and backup_cow_with_bounce_buffer contains a
lot of duplicated logic. Move it into backup_do_cow.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 97 ++++++++++++++++++++------------------------------
 1 file changed, 38 insertions(+), 59 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 65f7212c85..0ac31c2760 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -104,87 +104,61 @@ static void cow_request_end(CowRequest *req)
  * error occurred, return a negative error number
  *
  * @bounce_buffer is assumed to enough to store
- * MIN(BACKUP_MAX_BOUNCE_BUFFER, @end - @start) bytes
+ * MIN(BACKUP_MAX_BOUNCE_BUFFER, @bytes) bytes
  */
-static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
-                                                      int64_t start,
-                                                      int64_t end,
-                                                      bool is_write_notifier,
-                                                      bool *error_is_read,
-                                                      void *bounce_buffer)
+static int coroutine_fn backup_cow_with_bounce_buffer(
+        BackupBlockJob *job, int64_t offset, int64_t bytes,
+        BdrvRequestFlags read_flags, bool *error_is_read, void *bounce_buffer)
 {
-    int ret;
     BlockBackend *blk = job->common.blk;
-    int nbytes, remaining_bytes;
-    int read_flags = is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0;
-
-    assert(QEMU_IS_ALIGNED(start, job->cluster_size));
-    bdrv_reset_dirty_bitmap(job->copy_bitmap, start, end - start);
-    nbytes = MIN(end - start, job->len - start);
-
 
-    remaining_bytes = nbytes;
-    while (remaining_bytes) {
-        int chunk = MIN(BACKUP_MAX_BOUNCE_BUFFER, remaining_bytes);
+    while (bytes) {
+        int ret;
+        int chunk = MIN(BACKUP_MAX_BOUNCE_BUFFER, bytes);
 
-        ret = blk_co_pread(blk, start, chunk, bounce_buffer, read_flags);
+        ret = blk_co_pread(blk, offset, chunk, bounce_buffer, read_flags);
         if (ret < 0) {
-            trace_backup_do_cow_read_fail(job, start, ret);
+            trace_backup_do_cow_read_fail(job, offset, ret);
             if (error_is_read) {
                 *error_is_read = true;
             }
-            goto fail;
+            return ret;
         }
 
-        ret = blk_co_pwrite(job->target, start, chunk, bounce_buffer,
+        ret = blk_co_pwrite(job->target, offset, chunk, bounce_buffer,
                             job->write_flags);
         if (ret < 0) {
-            trace_backup_do_cow_write_fail(job, start, ret);
+            trace_backup_do_cow_write_fail(job, offset, ret);
             if (error_is_read) {
                 *error_is_read = false;
             }
-            goto fail;
+            return ret;
         }
 
-        start += chunk;
-        remaining_bytes -= chunk;
+        offset += chunk;
+        bytes -= chunk;
     }
 
-    return nbytes;
-fail:
-    bdrv_set_dirty_bitmap(job->copy_bitmap, start, job->cluster_size);
-    return ret;
-
+    return 0;
 }
 
 /* Copy range to target and return the bytes copied. If error occurred, return a
  * negative error number. */
 static int coroutine_fn backup_cow_with_offload(BackupBlockJob *job,
-                                                int64_t start,
-                                                int64_t end,
-                                                bool is_write_notifier)
+                                                int64_t offset,
+                                                int64_t bytes,
+                                                BdrvRequestFlags read_flags)
 {
     int ret;
-    int nr_clusters;
     BlockBackend *blk = job->common.blk;
-    int nbytes = MIN(end - start, job->len - start);
-    int read_flags = is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0;
-
-    assert(end - start < INT_MAX);
-    assert(QEMU_IS_ALIGNED(start, job->cluster_size));
-    nr_clusters = DIV_ROUND_UP(nbytes, job->cluster_size);
-    bdrv_reset_dirty_bitmap(job->copy_bitmap, start,
-                            job->cluster_size * nr_clusters);
-    ret = blk_co_copy_range(blk, start, job->target, start, nbytes,
+
+    ret = blk_co_copy_range(blk, offset, job->target, offset, bytes,
                             read_flags, job->write_flags);
     if (ret < 0) {
-        trace_backup_do_cow_copy_range_fail(job, start, ret);
-        bdrv_set_dirty_bitmap(job->copy_bitmap, start,
-                              job->cluster_size * nr_clusters);
-        return ret;
+        trace_backup_do_cow_copy_range_fail(job, offset, ret);
     }
 
-    return nbytes;
+    return ret;
 }
 
 /*
@@ -268,6 +242,8 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
     int64_t start, end; /* bytes */
     void *bounce_buffer = NULL;
     int64_t skip_bytes;
+    BdrvRequestFlags read_flags =
+            is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0;
 
     qemu_co_rwlock_rdlock(&job->flush_rwlock);
 
@@ -281,6 +257,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
 
     while (start < end) {
         int64_t dirty_end;
+        int64_t cur_bytes;
 
         if (!bdrv_dirty_bitmap_get(job->copy_bitmap, start)) {
             trace_backup_do_cow_skip(job, start);
@@ -304,10 +281,11 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
         }
 
         trace_backup_do_cow_process(job, start);
+        cur_bytes = MIN(dirty_end - start, job->len - start);
+        bdrv_reset_dirty_bitmap(job->copy_bitmap, start, dirty_end - start);
 
         if (job->use_copy_range) {
-            ret = backup_cow_with_offload(job, start, dirty_end,
-                                          is_write_notifier);
+            ret = backup_cow_with_offload(job, start, cur_bytes, read_flags);
             if (ret < 0) {
                 job->use_copy_range = false;
             }
@@ -315,24 +293,25 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
         if (!job->use_copy_range) {
             if (!bounce_buffer) {
                 size_t len = MIN(BACKUP_MAX_BOUNCE_BUFFER,
-                                 MAX(dirty_end - start, end - dirty_end));
+                                 MAX(cur_bytes,
+                                     end - start - cur_bytes));
                 bounce_buffer = blk_try_blockalign(job->common.blk, len);
             }
-            ret = backup_cow_with_bounce_buffer(job, start, dirty_end,
-                                                is_write_notifier,
-                                                error_is_read, bounce_buffer);
+            ret = backup_cow_with_bounce_buffer(job, start, cur_bytes,
+                                                read_flags, error_is_read,
+                                                bounce_buffer);
         }
         if (ret < 0) {
+            bdrv_set_dirty_bitmap(job->copy_bitmap, start, dirty_end - start);
             break;
         }
 
         /* Publish progress, guest I/O counts as progress too.  Note that the
          * offset field is an opaque progress value, it is not a disk offset.
          */
-        start += ret;
-        job->bytes_read += ret;
-        job_progress_update(&job->common.job, ret);
-        ret = 0;
+        start += cur_bytes;
+        job->bytes_read += cur_bytes;
+        job_progress_update(&job->common.job, cur_bytes);
     }
 
     if (bounce_buffer) {
-- 
2.18.0