diff mbox

[07/20] block: allow customizing the granularity of the dirty bitmap

Message ID 1355319999-30627-8-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Dec. 12, 2012, 1:46 p.m. UTC 
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block-migration.c |  5 +++--
 block.c           | 17 ++++++++++-------
 block.h           |  5 +----
 block/mirror.c    | 14 ++++----------
 qapi-schema.json  |  4 +++-
 5 files changed, 21 insertions(+), 24 deletions(-)

Comments

Eric Blake Dec. 14, 2012, 9:27 p.m. UTC | #1 
On 12/12/2012 06:46 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block-migration.c |  5 +++--
>  block.c           | 17 ++++++++++-------
>  block.h           |  5 +----
>  block/mirror.c    | 14 ++++----------
>  qapi-schema.json  |  4 +++-
>  5 files changed, 21 insertions(+), 24 deletions(-)
> 

> @@ -4218,16 +4220,17 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size)
>      return qemu_memalign((bs && bs->buffer_alignment) ? bs->buffer_alignment : 512, size);
>  }
>  
> -void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable)
> +void bdrv_set_dirty_tracking(BlockDriverState *bs, int granularity)
>  {
>      int64_t bitmap_size;
>  
> -    if (enable) {
> -        if (!bs->dirty_bitmap) {
> -            bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
> -            bs->dirty_bitmap = hbitmap_alloc(bitmap_size,
> -                                             BDRV_LOG_SECTORS_PER_DIRTY_CHUNK);
> -        }
> +    assert((granularity & (granularity - 1)) == 0);
> +
> +    if (granularity) {
> +        granularity >>= BDRV_SECTOR_BITS;

Given that granularity is specified in bytes, does it make sense for a
user to want a granularity of 4G?  I guess another way of wording this
question is: Are you sure 'int granularity' is the right type?

> +++ b/qapi-schema.json
> @@ -667,10 +667,12 @@
>  #
>  # @count: number of dirty bytes according to the dirty bitmap
>  #
> +# @granularity: granularity of the dirty bitmap in bytes

Mention that this field was added in 1.4.

> +#
>  # Since: 1.3
>  ##
>  { 'type': 'BlockDirtyInfo',
> -  'data': {'count': 'int'} }
> +  'data': {'count': 'int', 'granularity': 'int'} }
>  
>  ##
>  # @BlockInfo:
>
Paolo Bonzini Dec. 15, 2012, 9:11 a.m. UTC | #2 
----- Messaggio originale -----
> Da: "Eric Blake" <eblake@redhat.com>
> A: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: qemu-devel@nongnu.org, kwolf@redhat.com, stefanha@redhat.com, jcody@redhat.com
> Inviato: Venerdì, 14 dicembre 2012 22:27:04
> Oggetto: Re: [PATCH 07/20] block: allow customizing the granularity of the dirty bitmap
> 
> On 12/12/2012 06:46 AM, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  block-migration.c |  5 +++--
> >  block.c           | 17 ++++++++++-------
> >  block.h           |  5 +----
> >  block/mirror.c    | 14 ++++----------
> >  qapi-schema.json  |  4 +++-
> >  5 files changed, 21 insertions(+), 24 deletions(-)
> > 
> 
> > @@ -4218,16 +4220,17 @@ void *qemu_blockalign(BlockDriverState *bs,
> > size_t size)
> >      return qemu_memalign((bs && bs->buffer_alignment) ?
> >      bs->buffer_alignment : 512, size);
> >  }
> >  
> > -void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable)
> > +void bdrv_set_dirty_tracking(BlockDriverState *bs, int
> > granularity)
> >  {
> >      int64_t bitmap_size;
> >  
> > -    if (enable) {
> > -        if (!bs->dirty_bitmap) {
> > -            bitmap_size = (bdrv_getlength(bs) >>
> > BDRV_SECTOR_BITS);
> > -            bs->dirty_bitmap = hbitmap_alloc(bitmap_size,
> > -
> >                                             BDRV_LOG_SECTORS_PER_DIRTY_CHUNK);
> > -        }
> > +    assert((granularity & (granularity - 1)) == 0);
> > +
> > +    if (granularity) {
> > +        granularity >>= BDRV_SECTOR_BITS;
> 
> Given that granularity is specified in bytes, does it make sense for
> a user to want a granularity of 4G?

No, the point of the HBitmap is to allow small granularities.  Anything
bigger than 1 or 2 MB makes little sense.

Paolo
diff mbox

Patch

diff --git a/block-migration.c b/block-migration.c
index 06967d8..1e1d25e 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -23,7 +23,8 @@ 
 #include "blockdev.h"
 #include <assert.h>
 
-#define BLOCK_SIZE (BDRV_SECTORS_PER_DIRTY_CHUNK << BDRV_SECTOR_BITS)
+#define BLOCK_SIZE                       (1 << 20)
+#define BDRV_SECTORS_PER_DIRTY_CHUNK     (BLOCK_SIZE >> BDRV_SECTOR_BITS)
 
 #define BLK_MIG_FLAG_DEVICE_BLOCK       0x01
 #define BLK_MIG_FLAG_EOS                0x02
@@ -264,7 +265,7 @@  static void set_dirty_tracking(int enable)
     BlkMigDevState *bmds;
 
     QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
-        bdrv_set_dirty_tracking(bmds->bs, enable);
+        bdrv_set_dirty_tracking(bmds->bs, enable ? BLOCK_SIZE : 0);
     }
 }
 
diff --git a/block.c b/block.c
index ece2ad5..09f9ca9 100644
--- a/block.c
+++ b/block.c
@@ -2820,6 +2820,8 @@  BlockInfo *bdrv_query_info(BlockDriverState *bs)
         info->has_dirty = true;
         info->dirty = g_malloc0(sizeof(*info->dirty));
         info->dirty->count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE;
+        info->dirty->granularity =
+            ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bs->dirty_bitmap));
     }
 
     if (bs->drv) {
@@ -4218,16 +4220,17 @@  void *qemu_blockalign(BlockDriverState *bs, size_t size)
     return qemu_memalign((bs && bs->buffer_alignment) ? bs->buffer_alignment : 512, size);
 }
 
