diff mbox

[v4,06/15] block: Hide HBitmap in block dirty bitmap interface

Message ID 1457412306-18940-7-git-send-email-famz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fam Zheng March 8, 2016, 4:44 a.m. UTC
HBitmap is an implementation detail of block dirty bitmap that should be hidden
from users. Introduce a BdrvDirtyBitmapIter to encapsulate the underlying
HBitmapIter.

A small difference in the interface is, before, an HBitmapIter is initialized
in place, now the new BdrvDirtyBitmapIter must be dynamically allocated because
the structure definition is in block.c.

Two current users are converted too.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/backup.c               | 14 ++++++++------
 block/dirty-bitmap.c         | 39 +++++++++++++++++++++++++++++++++------
 block/mirror.c               | 15 +++++++++------
 include/block/dirty-bitmap.h |  7 +++++--
 include/qemu/typedefs.h      |  1 +
 5 files changed, 56 insertions(+), 20 deletions(-)

Comments

Max Reitz March 11, 2016, 1:54 p.m. UTC | #1
On 08.03.2016 05:44, Fam Zheng wrote:
> HBitmap is an implementation detail of block dirty bitmap that should be hidden
> from users. Introduce a BdrvDirtyBitmapIter to encapsulate the underlying
> HBitmapIter.
> 
> A small difference in the interface is, before, an HBitmapIter is initialized
> in place, now the new BdrvDirtyBitmapIter must be dynamically allocated because
> the structure definition is in block.c.

It's block/dirty-bitmap.c now. :-)

> Two current users are converted too.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/backup.c               | 14 ++++++++------
>  block/dirty-bitmap.c         | 39 +++++++++++++++++++++++++++++++++------
>  block/mirror.c               | 15 +++++++++------
>  include/block/dirty-bitmap.h |  7 +++++--
>  include/qemu/typedefs.h      |  1 +
>  5 files changed, 56 insertions(+), 20 deletions(-)
> 

[...]

> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 556e1d1..16f73b2 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c

[...]

> @@ -373,12 +401,11 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
>  }
>  
>  /**
> - * Advance an HBitmapIter to an arbitrary offset.
> + * Advance an BdrvDirtyBitmapIter to an arbitrary offset.

This should be "a BdrvDirtyBitmapIter".

>   */
> -void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
> +void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *iter, int64_t sector_num)
>  {
> -    assert(hbi->hb);
> -    hbitmap_iter_init(hbi, hbi->hb, offset);
> +    hbitmap_iter_init(&iter->hbi, iter->hbi.hb, sector_num);
>  }
>  
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
> diff --git a/block/mirror.c b/block/mirror.c
> index 9635fa8..87a5a86 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c

[...]

> @@ -304,10 +304,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>      int64_t end = s->bdev_length / BDRV_SECTOR_SIZE;
>      int sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
>  
> -    sector_num = hbitmap_iter_next(&s->hbi);
> +    sector_num = bdrv_dirty_iter_next(s->dbi);
>      if (sector_num < 0) {
> -        bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
> -        sector_num = hbitmap_iter_next(&s->hbi);
> +        bdrv_dirty_iter_free(s->dbi);
> +        s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0);

Works, but wouldn't bdrv_set_dirty_iter(s->dbi, 0); suffice?

> +        sector_num = bdrv_dirty_iter_next(s->dbi);
>          trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
>          assert(sector_num >= 0);
>      }
> @@ -330,7 +331,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>              mirror_wait_for_io(s);
>              /* Now retry.  */
>          } else {
> -            hbitmap_next = hbitmap_iter_next(&s->hbi);
> +            hbitmap_next = bdrv_dirty_iter_next(s->dbi);

It would make sense to change this variable's name from hbitmap_next to
e.g. bdrv_dirty_next or bs_next_dirty.

>              assert(hbitmap_next == next_sector);
>              nb_chunks++;
>          }
> @@ -577,7 +578,8 @@ static void coroutine_fn mirror_run(void *opaque)
>          }
>      }
>  
> -    bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
> +    bdrv_dirty_iter_free(s->dbi);
> +    s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0);

Again, bdrv_set_dirty_iter(s->dbi, 0); should work just fine.

Since this patch is technically correct, I could still imagine giving an
R-b, but I'm really reluctant to do so because of the
bdrv_dirty_iter_free()/bdrv_dirty_iter_new() pairs.

Max

