diff mbox

[v2,08/13] block: Support meta dirty bitmap

Message ID 1453270306-16608-9-git-send-email-famz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fam Zheng Jan. 20, 2016, 6:11 a.m. UTC
The added group of operations enables tracking of the changed bits in
the dirty bitmap.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/dirty-bitmap.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++
 include/block/dirty-bitmap.h |  9 ++++++++
 2 files changed, 60 insertions(+)

Comments

Vladimir Sementsov-Ogievskiy Jan. 22, 2016, 11:34 a.m. UTC | #1
On 20.01.2016 09:11, Fam Zheng wrote:
> The added group of operations enables tracking of the changed bits in
> the dirty bitmap.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   block/dirty-bitmap.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++
>   include/block/dirty-bitmap.h |  9 ++++++++
>   2 files changed, 60 insertions(+)
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index bd7758b..d75dcf7 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -37,6 +37,7 @@
>    */
>   struct BdrvDirtyBitmap {
>       HBitmap *bitmap;            /* Dirty sector bitmap implementation */
> +    HBitmap *meta;              /* Meta dirty bitmap */
>       BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
>       char *name;                 /* Optional non-empty unique ID */
>       int64_t size;               /* Size of the bitmap (Number of sectors) */
> @@ -102,6 +103,56 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>       return bitmap;
>   }
>   
> +/* bdrv_create_meta_dirty_bitmap
> + *
> + * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e.
> + * when a dirty status bit in @bitmap is changed (either from reset to set or
> + * the other way around), its respective meta dirty bitmap bit will be marked
> + * dirty as well.
> + *
> + * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
> + * @granularity: how many bytes of bitmap data does each bit in the meta bitmap
> + * track.
> + */
> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> +                                   int granularity)
> +{
> +    assert(!bitmap->meta);
> +    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
> +                                       BDRV_SECTOR_SIZE * BITS_PER_BYTE);
> +}

what is granularity here? Is it unused?

Here should be chunk_size parameter, which then will be somehow send to 
hbitmap_create_meta.

> +
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> +    assert(bitmap->meta);
> +    hbitmap_free_meta(bitmap->bitmap);
> +    bitmap->meta = NULL;
> +}
> +
> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
> +                               int nb_sectors)
> +{
> +    uint64_t i;
> +    int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
> +
> +    /* To optimize: we can make hbitmap to internally check the range in a
> +     * coarse level, or at least do it word by word. */
> +    for (i = sector; i < sector + nb_sectors; i += gran) {
> +        if (hbitmap_get(bitmap->meta, i)) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> +                                  int nb_sectors)
> +{
> +    hbitmap_reset(bitmap->meta, sector, nb_sectors);
> +}
> +
>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
>   {
>       return bitmap->successor;
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 120bac6..d9b281a 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -9,6 +9,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                             uint32_t granularity,
>                                             const char *name,
>                                             Error **errp);
> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> +                                   int granularity);
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>   int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>                                          BdrvDirtyBitmap *bitmap,
>                                          Error **errp);
> @@ -35,6 +38,12 @@ 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);
> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
> +                               int nb_sectors);
> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> +                                  int nb_sectors);
>   BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>                                            uint64_t first_sector);
>   void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
Vladimir Sementsov-Ogievskiy Jan. 22, 2016, 11:58 a.m. UTC | #2
On 20.01.2016 09:11, Fam Zheng wrote:
> The added group of operations enables tracking of the changed bits in
> the dirty bitmap.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   block/dirty-bitmap.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++
>   include/block/dirty-bitmap.h |  9 ++++++++
>   2 files changed, 60 insertions(+)
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index bd7758b..d75dcf7 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -37,6 +37,7 @@
>    */
>   struct BdrvDirtyBitmap {
>       HBitmap *bitmap;            /* Dirty sector bitmap implementation */
> +    HBitmap *meta;              /* Meta dirty bitmap */
>       BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
>       char *name;                 /* Optional non-empty unique ID */
>       int64_t size;               /* Size of the bitmap (Number of sectors) */
> @@ -102,6 +103,56 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>       return bitmap;
>   }
>   
> +/* bdrv_create_meta_dirty_bitmap
> + *
> + * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e.
> + * when a dirty status bit in @bitmap is changed (either from reset to set or
> + * the other way around), its respective meta dirty bitmap bit will be marked
> + * dirty as well.
> + *
> + * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
> + * @granularity: how many bytes of bitmap data does each bit in the meta bitmap
> + * track.
> + */
> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> +                                   int granularity)
> +{
> +    assert(!bitmap->meta);
> +    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
> +                                       BDRV_SECTOR_SIZE * BITS_PER_BYTE);
> +}
> +
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> +    assert(bitmap->meta);
> +    hbitmap_free_meta(bitmap->bitmap);
> +    bitmap->meta = NULL;
> +}
> +
> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
> +                               int nb_sectors)
> +{
> +    uint64_t i;
> +    int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
> +
> +    /* To optimize: we can make hbitmap to internally check the range in a
> +     * coarse level, or at least do it word by word. */
> +    for (i = sector; i < sector + nb_sectors; i += gran) {
> +        if (hbitmap_get(bitmap->meta, i)) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> +                                  int nb_sectors)
> +{
> +    hbitmap_reset(bitmap->meta, sector, nb_sectors);
> +}
> +
>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
>   {
>       return bitmap->successor;
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 120bac6..d9b281a 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -9,6 +9,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                             uint32_t granularity,
>                                             const char *name,
>                                             Error **errp);
> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> +                                   int granularity);
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>   int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>                                          BdrvDirtyBitmap *bitmap,
>                                          Error **errp);
> @@ -35,6 +38,12 @@ 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);
> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
> +                               int nb_sectors);
> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> +                                  int nb_sectors);
>   BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>                                            uint64_t first_sector);
>   void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);

