[Qemu-devel] [PATCH] block: remove bdrv_dirty_bitmap_make_anon

Paolo Bonzini posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180323164254.26487-1-pbonzini@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 failed
Test s390x passed
block/dirty-bitmap.c         | 9 ---------
blockdev.c                   | 1 -
include/block/dirty-bitmap.h | 1 -
3 files changed, 11 deletions(-)
[Qemu-devel] [PATCH] block: remove bdrv_dirty_bitmap_make_anon
Posted by Paolo Bonzini 6 years ago
All this function is doing will be repeated by
bdrv_do_release_matching_dirty_bitmap_locked, except
resetting bm->persistent.  But even that does not matter
because the bitmap will be freed.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/dirty-bitmap.c         | 9 ---------
 blockdev.c                   | 1 -
 include/block/dirty-bitmap.h | 1 -
 3 files changed, 11 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index e27b3096dd..13bb5066a4 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -97,15 +97,6 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
     return NULL;
 }
 
-/* Called with BQL taken.  */
-void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
-{
-    assert(!bdrv_dirty_bitmap_frozen(bitmap));
-    g_free(bitmap->name);
-    bitmap->name = NULL;
-    bitmap->persistent = false;
-}
-
 /* Called with BQL taken.  */
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           uint32_t granularity,
diff --git a/blockdev.c b/blockdev.c
index c31bf3d98d..6eb18c4ef1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2881,7 +2881,6 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
         }
     }
 
-    bdrv_dirty_bitmap_make_anon(bitmap);
     bdrv_release_dirty_bitmap(bs, bitmap);
 }
 
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 1ff8949b1b..5a51a78b63 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -24,7 +24,6 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
 void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap);
 BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
                                         const char *name);
-void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs);
 void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs);
-- 
2.16.2


Re: [Qemu-devel] [PATCH] block: remove bdrv_dirty_bitmap_make_anon
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
23.03.2018 19:42, Paolo Bonzini wrote:
> All this function is doing will be repeated by
> bdrv_do_release_matching_dirty_bitmap_locked, except
> resetting bm->persistent.  But even that does not matter
> because the bitmap will be freed.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [Qemu-block] [PATCH] block: remove bdrv_dirty_bitmap_make_anon
Posted by Stefan Hajnoczi 6 years ago
On Fri, Mar 23, 2018 at 05:42:53PM +0100, Paolo Bonzini wrote:
> All this function is doing will be repeated by
> bdrv_do_release_matching_dirty_bitmap_locked, except
> resetting bm->persistent.  But even that does not matter
> because the bitmap will be freed.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/dirty-bitmap.c         | 9 ---------
>  blockdev.c                   | 1 -
>  include/block/dirty-bitmap.h | 1 -
>  3 files changed, 11 deletions(-)

CCing Fam and John, who maintain block/dirty-bitmap.c.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [Qemu-devel] [Qemu-block] [PATCH] block: remove bdrv_dirty_bitmap_make_anon
Posted by John Snow 6 years ago

On 03/23/2018 12:42 PM, Paolo Bonzini wrote:
> All this function is doing will be repeated by
> bdrv_do_release_matching_dirty_bitmap_locked, except
> resetting bm->persistent.  But even that does not matter
> because the bitmap will be freed.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/dirty-bitmap.c         | 9 ---------
>  blockdev.c                   | 1 -
>  include/block/dirty-bitmap.h | 1 -
>  3 files changed, 11 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index e27b3096dd..13bb5066a4 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -97,15 +97,6 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
>      return NULL;
>  }
>  
> -/* Called with BQL taken.  */
> -void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
> -{
> -    assert(!bdrv_dirty_bitmap_frozen(bitmap));
> -    g_free(bitmap->name);
> -    bitmap->name = NULL;
> -    bitmap->persistent = false;
> -}
> -
>  /* Called with BQL taken.  */
>  BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                            uint32_t granularity,
> diff --git a/blockdev.c b/blockdev.c
> index c31bf3d98d..6eb18c4ef1 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2881,7 +2881,6 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
>          }
>      }
>  
> -    bdrv_dirty_bitmap_make_anon(bitmap);
>      bdrv_release_dirty_bitmap(bs, bitmap);
>  }
>  
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 1ff8949b1b..5a51a78b63 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -24,7 +24,6 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
>  void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap);
>  BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
>                                          const char *name);
> -void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
>  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
>  void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs);
>  void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs);
> 

Just a weird artifact of the way in which we used to think this
subsystem would work; that maybe we'd want to return named bitmaps to
being anonymous.

Thanks, applied to my bitmaps tree:

https://github.com/jnsnow/qemu/commits/bitmaps
https://github.com/jnsnow/qemu.git

--js