[Qemu-devel] [PATCH v2 10/18] block/dirty-bitmap: add bdrv_dirty_bitmap_get

John Snow posted 18 patches 6 years, 7 months ago
Maintainers: Eric Blake <eblake@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Xie Changlong <xiechanglong.d@gmail.com>, John Snow <jsnow@redhat.com>, Juan Quintela <quintela@redhat.com>, Max Reitz <mreitz@redhat.com>, Fam Zheng <fam@euphon.net>, Wen Congyang <wencongyang2@huawei.com>, Markus Armbruster <armbru@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v2 10/18] block/dirty-bitmap: add bdrv_dirty_bitmap_get
Posted by John Snow 6 years, 7 months ago
Add a public interface for get. While we're at it,
rename "bdrv_get_dirty_bitmap_locked" to "bdrv_dirty_bitmap_get_locked".

(There are more functions to rename to the bdrv_dirty_bitmap_VERB form,
but they will wait until the conclusion of this series.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/dirty-bitmap.c         | 18 +++++++++++-------
 block/mirror.c               |  2 +-
 include/block/dirty-bitmap.h |  4 ++--
 migration/block.c            |  5 ++---
 nbd/server.c                 |  2 +-
 5 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index b0f76826b3..97541521ab 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -509,14 +509,18 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 }
 
 /* Called within bdrv_dirty_bitmap_lock..unlock */
-bool bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
-                           int64_t offset)
+bool bdrv_dirty_bitmap_get_locked(BdrvDirtyBitmap *bitmap, int64_t offset)
 {
-    if (bitmap) {
-        return hbitmap_get(bitmap->bitmap, offset);
-    } else {
-        return false;
-    }
+    return hbitmap_get(bitmap->bitmap, offset);
+}
+
+bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset) {
+    bool ret;
+    bdrv_dirty_bitmap_lock(bitmap);
+    ret = bdrv_dirty_bitmap_get_locked(bitmap, offset);
+    bdrv_dirty_bitmap_unlock(bitmap);
+
+    return ret;
 }
 
 /**
diff --git a/block/mirror.c b/block/mirror.c
index 42b3d9acd0..1da57409f0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -476,7 +476,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         int64_t next_offset = offset + nb_chunks * s->granularity;
         int64_t next_chunk = next_offset / s->granularity;
         if (next_offset >= s->bdev_length ||
-            !bdrv_get_dirty_locked(source, s->dirty_bitmap, next_offset)) {
+            !bdrv_dirty_bitmap_get_locked(s->dirty_bitmap, next_offset)) {
             break;
         }
         if (test_bit(next_chunk, s->in_flight_bitmap)) {
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 62682eb865..0120ef3f05 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -84,12 +84,12 @@ void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy);
 void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
                              HBitmap **backup, Error **errp);
 void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration);
+bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset);
 
 /* Functions that require manual locking.  */
 void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_unlock(BdrvDirtyBitmap *bitmap);
-bool bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
-                           int64_t offset);
+bool bdrv_dirty_bitmap_get_locked(BdrvDirtyBitmap *bitmap, int64_t offset);
 void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
                                   int64_t offset, int64_t bytes);
 void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
diff --git a/migration/block.c b/migration/block.c
index 91f98ef44a..a5b60456ae 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -520,7 +520,6 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
                                  int is_async)
 {
     BlkMigBlock *blk;
-    BlockDriverState *bs = blk_bs(bmds->blk);
     int64_t total_sectors = bmds->total_sectors;
     int64_t sector;
     int nr_sectors;
@@ -535,8 +534,8 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
             blk_mig_unlock();
         }
         bdrv_dirty_bitmap_lock(bmds->dirty_bitmap);
