diff mbox

[24/47] block: introduce new dirty bitmap functionality

Message ID 1343127865-16608-25-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini July 24, 2012, 11:04 a.m. UTC 
Assert that write_compressed is never used with the dirty bitmap.
Setting the bits early is wrong, because a coroutine might concurrently
examine them and copy incomplete data from the source.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c |   51 +++++++++++++++++++++++++++++++++++++++++++++------
 block.h |    5 +++--
 2 files changed, 48 insertions(+), 8 deletions(-)

Comments

Kevin Wolf Sept. 11, 2012, 2:57 p.m. UTC | #1 
Am 24.07.2012 13:04, schrieb Paolo Bonzini:
> Assert that write_compressed is never used with the dirty bitmap.
> Setting the bits early is wrong, because a coroutine might concurrently
> examine them and copy incomplete data from the source.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Looks good, but I think it should be documented at least that
bdrv_get_next_dirty() hangs when it's called and there is no dirty block.

Kevin
Paolo Bonzini Sept. 11, 2012, 4:17 p.m. UTC | #2 
Il 11/09/2012 16:57, Kevin Wolf ha scritto:
>> > Assert that write_compressed is never used with the dirty bitmap.
>> > Setting the bits early is wrong, because a coroutine might concurrently
>> > examine them and copy incomplete data from the source.
>> > 
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Looks good, but I think it should be documented at least that
> bdrv_get_next_dirty() hangs when it's called and there is no dirty block.

Since bdrv_get_next_dirty disappears soon, I'll just add an assertion.

Paolo
diff mbox

Patch

diff --git a/block.c b/block.c
index 002b442..81c2bc5 100644
--- a/block.c
+++ b/block.c
@@ -2048,7 +2048,7 @@  static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
     }
 
     if (bs->dirty_bitmap) {
-        set_dirty_bitmap(bs, sector_num, nb_sectors, 1);
+        bdrv_set_dirty(bs, sector_num, nb_sectors);
     }
 
     if (bs->wr_highest_sector < sector_num + nb_sectors - 1) {
@@ -2607,9 +2607,7 @@  int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
     if (bdrv_check_request(bs, sector_num, nb_sectors))
         return -EIO;
 
-    if (bs->dirty_bitmap) {
-        set_dirty_bitmap(bs, sector_num, nb_sectors, 1);
-    }
+    assert(!bs->dirty_bitmap);
 
     return drv->bdrv_write_compressed(bs, sector_num, buf, nb_sectors);
 }
@@ -3838,13 +3836,54 @@  int bdrv_get_dirty(BlockDriverState *bs, int64_t sector)
 
     if (bs->dirty_bitmap &&
         (sector << BDRV_SECTOR_BITS) < bdrv_getlength(bs)) {
-        return !!(bs->dirty_bitmap[chunk / (sizeof(unsigned long) * 8)] &
-            (1UL << (chunk % (sizeof(unsigned long) * 8))));
+        return !!(bs->dirty_bitmap[chunk / BITS_PER_LONG] &
+            (1UL << (chunk % BITS_PER_LONG)));
     } else {
         return 0;
     }
 }
 
+int64_t bdrv_get_next_dirty(BlockDriverState *bs, int64_t sector)
+{
+    int64_t chunk;
+    int bit, elem;
+
+    /* Avoid an infinite loop.  */
+    assert(bs->dirty_count > 0);
+
+    sector = (sector | (BDRV_SECTORS_PER_DIRTY_CHUNK - 1)) + 1;
+    chunk = sector / (int64_t)BDRV_SECTORS_PER_DIRTY_CHUNK;
+
+    QEMU_BUILD_BUG_ON(sizeof(bs->dirty_bitmap[0]) * 8 != BITS_PER_LONG);
+    elem = chunk / BITS_PER_LONG;
+    bit = chunk % BITS_PER_LONG;
+    for (;;) {
+        if (sector >= bs->total_sectors) {
+            sector = 0;
+            bit = elem = 0;
+        }
+        if (bit == 0 && bs->dirty_bitmap[elem] == 0) {
+            sector += BDRV_SECTORS_PER_DIRTY_CHUNK * BITS_PER_LONG;
+            elem++;
+        } else {
+            if (bs->dirty_bitmap[elem] & (1UL << bit)) {
+                return sector;
+            }
+            sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
+            if (++bit == BITS_PER_LONG) {
+                bit = 0;
+                elem++;
+            }
+        }
+    }
+}
+
+void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
+                    int nr_sectors)
+{
+    set_dirty_bitmap(bs, cur_sector, nr_sectors, 1);
+}
+
 void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
                       int nr_sectors)
 {
diff --git a/block.h b/block.h
index 8aeb7a9..e7440f6 100644
--- a/block.h
+++ b/block.h
@@ -328,8 +328,9 @@  void *qemu_blockalign(BlockDriverState *bs, size_t size);
 
 void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable);
 int bdrv_get_dirty(BlockDriverState *bs, int64_t sector);
-void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
-                      int nr_sectors);
+void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
+void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
+int64_t bdrv_get_next_dirty(BlockDriverState *bs, int64_t sector);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs);
 
 void bdrv_enable_copy_on_read(BlockDriverState *bs);