qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [Qemu-devel] [PULL 13/21] block/backup: refactor: split out backup_calculate_cluster_size
Date: Tue, 28 May 2019 21:28:39 +0200	[thread overview]
Message-ID: <20190528192847.2730-14-mreitz@redhat.com> (raw)
In-Reply-To: <20190528192847.2730-1-mreitz@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Split out cluster_size calculation. Move copy-bitmap creation above
block-job creation, as we are going to share it with upcoming
backup-top filter, which also should be created before actual block job
creation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190429090842.57910-6-vsementsov@virtuozzo.com
[mreitz: Dropped a paragraph from the commit message that was left over
         from a previous version]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 82 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 30 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 5b3fc9d123..00f4f8af53 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -497,6 +497,42 @@ static const BlockJobDriver backup_job_driver = {
     .drain                  = backup_drain,
 };
 
+static int64_t backup_calculate_cluster_size(BlockDriverState *target,
+                                             Error **errp)
+{
+    int ret;
+    BlockDriverInfo bdi;
+
+    /*
+     * 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);
+        return 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 ret;
+    } else if (ret < 0 && target->backing) {
+        /* Not fatal; just trudge on ahead. */
+        return BACKUP_CLUSTER_SIZE_DEFAULT;
+    }
+
+    return MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+}
+
 BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   BlockDriverState *target, int64_t speed,
                   MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
@@ -508,9 +544,10 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   JobTxn *txn, Error **errp)
 {
     int64_t len;
-    BlockDriverInfo bdi;
     BackupBlockJob *job = NULL;
     int ret;
+    int64_t cluster_size;
+    HBitmap *copy_bitmap = NULL;
 
     assert(bs);
     assert(target);
@@ -572,6 +609,13 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         goto error;
     }
 
+    cluster_size = backup_calculate_cluster_size(target, errp);
+    if (cluster_size < 0) {
+        goto error;
+    }
+
+    copy_bitmap = hbitmap_alloc(len, ctz32(cluster_size));
+
     /* job->len is fixed, so we can't allow resize */
     job = block_job_create(job_id, &backup_job_driver, txn, bs,
                            BLK_PERM_CONSISTENT_READ,
@@ -600,35 +644,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);
-    }
-
-    job->copy_bitmap = hbitmap_alloc(len, ctz32(job->cluster_size));
+    job->cluster_size = cluster_size;
+    job->copy_bitmap = copy_bitmap;
+    copy_bitmap = NULL;
     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));
@@ -644,6 +662,10 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     return &job->common;
 
  error:
+    if (copy_bitmap) {
+        assert(!job || !job->copy_bitmap);
+        hbitmap_free(copy_bitmap);
+    }
     if (sync_bitmap) {
         bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
     }
-- 
2.21.0



  parent reply	other threads:[~2019-05-28 19:39 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-28 19:28 [Qemu-devel] [PULL 00/21] Block patches Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 01/21] qcow2.h: add missing include Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 02/21] qcow2: add separate file for threaded data processing functions Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 03/21] qcow2-threads: use thread_pool_submit_co Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 04/21] qcow2-threads: qcow2_co_do_compress: protect queuing by mutex Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 05/21] qcow2-threads: split out generic path Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 06/21] qcow2: qcow2_co_preadv: improve locking Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 07/21] qcow2: bdrv_co_pwritev: move encryption code out of the lock Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 08/21] qcow2: do encryption in threads Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 09/21] block/backup: simplify backup_incremental_init_copy_bitmap Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 10/21] block/backup: move to copy_bitmap with granularity Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 11/21] block/backup: refactor and tolerate unallocated cluster skipping Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 12/21] block/backup: unify different modes code path Max Reitz
2019-05-28 19:28 ` Max Reitz [this message]
2019-05-28 19:28 ` [Qemu-devel] [PULL 14/21] block: Use bdrv_unref_child() for all children in bdrv_close() Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 15/21] block: Make bdrv_root_attach_child() unref child_bs on failure Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 16/21] qemu-img: rebase: Reuse parent BlockDriverState Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 17/21] qemu-img: rebase: Reduce reads on in-chain rebase Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 18/21] qemu-img: rebase: Reuse in-chain BlockDriverState Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 19/21] qcow2: skip writing zero buffers to empty COW areas Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 20/21] qcow2-bitmap: initialize bitmap directory alignment Max Reitz
2019-05-28 19:28 ` [Qemu-devel] [PULL 21/21] blockdev: loosen restrictions on drive-backup source node Max Reitz
2019-05-30 11:09 ` [Qemu-devel] [PULL 00/21] Block patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190528192847.2730-14-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).