>      for (;;) {
>          uint64_t delay_ns = 0;
>          int64_t cnt;
> @@ -688,6 +690,7 @@ immediate_exit:
>      qemu_vfree(s->buf);
>      g_free(s->cow_bitmap);
>      g_free(s->in_flight_bitmap);
> +    bdrv_dirty_iter_free(s->dbi);
>      bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
>      if (s->target->blk) {
>          blk_iostatus_disable(s->target->blk);
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 80afe60..2ea601b 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -36,8 +36,11 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                             int64_t cur_sector, int nr_sectors);
>  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                               int64_t cur_sector, int nr_sectors);
> -void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
> -void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
> +BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
> +                                         uint64_t first_sector);
> +void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
> +int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
> +void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>  
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index fd039e0..7c39f0f 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -11,6 +11,7 @@ typedef struct AioContext AioContext;
>  typedef struct AllwinnerAHCIState AllwinnerAHCIState;
>  typedef struct AudioState AudioState;
>  typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
> +typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter;
>  typedef struct BlockBackend BlockBackend;
>  typedef struct BlockBackendRootState BlockBackendRootState;
>  typedef struct BlockDriverState BlockDriverState;
>
Max Reitz March 11, 2016, 2:27 p.m. UTC | #2
On 11.03.2016 14:54, Max Reitz wrote:
> On 08.03.2016 05:44, Fam Zheng wrote:
>> HBitmap is an implementation detail of block dirty bitmap that should be hidden
>> from users. Introduce a BdrvDirtyBitmapIter to encapsulate the underlying
>> HBitmapIter.
>>
>> A small difference in the interface is, before, an HBitmapIter is initialized
>> in place, now the new BdrvDirtyBitmapIter must be dynamically allocated because
>> the structure definition is in block.c.
> 
> It's block/dirty-bitmap.c now. :-)
> 
>> Two current users are converted too.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>>  block/backup.c               | 14 ++++++++------
>>  block/dirty-bitmap.c         | 39 +++++++++++++++++++++++++++++++++------
>>  block/mirror.c               | 15 +++++++++------
>>  include/block/dirty-bitmap.h |  7 +++++--
>>  include/qemu/typedefs.h      |  1 +
>>  5 files changed, 56 insertions(+), 20 deletions(-)
>>
> 
> [...]
> 
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 556e1d1..16f73b2 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
> 
> [...]
> 
>> @@ -373,12 +401,11 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
>>  }
>>  
>>  /**
>> - * Advance an HBitmapIter to an arbitrary offset.
>> + * Advance an BdrvDirtyBitmapIter to an arbitrary offset.
> 
> This should be "a BdrvDirtyBitmapIter".
> 
>>   */
>> -void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
>> +void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *iter, int64_t sector_num)
>>  {
>> -    assert(hbi->hb);
>> -    hbitmap_iter_init(hbi, hbi->hb, offset);
>> +    hbitmap_iter_init(&iter->hbi, iter->hbi.hb, sector_num);
>>  }
>>  
>>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 9635fa8..87a5a86 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
> 
> [...]
> 
>> @@ -304,10 +304,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>>      int64_t end = s->bdev_length / BDRV_SECTOR_SIZE;
>>      int sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
>>  
>> -    sector_num = hbitmap_iter_next(&s->hbi);
>> +    sector_num = bdrv_dirty_iter_next(s->dbi);
>>      if (sector_num < 0) {
>> -        bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
>> -        sector_num = hbitmap_iter_next(&s->hbi);
>> +        bdrv_dirty_iter_free(s->dbi);
>> +        s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0);
> 
> Works, but wouldn't bdrv_set_dirty_iter(s->dbi, 0); suffice?
> 
>> +        sector_num = bdrv_dirty_iter_next(s->dbi);
>>          trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
>>          assert(sector_num >= 0);
>>      }
>> @@ -330,7 +331,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>>              mirror_wait_for_io(s);
>>              /* Now retry.  */
>>          } else {
>> -            hbitmap_next = hbitmap_iter_next(&s->hbi);
>> +            hbitmap_next = bdrv_dirty_iter_next(s->dbi);
> 
> It would make sense to change this variable's name from hbitmap_next to
> e.g. bdrv_dirty_next or bs_next_dirty.
> 
>>              assert(hbitmap_next == next_sector);
>>              nb_chunks++;
>>          }
>> @@ -577,7 +578,8 @@ static void coroutine_fn mirror_run(void *opaque)
>>          }
>>      }
>>  
>> -    bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
>> +    bdrv_dirty_iter_free(s->dbi);
>> +    s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0);
> 
> Again, bdrv_set_dirty_iter(s->dbi, 0); should work just fine.

OK, here it wouldn't because s->dbi is still NULL. Why do we need the
bdrv_dirty_iter_free(), then? It wasn't present in v3.