In my migration series I need iterators, get granularity, and something 
like hbitmap_count  for meta bitmaps. You can add them here if you want, 
or I can add them in my series.
Vladimir Sementsov-Ogievskiy Jan. 22, 2016, 12:05 p.m. UTC | #3
On 22.01.2016 14:58, Vladimir Sementsov-Ogievskiy wrote:
> On 20.01.2016 09:11, Fam Zheng wrote:
>> The added group of operations enables tracking of the changed bits in
>> the dirty bitmap.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>>   block/dirty-bitmap.c         | 51 
>> ++++++++++++++++++++++++++++++++++++++++++++
>>   include/block/dirty-bitmap.h |  9 ++++++++
>>   2 files changed, 60 insertions(+)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index bd7758b..d75dcf7 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -37,6 +37,7 @@
>>    */
>>   struct BdrvDirtyBitmap {
>>       HBitmap *bitmap;            /* Dirty sector bitmap 
>> implementation */
>> +    HBitmap *meta;              /* Meta dirty bitmap */
>>       BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen 
>> status */
>>       char *name;                 /* Optional non-empty unique ID */
>>       int64_t size;               /* Size of the bitmap (Number of 
>> sectors) */
>> @@ -102,6 +103,56 @@ BdrvDirtyBitmap 
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>       return bitmap;
>>   }
>>   +/* bdrv_create_meta_dirty_bitmap
>> + *
>> + * Create a meta dirty bitmap that tracks the changes of bits in 
>> @bitmap. I.e.
>> + * when a dirty status bit in @bitmap is changed (either from reset 
>> to set or
>> + * the other way around), its respective meta dirty bitmap bit will 
>> be marked
>> + * dirty as well.
>> + *
>> + * @bitmap: the block dirty bitmap for which to create a meta dirty 
>> bitmap.
>> + * @granularity: how many bytes of bitmap data does each bit in the 
>> meta bitmap
>> + * track.
>> + */
>> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>> +                                   int granularity)
>> +{
>> +    assert(!bitmap->meta);
>> +    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
>> +                                       BDRV_SECTOR_SIZE * 
>> BITS_PER_BYTE);
>> +}
>> +
>> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>> +{
>> +    assert(bitmap->meta);
>> +    hbitmap_free_meta(bitmap->bitmap);
>> +    bitmap->meta = NULL;
>> +}
>> +
>> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
>> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
>> +                               int nb_sectors)
>> +{
>> +    uint64_t i;
>> +    int gran = bdrv_dirty_bitmap_granularity(bitmap) >> 
>> BDRV_SECTOR_BITS;
>> +
>> +    /* To optimize: we can make hbitmap to internally check the 
>> range in a
>> +     * coarse level, or at least do it word by word. */
>> +    for (i = sector; i < sector + nb_sectors; i += gran) {
>> +        if (hbitmap_get(bitmap->meta, i)) {
>> +            return true;
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
>> +                                  BdrvDirtyBitmap *bitmap, int64_t 
>> sector,
>> +                                  int nb_sectors)
>> +{
>> +    hbitmap_reset(bitmap->meta, sector, nb_sectors);
>> +}
>> +
>>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
>>   {
>>       return bitmap->successor;
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index 120bac6..d9b281a 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -9,6 +9,9 @@ BdrvDirtyBitmap 
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>                                             uint32_t granularity,
>>                                             const char *name,
>>                                             Error **errp);
>> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>> +                                   int granularity);
>> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>>   int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>>                                          BdrvDirtyBitmap *bitmap,
>>                                          Error **errp);
>> @@ -35,6 +38,12 @@ 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);
>> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
>> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
>> +                               int nb_sectors);
>> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
>> +                                  BdrvDirtyBitmap *bitmap, int64_t 
>> sector,
>> +                                  int nb_sectors);
>>   BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>>                                            uint64_t first_sector);
>>   void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
>
> In my migration series I need iterators, get granularity, and 
> something like hbitmap_count  for meta bitmaps. You can add them here 
> if you want, or I can add them in my series.

Oh, sorry, I can't. I don't know what to do with active_iterators in 
this case, as it relates to bitmap, not to meta.. It is not as easy as 
it seemed. So, this should be resolved in these series.
John Snow Jan. 25, 2016, 10:16 p.m. UTC | #4
On 01/20/2016 01:11 AM, Fam Zheng wrote:
> The added group of operations enables tracking of the changed bits in
> the dirty bitmap.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  block/dirty-bitmap.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++
>  include/block/dirty-bitmap.h |  9 ++++++++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index bd7758b..d75dcf7 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -37,6 +37,7 @@
>   */
>  struct BdrvDirtyBitmap {
>      HBitmap *bitmap;            /* Dirty sector bitmap implementation */
> +    HBitmap *meta;              /* Meta dirty bitmap */
>      BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
>      char *name;                 /* Optional non-empty unique ID */
>      int64_t size;               /* Size of the bitmap (Number of sectors) */
> @@ -102,6 +103,56 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>      return bitmap;
>  }
>  
> +/* bdrv_create_meta_dirty_bitmap
> + *
> + * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e.
> + * when a dirty status bit in @bitmap is changed (either from reset to set or
> + * the other way around), its respective meta dirty bitmap bit will be marked
> + * dirty as well.
> + *
> + * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
> + * @granularity: how many bytes of bitmap data does each bit in the meta bitmap
> + * track.
> + */
> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> +                                   int granularity)
> +{
> +    assert(!bitmap->meta);
> +    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
> +                                       BDRV_SECTOR_SIZE * BITS_PER_BYTE);
> +}
> +
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> +    assert(bitmap->meta);
> +    hbitmap_free_meta(bitmap->bitmap);
> +    bitmap->meta = NULL;
> +}
> +
> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
> +                               int nb_sectors)
> +{
> +    uint64_t i;
> +    int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
> +
> +    /* To optimize: we can make hbitmap to internally check the range in a
> +     * coarse level, or at least do it word by word. */
> +    for (i = sector; i < sector + nb_sectors; i += gran) {
> +        if (hbitmap_get(bitmap->meta, i)) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> +                                  int nb_sectors)
> +{
> +    hbitmap_reset(bitmap->meta, sector, nb_sectors);
> +}
> +
>  bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
>  {
>      return bitmap->successor;
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 120bac6..d9b281a 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -9,6 +9,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                            uint32_t granularity,
>                                            const char *name,
>                                            Error **errp);
> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> +                                   int granularity);
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>  int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>                                         BdrvDirtyBitmap *bitmap,
>                                         Error **errp);
> @@ -35,6 +38,12 @@ 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);
> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> +                               BdrvDirtyBitmap *bitmap, int64_t sector,
> +                               int nb_sectors);
> +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> +                                  int nb_sectors);
>  BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>                                           uint64_t first_sector);
>  void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
>
Fam Zheng Jan. 26, 2016, 4:31 a.m. UTC | #5
On Fri, 01/22 14:34, Vladimir Sementsov-Ogievskiy wrote:
> >+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> >+                                   int granularity)
> >+{
> >+    assert(!bitmap->meta);
> >+    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
> >+                                       BDRV_SECTOR_SIZE * BITS_PER_BYTE);
> >+}
> 
> what is granularity here? Is it unused?
> 
> Here should be chunk_size parameter, which then will be somehow send
> to hbitmap_create_meta.

Right, it should replace BDRV_SECTOR_SIZE.

Fam
Fam Zheng Jan. 26, 2016, 6:25 a.m. UTC | #6
On Fri, 01/22 15:05, Vladimir Sementsov-Ogievskiy wrote:
> >In my migration series I need iterators, get granularity, and
> >something like hbitmap_count  for meta bitmaps. You can add them
> >here if you want, or I can add them in my series.

Okay, I can add that.

I have one more question on the interface: what dirty bitmaps are going to be
migrated? At this moment there are dirty bitmaps created by block jobs
(drive-mirror), and in-memory dirty bitmaps created for incremental backup;
later there will be persistent dirty bitmaps owned by block drivers (extended
version of qcow2, and in QBM driver I'm working on). Which of them are subject
of migration in your series?

I'm asking because I want to know whether we need to implement multiple meta
bitmaps on one block dirty bitmap.

Fam
Vladimir Sementsov-Ogievskiy Jan. 26, 2016, 7:49 a.m. UTC | #7
On 26.01.2016 09:25, Fam Zheng wrote:
> On Fri, 01/22 15:05, Vladimir Sementsov-Ogievskiy wrote:
>>> In my migration series I need iterators, get granularity, and
>>> something like hbitmap_count  for meta bitmaps. You can add them
>>> here if you want, or I can add them in my series.
> Okay, I can add that.
>
> I have one more question on the interface: what dirty bitmaps are going to be
> migrated? At this moment there are dirty bitmaps created by block jobs
> (drive-mirror), and in-memory dirty bitmaps created for incremental backup;
> later there will be persistent dirty bitmaps owned by block drivers (extended
> version of qcow2, and in QBM driver I'm working on). Which of them are subject
> of migration in your series?
>
> I'm asking because I want to know whether we need to implement multiple meta
> bitmaps on one block dirty bitmap.
>
> Fam
Only named bitmaps are migrated. For now, only qmp-created bitmaps are 
named. So, it can be in-memory dirty bitmaps or, in future, persistent 
dirty bitmaps.

Why multiple meta bitmaps? What is your plan about meta, should it be 
used for something other than migration? There was ideas about async 
loading persistent metabitmaps - maybe here? We can disallow processes 
using meta bitmaps to be simultaneous.
Fam Zheng Jan. 26, 2016, 8:23 a.m. UTC | #8
On Tue, 01/26 10:49, Vladimir Sementsov-Ogievskiy wrote:
> On 26.01.2016 09:25, Fam Zheng wrote:
> >On Fri, 01/22 15:05, Vladimir Sementsov-Ogievskiy wrote:
> >>>In my migration series I need iterators, get granularity, and
> >>>something like hbitmap_count  for meta bitmaps. You can add them
> >>>here if you want, or I can add them in my series.
> >Okay, I can add that.
> >
> >I have one more question on the interface: what dirty bitmaps are going to be
> >migrated? At this moment there are dirty bitmaps created by block jobs
> >(drive-mirror), and in-memory dirty bitmaps created for incremental backup;
> >later there will be persistent dirty bitmaps owned by block drivers (extended
> >version of qcow2, and in QBM driver I'm working on). Which of them are subject
> >of migration in your series?
> >
> >I'm asking because I want to know whether we need to implement multiple meta
> >bitmaps on one block dirty bitmap.
> >
> >Fam
> Only named bitmaps are migrated. For now, only qmp-created bitmaps
> are named. So, it can be in-memory dirty bitmaps or, in future,
> persistent dirty bitmaps.
> 
> Why multiple meta bitmaps?

The complication is from combining persistence and migration of a dirty bitmap.

To begin with, persistence drivers (qcow2 or QBM) would need meta dirty bitmaps
to avoid writing unchanged dirty bit ranges to disk, just like in migration.
This means if the persisted named dirty bitmap is being migrated, both the
block driver and migration code will then need their own meta dirty bitmaps,
that is where the question rose.

One step back, we haven't sorted out "migrating persistent dirty bitmap" at all
(see below). So it's probably okay to disallow that as the first step.

P.S. For discussion, I think we ultimately want users to be able to continue
their incremental backup even across the migration point.  If they're using
shared storage, I think it is possible even without dirty bitmap migration, by
flushing the dirty bitmap at src side then loading it at dest side. Otherwise
it is trickier as we will have to migrate the persisted dirty bitmap - the dest
side must use a capable format, and we need to check this capability when
migration starts.   By that time, the meta dirty bitmap interface can be
extended to allow at least two meta bitmaps on the same dirty bitmap.

Fam
diff mbox

Patch

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bd7758b..d75dcf7 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -37,6 +37,7 @@ 
  */
 struct BdrvDirtyBitmap {
     HBitmap *bitmap;            /* Dirty sector bitmap implementation */
+    HBitmap *meta;              /* Meta dirty bitmap */
     BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
     char *name;                 /* Optional non-empty unique ID */
     int64_t size;               /* Size of the bitmap (Number of sectors) */
@@ -102,6 +103,56 @@  BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
     return bitmap;
 }
 
+/* bdrv_create_meta_dirty_bitmap
+ *
+ * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e.
+ * when a dirty status bit in @bitmap is changed (either from reset to set or
+ * the other way around), its respective meta dirty bitmap bit will be marked
+ * dirty as well.
+ *
+ * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
+ * @granularity: how many bytes of bitmap data does each bit in the meta bitmap
+ * track.
+ */
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+                                   int granularity)
+{
+    assert(!bitmap->meta);
+    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
+                                       BDRV_SECTOR_SIZE * BITS_PER_BYTE);
+}
+
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+    assert(bitmap->meta);
+    hbitmap_free_meta(bitmap->bitmap);
+    bitmap->meta = NULL;
+}
+
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+                               BdrvDirtyBitmap *bitmap, int64_t sector,
+                               int nb_sectors)
+{
+    uint64_t i;
+    int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
+
+    /* To optimize: we can make hbitmap to internally check the range in a
+     * coarse level, or at least do it word by word. */
+    for (i = sector; i < sector + nb_sectors; i += gran) {
+        if (hbitmap_get(bitmap->meta, i)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+                                  BdrvDirtyBitmap *bitmap, int64_t sector,
+                                  int nb_sectors)
+{
+    hbitmap_reset(bitmap->meta, sector, nb_sectors);
+}
+
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
 {
     return bitmap->successor;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 120bac6..d9b281a 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -9,6 +9,9 @@  BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           uint32_t granularity,
                                           const char *name,
                                           Error **errp);
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+                                   int granularity);
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
                                        BdrvDirtyBitmap *bitmap,
                                        Error **errp);
@@ -35,6 +38,12 @@  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);
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+                               BdrvDirtyBitmap *bitmap, int64_t sector,
+                               int nb_sectors);
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+                                  BdrvDirtyBitmap *bitmap, int64_t sector,
+                                  int nb_sectors);
 BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
                                          uint64_t first_sector);
 void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);