-        if (bdrv_get_dirty_locked(bs, bmds->dirty_bitmap,
-                                  sector * BDRV_SECTOR_SIZE)) {
+        if (bdrv_dirty_bitmap_get_locked(bmds->dirty_bitmap,
+                                         sector * BDRV_SECTOR_SIZE)) {
             if (total_sectors - sector < BDRV_SECTORS_PER_DIRTY_CHUNK) {
                 nr_sectors = total_sectors - sector;
             } else {
diff --git a/nbd/server.c b/nbd/server.c
index 10faedcfc5..fbd51b48a7 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2003,7 +2003,7 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
     bdrv_dirty_bitmap_lock(bitmap);
 
     it = bdrv_dirty_iter_new(bitmap);
-    dirty = bdrv_get_dirty_locked(NULL, bitmap, offset);
+    dirty = bdrv_dirty_bitmap_get_locked(bitmap, offset);
 
     assert(begin < overall_end && nb_extents);
     while (begin < overall_end && i < nb_extents) {
-- 
2.21.0


Re: [Qemu-devel] [PATCH v2 10/18] block/dirty-bitmap: add bdrv_dirty_bitmap_get
Posted by Max Reitz 6 years, 7 months ago
On 03.07.19 23:55, John Snow wrote:
> Add a public interface for get. While we're at it,
> rename "bdrv_get_dirty_bitmap_locked" to "bdrv_dirty_bitmap_get_locked".
> 
> (There are more functions to rename to the bdrv_dirty_bitmap_VERB form,
> but they will wait until the conclusion of this series.)
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/dirty-bitmap.c         | 18 +++++++++++-------
>  block/mirror.c               |  2 +-
>  include/block/dirty-bitmap.h |  4 ++--
>  migration/block.c            |  5 ++---
>  nbd/server.c                 |  2 +-
>  5 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index b0f76826b3..97541521ab 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -509,14 +509,18 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>  }
>  
>  /* Called within bdrv_dirty_bitmap_lock..unlock */
> -bool bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
> -                           int64_t offset)
> +bool bdrv_dirty_bitmap_get_locked(BdrvDirtyBitmap *bitmap, int64_t offset)
>  {
> -    if (bitmap) {
> -        return hbitmap_get(bitmap->bitmap, offset);
> -    } else {
> -        return false;
> -    }
> +    return hbitmap_get(bitmap->bitmap, offset);
> +}
> +
> +bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset) {

I’m sure Patchew has told this already, but this is not Rust yet.

With that fixed:

Reviewed-by: Max Reitz <mreitz@redhat.com>

> +    bool ret;
> +    bdrv_dirty_bitmap_lock(bitmap);
> +    ret = bdrv_dirty_bitmap_get_locked(bitmap, offset);
> +    bdrv_dirty_bitmap_unlock(bitmap);
> +
> +    return ret;
>  }
>  
>  /**

Re: [Qemu-devel] [PATCH v2 10/18] block/dirty-bitmap: add bdrv_dirty_bitmap_get
Posted by John Snow 6 years, 7 months ago

On 7/4/19 1:01 PM, Max Reitz wrote:
> On 03.07.19 23:55, John Snow wrote:
>> Add a public interface for get. While we're at it,
>> rename "bdrv_get_dirty_bitmap_locked" to "bdrv_dirty_bitmap_get_locked".
>>
>> (There are more functions to rename to the bdrv_dirty_bitmap_VERB form,
>> but they will wait until the conclusion of this series.)
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  block/dirty-bitmap.c         | 18 +++++++++++-------
>>  block/mirror.c               |  2 +-
>>  include/block/dirty-bitmap.h |  4 ++--
>>  migration/block.c            |  5 ++---
>>  nbd/server.c                 |  2 +-
>>  5 files changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index b0f76826b3..97541521ab 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -509,14 +509,18 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>>  }
>>  
>>  /* Called within bdrv_dirty_bitmap_lock..unlock */
>> -bool bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
>> -                           int64_t offset)
>> +bool bdrv_dirty_bitmap_get_locked(BdrvDirtyBitmap *bitmap, int64_t offset)
>>  {
>> -    if (bitmap) {
>> -        return hbitmap_get(bitmap->bitmap, offset);
>> -    } else {
>> -        return false;
>> -    }
>> +    return hbitmap_get(bitmap->bitmap, offset);
>> +}
>> +
>> +bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset) {
> 
> I’m sure Patchew has told this already, but this is not Rust yet.
> 
> With that fixed:
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 

Why wait for rust to start living dangerously?

(Actually, just a thinko: I've been doing so much python lately that I
can only just barely remember to use a { instead of a :, and ...)