[Qemu-devel] [PATCH v3 17/18] block/backup: tiny refactor backup_job_create

[Vladimir Sementsov-Ogievskiy]

Move copy-bitmap find/create code. It's needed for the following
commit, as we'll need copy_bitmap before actual block job creation. Do
it in a separate commit to simplify review.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
 block/backup.c | 95 +++++++++++++++++++++++++++-----------------------
 1 file changed, 52 insertions(+), 43 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 11aa31a323..6cab54dea4 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -570,6 +570,9 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
     BackupBlockJob *job = NULL;
     int ret;
     char *gen_bitmap_name = NULL;
+    int64_t cluster_size;
+    BdrvDirtyBitmap *copy_bitmap = NULL;
+    bool copy_bitmap_created = false;
 
     assert(bs);
     assert(target);
@@ -624,6 +627,48 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
         return NULL;
     }
 
+    /* If there is no backing file on the target, we cannot rely on COW if our
+     * backup cluster size is smaller than the target cluster size. Even for
+     * targets with a backing file, try to avoid COW if possible. */
+    ret = bdrv_get_info(target, &bdi);
+    if (ret == -ENOTSUP && !target->backing) {
+        /* Cluster size is not defined */
+        warn_report("The target block device doesn't provide "
+                    "information about the block size and it doesn't have a "
+                    "backing file. The default block size of %u bytes is "
+                    "used. If the actual block size of the target exceeds "
+                    "this default, the backup may be unusable",
+                    BACKUP_CLUSTER_SIZE_DEFAULT);
+        cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
+    } else if (ret < 0 && !target->backing) {
+        error_setg_errno(errp, -ret,
+            "Couldn't determine the cluster size of the target image, "
+            "which has no backing file");
+        error_append_hint(errp,
+            "Aborting, since this may create an unusable destination image\n");
+        return NULL;
+    } else if (ret < 0 && target->backing) {
+        /* Not fatal; just trudge on ahead. */
+        cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
+    } else {
+        cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+    }
+
+    if (x_copy_bitmap) {
+        copy_bitmap = bdrv_find_dirty_bitmap(bs, x_copy_bitmap);
+    } else {
+        x_copy_bitmap = gen_bitmap_name = id_generate(ID_BLOCK_BITMAP);
+    }
+    if (!copy_bitmap) {
+        copy_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size,
+                                               x_copy_bitmap, errp);
+        copy_bitmap_created = !!copy_bitmap;
+    }
+    g_free(gen_bitmap_name);
+    if (!copy_bitmap) {
+        return NULL;
+    }
+
     len = bdrv_getlength(bs);
     if (len < 0) {
         error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -659,49 +704,9 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 
     /* Detect image-fleecing (and similar) schemes */
     job->serialize_target_writes = bdrv_chain_contains(target, bs);
-
-    /* If there is no backing file on the target, we cannot rely on COW if our
-     * backup cluster size is smaller than the target cluster size. Even for
-     * targets with a backing file, try to avoid COW if possible. */
-    ret = bdrv_get_info(target, &bdi);
-    if (ret == -ENOTSUP && !target->backing) {
-        /* Cluster size is not defined */
-        warn_report("The target block device doesn't provide "
-                    "information about the block size and it doesn't have a "
-                    "backing file. The default block size of %u bytes is "
-                    "used. If the actual block size of the target exceeds "
-                    "this default, the backup may be unusable",
-                    BACKUP_CLUSTER_SIZE_DEFAULT);
-        job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
-    } else if (ret < 0 && !target->backing) {
-        error_setg_errno(errp, -ret,
-            "Couldn't determine the cluster size of the target image, "
-            "which has no backing file");
-        error_append_hint(errp,
-            "Aborting, since this may create an unusable destination image\n");
-        goto error;
-    } else if (ret < 0 && target->backing) {
-        /* Not fatal; just trudge on ahead. */
-        job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
-    } else {
-        job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
-    }
-
-    if (x_copy_bitmap) {
-        job->copy_bitmap = bdrv_find_dirty_bitmap(bs, x_copy_bitmap);
-    } else {
-        x_copy_bitmap = gen_bitmap_name = id_generate(ID_BLOCK_BITMAP);
-    }
-    if (!job->copy_bitmap) {
-        job->copy_bitmap = bdrv_create_dirty_bitmap(bs, job->cluster_size,
-                                                    x_copy_bitmap, errp);
-        job->copy_bitmap_created = !!job->copy_bitmap;
-    }
-    g_free(gen_bitmap_name);
-    if (!job->copy_bitmap) {
-        goto error;
-    }
-
+    job->cluster_size = cluster_size;
+    job->copy_bitmap = copy_bitmap;
+    job->copy_bitmap_created = copy_bitmap_created;
     job->use_copy_range = true;
     job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
                                         blk_get_max_transfer(job->target));
@@ -717,6 +722,10 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
     return &job->common;
 
  error:
+    if (copy_bitmap_created) {
+        assert(!job || !job->copy_bitmap_created);
+        bdrv_release_dirty_bitmap(bs, copy_bitmap);
+    }
     if (sync_bitmap) {
         bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
     }
-- 
2.18.0