-void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable)
+void bdrv_set_dirty_tracking(BlockDriverState *bs, int granularity)
 {
     int64_t bitmap_size;
 
-    if (enable) {
-        if (!bs->dirty_bitmap) {
-            bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
-            bs->dirty_bitmap = hbitmap_alloc(bitmap_size,
-                                             BDRV_LOG_SECTORS_PER_DIRTY_CHUNK);
-        }
+    assert((granularity & (granularity - 1)) == 0);
+
+    if (granularity) {
+        granularity >>= BDRV_SECTOR_BITS;
+        assert(!bs->dirty_bitmap);
+        bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
+        bs->dirty_bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
     } else {
         if (bs->dirty_bitmap) {
             hbitmap_free(bs->dirty_bitmap);
diff --git a/block.h b/block.h
index 1418915..b57896a 100644
--- a/block.h
+++ b/block.h
@@ -354,11 +354,8 @@  int bdrv_img_create(const char *filename, const char *fmt,
 void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
 
-#define BDRV_SECTORS_PER_DIRTY_CHUNK     (1 << BDRV_LOG_SECTORS_PER_DIRTY_CHUNK)
-#define BDRV_LOG_SECTORS_PER_DIRTY_CHUNK 11
-
 struct HBitmapIter;
-void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable);
+void bdrv_set_dirty_tracking(BlockDriverState *bs, int granularity);
 int bdrv_get_dirty(BlockDriverState *bs, int64_t sector);
 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);
diff --git a/block/mirror.c b/block/mirror.c
index 48c37a7..1b0478b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -17,14 +17,8 @@ 
 #include "qemu/ratelimit.h"
 #include "bitmap.h"
 
-enum {
-    /*
-     * Size of data buffer for populating the image file.  This should be large
-     * enough to process multiple clusters in a single call, so that populating
-     * contiguous regions of the image is efficient.
-     */
-    BLOCK_SIZE = 512 * BDRV_SECTORS_PER_DIRTY_CHUNK, /* in bytes */
-};
+#define BLOCK_SIZE                       (1 << 20)
+#define BDRV_SECTORS_PER_DIRTY_CHUNK     (BLOCK_SIZE >> BDRV_SECTOR_BITS)
 
 #define SLICE_TIME 100000000ULL /* ns */
 
@@ -274,7 +268,7 @@  static void coroutine_fn mirror_run(void *opaque)
 immediate_exit:
     g_free(s->buf);
     g_free(s->cow_bitmap);
-    bdrv_set_dirty_tracking(bs, false);
+    bdrv_set_dirty_tracking(bs, 0);
     bdrv_iostatus_disable(s->target);
     if (s->should_complete && ret == 0) {
         if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) {
@@ -362,7 +356,7 @@  void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     s->mode = mode;
     s->buf_size = BLOCK_SIZE;
 
-    bdrv_set_dirty_tracking(bs, true);
+    bdrv_set_dirty_tracking(bs, BLOCK_SIZE);
     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);
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..75aacdd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -667,10 +667,12 @@ 
 #
 # @count: number of dirty bytes according to the dirty bitmap
 #
+# @granularity: granularity of the dirty bitmap in bytes
+#
 # Since: 1.3
 ##
 { 'type': 'BlockDirtyInfo',
-  'data': {'count': 'int'} }
+  'data': {'count': 'int', 'granularity': 'int'} }
 
 ##
 # @BlockInfo: