We have been neglecting to do so, which results in wrong counts
after merge. In the worst case, we may think the bitmap is empty
when it has had new writes merged into it.
Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
util/hbitmap.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/util/hbitmap.c b/util/hbitmap.c
index d5aca5159f..28e9c523ab 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -759,6 +759,9 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result)
}
}
+ /* Recompute the dirty count */
+ a->count = hb_count_between(a, 0, a->size - 1);
+
return true;
}
--
2.14.4
27.09.2018 00:28, John Snow wrote: > We have been neglecting to do so, which results in wrong counts > after merge. In the worst case, we may think the bitmap is empty > when it has had new writes merged into it. > > Reported-by: Eric Blake <eblake@redhat.com> > Signed-off-by: John Snow <jsnow@redhat.com> > --- > util/hbitmap.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/util/hbitmap.c b/util/hbitmap.c > index d5aca5159f..28e9c523ab 100644 > --- a/util/hbitmap.c > +++ b/util/hbitmap.c > @@ -759,6 +759,9 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result) > } > } > > + /* Recompute the dirty count */ > + a->count = hb_count_between(a, 0, a->size - 1); hmm, just looking at function header: shouldn't we update result->count instead? This code shouldn't compile (thanks to my const*) > + > return true; > } > -- Best regards, Vladimir
On 9/27/18 2:31 AM, Vladimir Sementsov-Ogievskiy wrote: > 27.09.2018 00:28, John Snow wrote: >> We have been neglecting to do so, which results in wrong counts >> after merge. In the worst case, we may think the bitmap is empty >> when it has had new writes merged into it. >> >> Reported-by: Eric Blake <eblake@redhat.com> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> util/hbitmap.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/util/hbitmap.c b/util/hbitmap.c >> index d5aca5159f..28e9c523ab 100644 >> --- a/util/hbitmap.c >> +++ b/util/hbitmap.c >> @@ -759,6 +759,9 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap >> *b, HBitmap *result) >> } >> } >> + /* Recompute the dirty count */ >> + a->count = hb_count_between(a, 0, a->size - 1); > > hmm, just looking at function header: shouldn't we update result->count > instead? This code shouldn't compile (thanks to my const*) Hmm. It looks like John and I posted essentially the same patch, but that John's version is based against his out-of-tree patch queue, which includes Vladimir's "dirty-bitmap: make it possible to restore bitmap after merge". -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 09/27/2018 10:09 AM, Eric Blake wrote: > On 9/27/18 2:31 AM, Vladimir Sementsov-Ogievskiy wrote: >> 27.09.2018 00:28, John Snow wrote: >>> We have been neglecting to do so, which results in wrong counts >>> after merge. In the worst case, we may think the bitmap is empty >>> when it has had new writes merged into it. >>> >>> Reported-by: Eric Blake <eblake@redhat.com> >>> Signed-off-by: John Snow <jsnow@redhat.com> >>> --- >>> util/hbitmap.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/util/hbitmap.c b/util/hbitmap.c >>> index d5aca5159f..28e9c523ab 100644 >>> --- a/util/hbitmap.c >>> +++ b/util/hbitmap.c >>> @@ -759,6 +759,9 @@ bool hbitmap_merge(const HBitmap *a, const >>> HBitmap *b, HBitmap *result) >>> } >>> } >>> + /* Recompute the dirty count */ >>> + a->count = hb_count_between(a, 0, a->size - 1); >> >> hmm, just looking at function header: shouldn't we update >> result->count instead? This code shouldn't compile (thanks to my const*) > > Hmm. It looks like John and I posted essentially the same patch, but > that John's version is based against his out-of-tree patch queue, which > includes Vladimir's "dirty-bitmap: make it possible to restore bitmap > after merge". > Oh, I see, we each posted one. Whoops! Yes, mine is based off of the transactionable merge series. --js
© 2016 - 2024 Red Hat, Inc.