[Qemu-devel] [PATCH 19/20] block: choose the default dirty bitmap granularity in bdrv_enable_dirty_tracking

[Paolo Bonzini]

This is useful because we want the same logic in the monitor commands
that enable the persistent dirty bitmap.

Signed-off-by: Paolo Bonzini <address@hidden>
---
 block-migration.c |  2 +-
 block.c           | 46 +++++++++++++++++++++++++++++++++++++---------
 block.h           |  3 ++-
 block/mirror.c    | 30 ++++++++++++++----------------
 blockdev.c        | 11 +----------
 5 files changed, 55 insertions(+), 37 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index bf995f5..948b56b 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -280,7 +280,7 @@ static void init_blk_migration_it(void *opaque, 
BlockDriverState *bs)
         alloc_aio_bitmap(bmds);
         drive_get_ref(drive_get_by_blockdev(bs));
         bdrv_set_in_use(bs, 1);
-        bdrv_enable_dirty_tracking(bs, BLOCK_SIZE);
+        bdrv_enable_dirty_tracking(bs, BLOCK_SIZE, NULL);
 
         block_mig_state.total_sector_sum += sectors;
 
diff --git a/block.c b/block.c
index d993d0b..2ea91ff 100644
--- a/block.c
+++ b/block.c
@@ -4231,23 +4231,51 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size)
     return qemu_memalign((bs && bs->buffer_alignment) ? bs->buffer_alignment : 
512, size);
 }
 
-void bdrv_enable_dirty_tracking(BlockDriverState *bs, int granularity)
+void bdrv_enable_dirty_tracking(BlockDriverState *bs, int granularity,
+                                Error **errp)
 {
     int64_t bitmap_size;
     int granularity_bits;
 
-    assert((granularity & (granularity - 1)) == 0);
-    granularity >>= BDRV_SECTOR_BITS;
-    granularity_bits = ffs(granularity) - 1;
-    bs->dirty_usage++;
-    if (bs->dirty_usage != 1) {
-        assert(granularity_bits == hbitmap_granularity(bs->dirty_bitmap));
+    if (granularity == -1) {
+        if (bs->dirty_usage != 0) {
+            granularity = bdrv_get_dirty_tracking_granularity(bs);
+        } else {
+            /* Choose the default granularity based on the block device's 
cluster
+             * size, clamped between 4k and 64k.  */
+            BlockDriverInfo bdi;
+            if (bdrv_get_info(bs, &bdi) >= 0 && bdi.cluster_size != 0) {
+                granularity = MAX(4096, bdi.cluster_size);
+                granularity = MIN(65536, granularity);
+            } else {
+                granularity = 65536;
+            }
+        }
+    }
+
+    if (granularity < 512 || granularity > 1048576 * 64) {
+        error_set(errp, QERR_INVALID_PARAMETER, bdrv_get_device_name(bs));
+        return;
+    }
+    if (granularity & (granularity - 1)) {
+        error_set(errp, QERR_INVALID_PARAMETER, bdrv_get_device_name(bs));
         return;
     }
 
-    assert(!bs->dirty_bitmap);
+    granularity_bits = ffs(granularity) - 1 - BDRV_SECTOR_BITS;
+    if (bs->dirty_usage != 0) {
+        if (granularity_bits == hbitmap_granularity(bs->dirty_bitmap)) {
+            bs->dirty_usage++;
+        } else {
+            error_set(errp, QERR_INVALID_PARAMETER, bdrv_get_device_name(bs));
+        }
+        return;
+    }
+
+    bs->dirty_usage++;
+    assert(!bs->dirty_bitmap && !bs->persistent_bitmap);
     bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
-    bs->dirty_bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+    bs->dirty_bitmap = hbitmap_alloc(bitmap_size, granularity_bits);
 }
 
 void bdrv_disable_dirty_tracking(BlockDriverState *bs)
diff --git a/block.h b/block.h
index 1eb4817..fee6bb5 100644
--- a/block.h
+++ b/block.h
@@ -355,7 +355,8 @@ void bdrv_set_buffer_alignment(BlockDriverState *bs, int 
align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
 
 struct HBitmapIter;
-void bdrv_enable_dirty_tracking(BlockDriverState *bs, int granularity);
+void bdrv_enable_dirty_tracking(BlockDriverState *bs, int granularity,
+                                Error **errp);
 void bdrv_disable_dirty_tracking(BlockDriverState *bs);
 int bdrv_get_dirty(BlockDriverState *bs, int64_t sector);
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
diff --git a/block/mirror.c b/block/mirror.c
index 3d37396..cc47c8d 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -550,22 +550,9 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
                   BlockDriverCompletionFunc *cb,
                   void *opaque, Error **errp)
 {
+    Error *local_err = NULL;
     MirrorBlockJob *s;
 
-    if (granularity == 0) {
-        /* Choose the default granularity based on the target file's cluster
-         * size, clamped between 4k and 64k.  */
-        BlockDriverInfo bdi;
-        if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
-            granularity = MAX(4096, bdi.cluster_size);
-            granularity = MIN(65536, granularity);
-        } else {
-            granularity = 65536;
-        }
-    }
-
-    assert((granularity & (granularity - 1)) == 0);
-
     if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
          on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
         !bdrv_iostatus_is_enabled(bs)) {
@@ -573,23 +560,34 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
         return;
     }
 
+    bdrv_enable_dirty_tracking(bs, granularity, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
     s = block_job_create(&mirror_job_type, bs, speed, cb, opaque, errp);
     if (!s) {
-        return;
+        goto fail;
     }
 
     s->on_source_error = on_source_error;
     s->on_target_error = on_target_error;
     s->target = target;
     s->mode = mode;
+
+    granularity = bdrv_get_dirty_tracking_granularity(bs);
     s->granularity = granularity;
     s->buf_size = MAX(buf_size, granularity);
 
-    bdrv_enable_dirty_tracking(bs, granularity);
     bdrv_set_enable_write_cache(s->target, true);
     bdrv_set_on_error(s->target, on_target_error, on_target_error);
     bdrv_iostatus_enable(s->target);
     s->common.co = qemu_coroutine_create(mirror_run);
     trace_mirror_start(bs, s, s->common.co, opaque);
     qemu_coroutine_enter(s->common.co, s);
+    return;
+
+fail:
+    bdrv_disable_dirty_tracking(bs);
 }
diff --git a/blockdev.c b/blockdev.c
index dff6c3d..37e6743 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1222,21 +1222,12 @@ void qmp_drive_mirror(const char *device, const char 
*target,
         mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
     }
     if (!has_granularity) {
-        granularity = 0;
+        granularity = -1;
     }
     if (!has_buf_size) {
         buf_size = DEFAULT_MIRROR_BUF_SIZE;
     }
 
-    if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) 
{
-        error_set(errp, QERR_INVALID_PARAMETER, device);
-        return;
-    }
-    if (granularity & (granularity - 1)) {
-        error_set(errp, QERR_INVALID_PARAMETER, device);
-        return;
-    }
-
     bs = bdrv_find(device);
     if (!bs) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-- 
1.8.0.1