[Qemu-devel] [PATCH 03/12] dirty-bitmap: Drop unused functions

Eric Blake posted 12 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 03/12] dirty-bitmap: Drop unused functions
Posted by Eric Blake 8 years, 10 months ago
We had several functions that no one was using, and which used
sector-based interfaces.  I'm trying to convert towards byte-based
interfaces, so it's easier to just drop the unused functions:

bdrv_dirty_bitmap_size
bdrv_dirty_bitmap_get_meta
bdrv_dirty_bitmap_reset_meta
bdrv_dirty_bitmap_meta_granularity

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/block/dirty-bitmap.h |  8 --------
 block/dirty-bitmap.c         | 34 ----------------------------------
 2 files changed, 42 deletions(-)

diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 9dea14b..a83979d 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -30,11 +30,9 @@ void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
 uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
-uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
 const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
-int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
 DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
                    int64_t sector);
@@ -42,12 +40,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                            int64_t cur_sector, int64_t nr_sectors);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                              int64_t cur_sector, int64_t 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_meta_iter_new(BdrvDirtyBitmap *bitmap);
 BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
                                          uint64_t first_sector);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 6d8ce5f..32698d5 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -130,35 +130,6 @@ void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
     bitmap->meta = NULL;
 }

-int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
-                               BdrvDirtyBitmap *bitmap, int64_t sector,
-                               int nb_sectors)
-{
-    uint64_t i;
-    int sectors_per_bit = 1 << hbitmap_granularity(bitmap->meta);
-
-    /* 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 += sectors_per_bit) {
-        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);
-}
-
-int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
-{
-    return bitmap->size;
-}
-
 const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
 {
     return bitmap->name;
@@ -393,11 +364,6 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
     return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
 }

-uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap)
-{
-    return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->meta);
-}
-
 BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
                                          uint64_t first_sector)
 {
-- 
2.9.3


Re: [Qemu-devel] [PATCH 03/12] dirty-bitmap: Drop unused functions
Posted by John Snow 8 years, 10 months ago

On 04/12/2017 01:49 PM, Eric Blake wrote:
> We had several functions that no one was using, and which used
> sector-based interfaces.  I'm trying to convert towards byte-based
> interfaces, so it's easier to just drop the unused functions:
> 
> bdrv_dirty_bitmap_size
> bdrv_dirty_bitmap_get_meta
> bdrv_dirty_bitmap_reset_meta
> bdrv_dirty_bitmap_meta_granularity
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  include/block/dirty-bitmap.h |  8 --------
>  block/dirty-bitmap.c         | 34 ----------------------------------
>  2 files changed, 42 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 9dea14b..a83979d 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -30,11 +30,9 @@ void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>  BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
>  uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
>  uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
> -uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap);
>  bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
>  bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
>  const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
> -int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
>  DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
>  int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
>                     int64_t sector);
> @@ -42,12 +40,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                             int64_t cur_sector, int64_t nr_sectors);
>  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                               int64_t cur_sector, int64_t 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_meta_iter_new(BdrvDirtyBitmap *bitmap);
>  BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>                                           uint64_t first_sector);
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 6d8ce5f..32698d5 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -130,35 +130,6 @@ void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>      bitmap->meta = NULL;
>  }
> 
> -int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> -                               BdrvDirtyBitmap *bitmap, int64_t sector,
> -                               int nb_sectors)
> -{
> -    uint64_t i;
> -    int sectors_per_bit = 1 << hbitmap_granularity(bitmap->meta);
> -
> -    /* 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 += sectors_per_bit) {
> -        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);
> -}
> -
> -int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
> -{
> -    return bitmap->size;
> -}
> -
>  const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
>  {
>      return bitmap->name;
> @@ -393,11 +364,6 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
>      return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
>  }
> 
> -uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap)
> -{
> -    return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->meta);
> -}
> -
>  BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>                                           uint64_t first_sector)
>  {
> 

I think it's likely Vladimir is or at least was relying on some of these
for his migration and persistence series.

Might be nice to let him chime in to see how much of a hassle this is.

Re: [Qemu-devel] [PATCH 03/12] dirty-bitmap: Drop unused functions
Posted by Eric Blake 8 years, 10 months ago
On 04/12/2017 05:47 PM, John Snow wrote:
> 
> 
> On 04/12/2017 01:49 PM, Eric Blake wrote:
>> We had several functions that no one was using, and which used
>> sector-based interfaces.  I'm trying to convert towards byte-based
>> interfaces, so it's easier to just drop the unused functions:
>>
>> bdrv_dirty_bitmap_size
>> bdrv_dirty_bitmap_get_meta
>> bdrv_dirty_bitmap_reset_meta
>> bdrv_dirty_bitmap_meta_granularity
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  include/block/dirty-bitmap.h |  8 --------
>>  block/dirty-bitmap.c         | 34 ----------------------------------
>>  2 files changed, 42 deletions(-)
>>

> 
> I think it's likely Vladimir is or at least was relying on some of these
> for his migration and persistence series.
> 
> Might be nice to let him chime in to see how much of a hassle this is.

Then let's add him in cc ;)

I'm okay if these functions stay because they have a user, but it would
also be nice if they were properly byte-based (like everything else in
dirty-bitmap at the end of my series).  So even if we remove them here,
we can revert the removal, and re-add them but with a sane interface.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 03/12] dirty-bitmap: Drop unused functions
Posted by John Snow 8 years, 10 months ago

On 04/12/2017 07:36 PM, Eric Blake wrote:
> On 04/12/2017 05:47 PM, John Snow wrote:
>>
>>
>> On 04/12/2017 01:49 PM, Eric Blake wrote:
>>> We had several functions that no one was using, and which used
>>> sector-based interfaces.  I'm trying to convert towards byte-based
>>> interfaces, so it's easier to just drop the unused functions:
>>>
>>> bdrv_dirty_bitmap_size
>>> bdrv_dirty_bitmap_get_meta
>>> bdrv_dirty_bitmap_reset_meta
>>> bdrv_dirty_bitmap_meta_granularity
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>  include/block/dirty-bitmap.h |  8 --------
>>>  block/dirty-bitmap.c         | 34 ----------------------------------
>>>  2 files changed, 42 deletions(-)
>>>
> 
>>
>> I think it's likely Vladimir is or at least was relying on some of these
>> for his migration and persistence series.
>>
>> Might be nice to let him chime in to see how much of a hassle this is.
> 
> Then let's add him in cc ;)
> 

Err... I can't just summon people by mentioning them?

> I'm okay if these functions stay because they have a user, but it would
> also be nice if they were properly byte-based (like everything else in
> dirty-bitmap at the end of my series).  So even if we remove them here,
> we can revert the removal, and re-add them but with a sane interface.
> 

OK, but I will offer to do the work in the interest of not slowing
things down any further.

Do you use any of these, Vladimir?

--js

Re: [Qemu-devel] [PATCH 03/12] dirty-bitmap: Drop unused functions
Posted by Vladimir Sementsov-Ogievskiy 8 years, 10 months ago
13.04.2017 02:40, John Snow wrote:
>
> On 04/12/2017 07:36 PM, Eric Blake wrote:
>> On 04/12/2017 05:47 PM, John Snow wrote:
>>>
>>> On 04/12/2017 01:49 PM, Eric Blake wrote:
>>>> We had several functions that no one was using, and which used
>>>> sector-based interfaces.  I'm trying to convert towards byte-based
>>>> interfaces, so it's easier to just drop the unused functions:
>>>>
>>>> bdrv_dirty_bitmap_size
>>>> bdrv_dirty_bitmap_get_meta
>>>> bdrv_dirty_bitmap_reset_meta
>>>> bdrv_dirty_bitmap_meta_granularity
>>>>
>>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>>> ---
>>>>   include/block/dirty-bitmap.h |  8 --------
>>>>   block/dirty-bitmap.c         | 34 ----------------------------------
>>>>   2 files changed, 42 deletions(-)
>>>>
>>> I think it's likely Vladimir is or at least was relying on some of these
>>> for his migration and persistence series.
>>>
>>> Might be nice to let him chime in to see how much of a hassle this is.
>> Then let's add him in cc ;)
>>
> Err... I can't just summon people by mentioning them?
>
>> I'm okay if these functions stay because they have a user, but it would
>> also be nice if they were properly byte-based (like everything else in
>> dirty-bitmap at the end of my series).  So even if we remove them here,
>> we can revert the removal, and re-add them but with a sane interface.
>>
> OK, but I will offer to do the work in the interest of not slowing
> things down any further.
>
> Do you use any of these, Vladimir?
>
> --js
HI all!

only bdrv_dirty_bitmap_size() is used. No problem to add it back with my 
series next update


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH 03/12] dirty-bitmap: Drop unused functions
Posted by John Snow 8 years, 10 months ago

On 04/13/2017 05:19 AM, Vladimir Sementsov-Ogievskiy wrote:
> 13.04.2017 02:40, John Snow wrote:
>>
>> On 04/12/2017 07:36 PM, Eric Blake wrote:
>>> On 04/12/2017 05:47 PM, John Snow wrote:
>>>>
>>>> On 04/12/2017 01:49 PM, Eric Blake wrote:
>>>>> We had several functions that no one was using, and which used
>>>>> sector-based interfaces.  I'm trying to convert towards byte-based
>>>>> interfaces, so it's easier to just drop the unused functions:
>>>>>
>>>>> bdrv_dirty_bitmap_size
>>>>> bdrv_dirty_bitmap_get_meta
>>>>> bdrv_dirty_bitmap_reset_meta
>>>>> bdrv_dirty_bitmap_meta_granularity
>>>>>
>>>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>>>> ---
>>>>>   include/block/dirty-bitmap.h |  8 --------
>>>>>   block/dirty-bitmap.c         | 34 ----------------------------------
>>>>>   2 files changed, 42 deletions(-)
>>>>>
>>>> I think it's likely Vladimir is or at least was relying on some of
>>>> these
>>>> for his migration and persistence series.
>>>>
>>>> Might be nice to let him chime in to see how much of a hassle this is.
>>> Then let's add him in cc ;)
>>>
>> Err... I can't just summon people by mentioning them?
>>
>>> I'm okay if these functions stay because they have a user, but it would
>>> also be nice if they were properly byte-based (like everything else in
>>> dirty-bitmap at the end of my series).  So even if we remove them here,
>>> we can revert the removal, and re-add them but with a sane interface.
>>>
>> OK, but I will offer to do the work in the interest of not slowing
>> things down any further.
>>
>> Do you use any of these, Vladimir?
>>
>> --js
> HI all!
> 
> only bdrv_dirty_bitmap_size() is used. No problem to add it back with my
> series next update
> 
> 

Excellent, thanks!

In that case:

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

Should be for the whole series, IIRC.

--js