diff mbox

[v2,4/7] dirty-bitmap: separate unused meta-bitmap related functions

Message ID 20180416114414.18406-5-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy April 16, 2018, 11:44 a.m. UTC 
Separate them in the header and clarify needed locking in comments.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/dirty-bitmap.h | 14 +++++++++-----
 block/dirty-bitmap.c         |  5 +++++
 2 files changed, 14 insertions(+), 5 deletions(-)

Comments

John Snow April 19, 2018, 9:54 p.m. UTC | #1 
On 04/16/2018 07:44 AM, Vladimir Sementsov-Ogievskiy wrote:
> Separate them in the header and clarify needed locking in comments.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/block/dirty-bitmap.h | 14 +++++++++-----
>  block/dirty-bitmap.c         |  5 +++++
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index c7e910016d..b7ccfd1363 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -9,9 +9,6 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                            uint32_t granularity,
>                                            const char *name,
>                                            Error **errp);
> -void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> -                                   int chunk_size);
> -void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>  int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>                                         BdrvDirtyBitmap *bitmap,
>                                         Error **errp);
> @@ -45,7 +42,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                             int64_t offset, int64_t bytes);
>  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                               int64_t offset, int64_t bytes);
> -BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
>  BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap);
>  void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
>  
> @@ -84,7 +80,6 @@ void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>  int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>  void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
> -int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
>  void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
>  bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>  bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
> @@ -99,4 +94,13 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
>                                                    BdrvDirtyBitmap *bitmap,
>                                                    Error **errp);
>  
> +/*
> + * Unused for now meta-bitmaps related functions
> + */
> +

I assume you have plans to use them still? I thought these were useful
for storage migration -- but I guess we don't use these for the postcopy
mechanism?

> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, int chunk_size);
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
> +BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
> +int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> +
>  #endif
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 6c00288fd7..1812e17549 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -149,6 +149,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>   * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
>   * @chunk_size: how many bytes of bitmap data does each bit in the meta bitmap
>   * track.
> + *
> + * Called with BQL taken.
>   */
>  void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                                     int chunk_size)
> @@ -160,6 +162,7 @@ void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>      qemu_mutex_unlock(bitmap->mutex);
>  }
>  
> +/* Called with BQL taken. */
>  void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>  {
>      assert(bitmap->meta);
> @@ -529,6 +532,7 @@ BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap)
>      return iter;
>  }
>  
> +/* Called with BQL and dirty_bitmap_mutex locked. */
>  BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap)
>  {
>      BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
> @@ -688,6 +692,7 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>      return hbitmap_count(bitmap->bitmap);
>  }
>  
> +/* Called with BQL or dirty_bitmap_mutex locked */
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
>  {
>      return hbitmap_count(bitmap->meta);
>
Vladimir Sementsov-Ogievskiy April 20, 2018, 12:23 p.m. UTC | #2 
20.04.2018 00:54, John Snow wrote:
> On 04/16/2018 07:44 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Separate them in the header and clarify needed locking in comments.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   include/block/dirty-bitmap.h | 14 +++++++++-----
>>   block/dirty-bitmap.c         |  5 +++++
>>   2 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index c7e910016d..b7ccfd1363 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -9,9 +9,6 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>                                             uint32_t granularity,
>>                                             const char *name,
>>                                             Error **errp);
>> -void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>> -                                   int chunk_size);
>> -void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>>   int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>>                                          BdrvDirtyBitmap *bitmap,
>>                                          Error **errp);
>> @@ -45,7 +42,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>                              int64_t offset, int64_t bytes);
>>   void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>                                int64_t offset, int64_t bytes);
>> -BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
>>   BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap);
>>   void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
>>   
>> @@ -84,7 +80,6 @@ void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>>   int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>>   void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset);
>>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>> -int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
>>   void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
>>   bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>>   bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>> @@ -99,4 +94,13 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
>>                                                     BdrvDirtyBitmap *bitmap,
>>                                                     Error **errp);
>>   
>> +/*
>> + * Unused for now meta-bitmaps related functions
>> + */
>> +
> I assume you have plans to use them still? I thought these were useful
> for storage migration -- but I guess we don't use these for the postcopy
> mechanism?

I don't have plans for near future, so for me it's ok to drop them.

Theoretical usage is partial storing (if we know, which portions of 
bitmaps are changed, we can save to file only these portions, not the 
whole bitmaps), but it don't look like near future. May be, Fam has some 
plans?

>
>> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, int chunk_size);
>> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>> +BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
>> +int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
>> +
>>   #endif
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 6c00288fd7..1812e17549 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -149,6 +149,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>    * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
>>    * @chunk_size: how many bytes of bitmap data does each bit in the meta bitmap
>>    * track.
>> + *
>> + * Called with BQL taken.
>>    */
>>   void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>                                      int chunk_size)
>> @@ -160,6 +162,7 @@ void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>       qemu_mutex_unlock(bitmap->mutex);
>>   }
>>   
>> +/* Called with BQL taken. */
>>   void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>>   {
>>       assert(bitmap->meta);
>> @@ -529,6 +532,7 @@ BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap)
>>       return iter;
>>   }
>>   
>> +/* Called with BQL and dirty_bitmap_mutex locked. */
>>   BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap)
>>   {
>>       BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
>> @@ -688,6 +692,7 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>>       return hbitmap_count(bitmap->bitmap);
>>   }
>>   
>> +/* Called with BQL or dirty_bitmap_mutex locked */
>>   int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
>>   {
>>       return hbitmap_count(bitmap->meta);
>>
diff mbox

Patch

diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index c7e910016d..b7ccfd1363 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -9,9 +9,6 @@  BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           uint32_t granularity,
                                           const char *name,
                                           Error **errp);
-void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-                                   int chunk_size);
-void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
                                        BdrvDirtyBitmap *bitmap,
                                        Error **errp);
@@ -45,7 +42,6 @@  void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                            int64_t offset, int64_t bytes);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                              int64_t offset, int64_t bytes);
-BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
 BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
 
@@ -84,7 +80,6 @@  void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
 int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
 void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
-int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
 bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
 bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
@@ -99,4 +94,13 @@  BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
                                                   BdrvDirtyBitmap *bitmap,
                                                   Error **errp);
 
+/*
+ * Unused for now meta-bitmaps related functions
+ */
+
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, int chunk_size);
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
+int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
+
 #endif
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 6c00288fd7..1812e17549 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -149,6 +149,8 @@  BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
  * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
  * @chunk_size: how many bytes of bitmap data does each bit in the meta bitmap
  * track.
+ *
+ * Called with BQL taken.
  */
 void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                                    int chunk_size)
@@ -160,6 +162,7 @@  void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
     qemu_mutex_unlock(bitmap->mutex);
 }
 
+/* Called with BQL taken. */
 void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
     assert(bitmap->meta);
@@ -529,6 +532,7 @@  BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap)
     return iter;
 }
 
+/* Called with BQL and dirty_bitmap_mutex locked. */
 BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap)
 {
     BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
@@ -688,6 +692,7 @@  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
     return hbitmap_count(bitmap->bitmap);
 }
 
+/* Called with BQL or dirty_bitmap_mutex locked */
 int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
 {
     return hbitmap_count(bitmap->meta);