Max

> Since this patch is technically correct, I could still imagine giving an
> R-b, but I'm really reluctant to do so because of the
> bdrv_dirty_iter_free()/bdrv_dirty_iter_new() pairs.
> 
> Max
> 
>>      for (;;) {
>>          uint64_t delay_ns = 0;
>>          int64_t cnt;
>> @@ -688,6 +690,7 @@ immediate_exit:
>>      qemu_vfree(s->buf);
>>      g_free(s->cow_bitmap);
>>      g_free(s->in_flight_bitmap);
>> +    bdrv_dirty_iter_free(s->dbi);
>>      bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
>>      if (s->target->blk) {
>>          blk_iostatus_disable(s->target->blk);
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index 80afe60..2ea601b 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -36,8 +36,11 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>                             int64_t cur_sector, int nr_sectors);
>>  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>                               int64_t cur_sector, int nr_sectors);
>> -void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
>> -void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>> +BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>> +                                         uint64_t first_sector);
>> +void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
>> +int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>> +void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
>>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>>  void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>>  
>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>> index fd039e0..7c39f0f 100644
>> --- a/include/qemu/typedefs.h
>> +++ b/include/qemu/typedefs.h
>> @@ -11,6 +11,7 @@ typedef struct AioContext AioContext;
>>  typedef struct AllwinnerAHCIState AllwinnerAHCIState;
>>  typedef struct AudioState AudioState;
>>  typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
>> +typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter;
>>  typedef struct BlockBackend BlockBackend;
>>  typedef struct BlockBackendRootState BlockBackendRootState;
>>  typedef struct BlockDriverState BlockDriverState;
>>
> 
>
diff mbox

Patch

diff --git a/block/backup.c b/block/backup.c
index ab3e345..f5a5e03 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -330,14 +330,14 @@  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
     int64_t last_cluster = -1;
     int64_t sectors_per_cluster = cluster_size_sectors(job);
     BlockDriverState *bs = job->common.bs;
-    HBitmapIter hbi;
+    BdrvDirtyBitmapIter *dbi;
 
     granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
     clusters_per_iter = MAX((granularity / job->cluster_size), 1);
-    bdrv_dirty_iter_init(job->sync_bitmap, &hbi);
+    dbi = bdrv_dirty_iter_new(job->sync_bitmap, 0);
 
     /* Find the next dirty sector(s) */
-    while ((sector = hbitmap_iter_next(&hbi)) != -1) {
+    while ((sector = bdrv_dirty_iter_next(dbi)) != -1) {
         cluster = sector / sectors_per_cluster;
 
         /* Fake progress updates for any clusters we skipped */
@@ -349,7 +349,7 @@  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
         for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
             do {
                 if (yield_and_check(job)) {
-                    return ret;
+                    goto out;
                 }
                 ret = backup_do_cow(bs, cluster * sectors_per_cluster,
                                     sectors_per_cluster, &error_is_read,
@@ -357,7 +357,7 @@  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
                 if ((ret < 0) &&
                     backup_error_action(job, error_is_read, -ret) ==
                     BLOCK_ERROR_ACTION_REPORT) {
-                    return ret;
+                    goto out;
                 }
             } while (ret < 0);
         }
@@ -365,7 +365,7 @@  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
         /* If the bitmap granularity is smaller than the backup granularity,
          * we need to advance the iterator pointer to the next cluster. */
         if (granularity < job->cluster_size) {
-            bdrv_set_dirty_iter(&hbi, cluster * sectors_per_cluster);
+            bdrv_set_dirty_iter(dbi, cluster * sectors_per_cluster);
         }
 
         last_cluster = cluster - 1;
@@ -377,6 +377,8 @@  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
         job->common.offset += ((end - last_cluster - 1) * job->cluster_size);
     }
 
+out:
+    bdrv_dirty_iter_free(dbi);
     return ret;
 }
 
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 556e1d1..16f73b2 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -42,9 +42,15 @@  struct BdrvDirtyBitmap {
     char *name;                 /* Optional non-empty unique ID */
     int64_t size;               /* Size of the bitmap (Number of sectors) */
     bool disabled;              /* Bitmap is read-only */
+    int active_iterators;       /* How many iterators are active */
     QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
+struct BdrvDirtyBitmapIter {
+    HBitmapIter hbi;
+    BdrvDirtyBitmap *bitmap;
+};
+
 BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
 {
     BdrvDirtyBitmap *bm;
@@ -212,6 +218,7 @@  void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
 
     QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
         assert(!bdrv_dirty_bitmap_frozen(bitmap));
+        assert(!bitmap->active_iterators);
         hbitmap_truncate(bitmap->bitmap, size);
         bitmap->size = size;
     }
@@ -224,6 +231,7 @@  static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
     BdrvDirtyBitmap *bm, *next;
     QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
         if ((!bitmap || bm == bitmap) && (!only_named || bm->name)) {
+            assert(!bitmap->active_iterators);
             assert(!bdrv_dirty_bitmap_frozen(bm));
             QLIST_REMOVE(bm, list);
             hbitmap_free(bm->bitmap);
@@ -320,9 +328,29 @@  uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
     return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
 }
 
-void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
+BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
+                                         uint64_t first_sector)
 {
-    hbitmap_iter_init(hbi, bitmap->bitmap, 0);
+    BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
+    hbitmap_iter_init(&iter->hbi, bitmap->bitmap, first_sector);
+    iter->bitmap = bitmap;
+    bitmap->active_iterators++;
+    return iter;
+}
+
+void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter)
+{
+    if (!iter) {
+        return;
+    }
+    assert(iter->bitmap->active_iterators > 0);
+    iter->bitmap->active_iterators--;
+    g_free(iter);
+}
+
+int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter)
+{
+    return hbitmap_iter_next(&iter->hbi);
 }
 
 void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
