All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-4.1] util/hbitmap: update orig_size on truncate
@ 2019-08-05 12:01 Vladimir Sementsov-Ogievskiy
  2019-08-05 12:19 ` Max Reitz
  2019-08-05 15:38 ` Max Reitz
  0 siblings, 2 replies; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-08-05 12:01 UTC (permalink / raw)
  To: qemu-block; +Cc: fam, kwolf, vsementsov, qemu-devel, mreitz, den, jsnow

Without this, hbitmap_next_zero and hbitmap_next_dirty_area are broken
after truncate. So, orig_size is broken since it's introduction in
76d570dc495c56bb.

Fixes: 76d570dc495c56bb
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---

Hi!

Here is one more hbitmap bug I noticed. It's my fault, I'm sorry :(
Broken in 4.0, but if we are already going to fix in 4.1 some things
around it, it's a small meaningful addition.

Users of broken API are incremental backup, sync mirror (but it may not
be broken, if truncates not allowed during mirror, are they?), bitmap
export through NBD.

 util/hbitmap.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/util/hbitmap.c b/util/hbitmap.c
index 7905212a8b..bcc0acdc6a 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -53,7 +53,9 @@
  */
 
 struct HBitmap {
-    /* Size of the bitmap, as requested in hbitmap_alloc. */
+    /*
+     * Size of the bitmap, as requested in hbitmap_alloc or in hbitmap_truncate.
+     */
     uint64_t orig_size;
 
     /* Number of total bits in the bottom level.  */
@@ -732,6 +734,8 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size)
     uint64_t num_elements = size;
     uint64_t old;
 
+    hb->orig_size = size;
+
     /* Size comes in as logical elements, adjust for granularity. */
     size = (size + (1ULL << hb->granularity) - 1) >> hb->granularity;
     assert(size <= ((uint64_t)1 << HBITMAP_LOG_MAX_SIZE));
-- 
2.18.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-4.1] util/hbitmap: update orig_size on truncate
  2019-08-05 12:01 [Qemu-devel] [PATCH for-4.1] util/hbitmap: update orig_size on truncate Vladimir Sementsov-Ogievskiy
@ 2019-08-05 12:19 ` Max Reitz
  2019-08-05 14:50   ` Paolo Bonzini
  2019-08-05 15:38 ` Max Reitz
  1 sibling, 1 reply; 4+ messages in thread
From: Max Reitz @ 2019-08-05 12:19 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: fam, kwolf, qemu-devel, den, jsnow


[-- Attachment #1.1: Type: text/plain, Size: 1095 bytes --]

On 05.08.19 14:01, Vladimir Sementsov-Ogievskiy wrote:
> Without this, hbitmap_next_zero and hbitmap_next_dirty_area are broken
> after truncate. So, orig_size is broken since it's introduction in
> 76d570dc495c56bb.
> 
> Fixes: 76d570dc495c56bb
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 
> Hi!
> 
> Here is one more hbitmap bug I noticed. It's my fault, I'm sorry :(
> Broken in 4.0, but if we are already going to fix in 4.1 some things
> around it, it's a small meaningful addition.

Hm. :-/

> Users of broken API are incremental backup, sync mirror (but it may not
> be broken, if truncates not allowed during mirror, are they?),

It doesn’t appear that way (we don’t share BLK_PERM_RESIZE).

> bitmap export through NBD.

I suppose that counts as block-y enough for me to take it.  Well, I’d
still like a test case to see the impact...  I’ll see whether I can come
up with something.

>  util/hbitmap.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-4.1] util/hbitmap: update orig_size on truncate
  2019-08-05 12:19 ` Max Reitz
@ 2019-08-05 14:50   ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2019-08-05 14:50 UTC (permalink / raw)
  To: Max Reitz, Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: fam, kwolf, jsnow, qemu-devel, den

On 05/08/19 14:19, Max Reitz wrote:
> On 05.08.19 14:01, Vladimir Sementsov-Ogievskiy wrote:
>> Without this, hbitmap_next_zero and hbitmap_next_dirty_area are broken
>> after truncate. So, orig_size is broken since it's introduction in
>> 76d570dc495c56bb.
>>
>> Fixes: 76d570dc495c56bb
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>
>> Hi!
>>
>> Here is one more hbitmap bug I noticed. It's my fault, I'm sorry :(
>> Broken in 4.0, but if we are already going to fix in 4.1 some things
>> around it, it's a small meaningful addition.
> 
> Hm. :-/
> 
>> Users of broken API are incremental backup, sync mirror (but it may not
>> be broken, if truncates not allowed during mirror, are they?),
> 
> It doesn’t appear that way (we don’t share BLK_PERM_RESIZE).
> 
>> bitmap export through NBD.
> 
> I suppose that counts as block-y enough for me to take it.  Well, I’d
> still like a test case to see the impact...  I’ll see whether I can come
> up with something.

And also a unit test...

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-4.1] util/hbitmap: update orig_size on truncate
  2019-08-05 12:01 [Qemu-devel] [PATCH for-4.1] util/hbitmap: update orig_size on truncate Vladimir Sementsov-Ogievskiy
  2019-08-05 12:19 ` Max Reitz
@ 2019-08-05 15:38 ` Max Reitz
  1 sibling, 0 replies; 4+ messages in thread
From: Max Reitz @ 2019-08-05 15:38 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: fam, kwolf, qemu-devel, den, jsnow


[-- Attachment #1.1: Type: text/plain, Size: 524 bytes --]

On 05.08.19 14:01, Vladimir Sementsov-Ogievskiy wrote:
> Without this, hbitmap_next_zero and hbitmap_next_dirty_area are broken
> after truncate. So, orig_size is broken since it's introduction in
> 76d570dc495c56bb.
> 
> Fixes: 76d570dc495c56bb
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

A unit test as a follow-up would indeed be nice, but I’ll just take it
now anyway.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-08-05 15:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-05 12:01 [Qemu-devel] [PATCH for-4.1] util/hbitmap: update orig_size on truncate Vladimir Sementsov-Ogievskiy
2019-08-05 12:19 ` Max Reitz
2019-08-05 14:50   ` Paolo Bonzini
2019-08-05 15:38 ` Max Reitz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.