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

Eric Blake posted 2 patches 8 years, 2 months ago
[Qemu-devel] [PATCH 1/2] dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
Posted by Eric Blake 8 years, 2 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>
Reviewed-by: John Snow <jsnow@redhat.com>

---
Too late for 2.9, since the regression has been unnoticed for
nine releases. But worth putting in 2.9.1 and 2.10.

v2-v4: no change
---
 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 543bddb9b5..30462d4f9a 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -461,7 +461,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.13.3


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

On 07/21/2017 02:32 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>
> Reviewed-by: John Snow <jsnow@redhat.com>
> 

I must have forgotten about this -- Didn't this get reviewed as part of 
your byte series? Is this a resend for a stable branch?

> ---
> Too late for 2.9, since the regression has been unnoticed for
> nine releases. But worth putting in 2.9.1 and 2.10.
> 
> v2-v4: no change
> ---
>   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 543bddb9b5..30462d4f9a 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -461,7 +461,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);
> 

-- 
—js

Re: [Qemu-devel] [PATCH 1/2] dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
Posted by Eric Blake 8 years, 2 months ago
On 07/25/2017 04:28 PM, John Snow wrote:
> 
> 
> On 07/21/2017 02:32 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>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>>
> 
> I must have forgotten about this -- Didn't this get reviewed as part of
> your byte series? Is this a resend for a stable branch?

Yes, it was reviewed during my part 2-of-4 series on byte-base block
status (the dirty bitmap series).  Series 1 made it into 2.10, but
series 2-4 did not; ergo, I filtered out the true bug-fixes that were
still worthy post-freeze.

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

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

On 07/25/2017 05:37 PM, Eric Blake wrote:
> On 07/25/2017 04:28 PM, John Snow wrote:
>>
>>
>> On 07/21/2017 02:32 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>
>>> Reviewed-by: John Snow <jsnow@redhat.com>
>>>
>>
>> I must have forgotten about this -- Didn't this get reviewed as part of
>> your byte series? Is this a resend for a stable branch?
> 
> Yes, it was reviewed during my part 2-of-4 series on byte-base block
> status (the dirty bitmap series).  Series 1 made it into 2.10, but
> series 2-4 did not; ergo, I filtered out the true bug-fixes that were
> still worthy post-freeze.
> 

Thanks for the clarification. Short term memory is a little strapped 
right now due to my move.

ACK