@@ -373,12 +401,11 @@  void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
 }
 
 /**
- * Advance an HBitmapIter to an arbitrary offset.
+ * Advance an BdrvDirtyBitmapIter to an arbitrary offset.
  */
-void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
+void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *iter, int64_t sector_num)
 {
-    assert(hbi->hb);
-    hbitmap_iter_init(hbi, hbi->hb, offset);
+    hbitmap_iter_init(&iter->hbi, iter->hbi.hb, sector_num);
 }
 
 int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
diff --git a/block/mirror.c b/block/mirror.c
index 9635fa8..87a5a86 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -52,7 +52,7 @@  typedef struct MirrorBlockJob {
     int64_t bdev_length;
     unsigned long *cow_bitmap;
     BdrvDirtyBitmap *dirty_bitmap;
-    HBitmapIter hbi;
+    BdrvDirtyBitmapIter *dbi;
     uint8_t *buf;
     QSIMPLEQ_HEAD(, MirrorBuffer) buf_free;
     int buf_free_count;
@@ -304,10 +304,11 @@  static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     int64_t end = s->bdev_length / BDRV_SECTOR_SIZE;
     int sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
 
-    sector_num = hbitmap_iter_next(&s->hbi);
+    sector_num = bdrv_dirty_iter_next(s->dbi);
     if (sector_num < 0) {
-        bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
-        sector_num = hbitmap_iter_next(&s->hbi);
+        bdrv_dirty_iter_free(s->dbi);
+        s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0);
+        sector_num = bdrv_dirty_iter_next(s->dbi);
         trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
         assert(sector_num >= 0);
     }
@@ -330,7 +331,7 @@  static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
             mirror_wait_for_io(s);
             /* Now retry.  */
         } else {
-            hbitmap_next = hbitmap_iter_next(&s->hbi);
+            hbitmap_next = bdrv_dirty_iter_next(s->dbi);
             assert(hbitmap_next == next_sector);
             nb_chunks++;
         }
@@ -577,7 +578,8 @@  static void coroutine_fn mirror_run(void *opaque)
         }
     }
 
-    bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
+    bdrv_dirty_iter_free(s->dbi);
+    s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0);
     for (;;) {
         uint64_t delay_ns = 0;
         int64_t cnt;
@@ -688,6 +690,7 @@  immediate_exit:
     qemu_vfree(s->buf);
     g_free(s->cow_bitmap);
     g_free(s->in_flight_bitmap);
+    bdrv_dirty_iter_free(s->dbi);
     bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
     if (s->target->blk) {
         blk_iostatus_disable(s->target->blk);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 80afe60..2ea601b 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -36,8 +36,11 @@  void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                            int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                              int64_t cur_sector, int nr_sectors);
-void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
-void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
+BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
+                                         uint64_t first_sector);
+void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
+int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
+void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
 int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
 
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index fd039e0..7c39f0f 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -11,6 +11,7 @@  typedef struct AioContext AioContext;
 typedef struct AllwinnerAHCIState AllwinnerAHCIState;
 typedef struct AudioState AudioState;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
+typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter;
 typedef struct BlockBackend BlockBackend;
 typedef struct BlockBackendRootState BlockBackendRootState;
 typedef struct BlockDriverState BlockDriverState;