[Qemu-devel] [PATCH 01/12] dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented

Eric Blake posted 12 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 01/12] dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
Posted by Eric Blake 8 years, 10 months ago
We've been documenting the value in bytes since its introduction
in commit b9a9b3a4 (v1.3), where it was actually reported in bytes.

Commit e4654d2 (v2.0) then removed things from block/qapi.c, in
preparation for a rewrite to a list of dirty sectors in the next
commit 21b5683 in block.c, but the new code mistakenly started
reporting in sectors.

Fixes: https://bugzilla.redhat.com/1441460

CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>

---
Too late for 2.9, since the regression has been unnoticed for
nine releases. But worth putting in 2.9.1.
---
 block/dirty-bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 519737c..6d8ce5f 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -345,7 +345,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
     QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
         BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
         BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
-        info->count = bdrv_get_dirty_count(bm);
+        info->count = bdrv_get_dirty_count(bm) << BDRV_SECTOR_BITS;
         info->granularity = bdrv_dirty_bitmap_granularity(bm);
         info->has_name = !!bm->name;
         info->name = g_strdup(bm->name);
-- 
2.9.3


Re: [Qemu-devel] [PATCH 01/12] dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
Posted by John Snow 8 years, 10 months ago

On 04/12/2017 01:49 PM, Eric Blake wrote:
> We've been documenting the value in bytes since its introduction
> in commit b9a9b3a4 (v1.3), where it was actually reported in bytes.
> 
> Commit e4654d2 (v2.0) then removed things from block/qapi.c, in
> preparation for a rewrite to a list of dirty sectors in the next
> commit 21b5683 in block.c, but the new code mistakenly started
> reporting in sectors.
> 
> Fixes: https://bugzilla.redhat.com/1441460
> 
> CC: qemu-stable@nongnu.org
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> Too late for 2.9, since the regression has been unnoticed for
> nine releases. But worth putting in 2.9.1.
> ---

Since before I started working here :)

I even documented the wrong thing in my two talks on the matter, but I
suppose I never committed it to bitmaps.md, so...

I guess this is technically correct?

http://i3.kym-cdn.com/photos/images/facebook/000/909/991/48c.jpg

>  block/dirty-bitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 519737c..6d8ce5f 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -345,7 +345,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>      QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
>          BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
>          BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
> -        info->count = bdrv_get_dirty_count(bm);
> +        info->count = bdrv_get_dirty_count(bm) << BDRV_SECTOR_BITS;
>          info->granularity = bdrv_dirty_bitmap_granularity(bm);
>          info->has_name = !!bm->name;
>          info->name = g_strdup(bm->name);
> 

This is strictly more useful than sectors anyway, so ...

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