20.01.2020 19:30, Vladimir Sementsov-Ogievskiy wrote: > 20.01.2020 16:14, Max Reitz wrote: >> On 19.12.19 11:03, Vladimir Sementsov-Ogievskiy wrote: >>> We have bdrv_dirty_bitmap_next_zero, let's add corresponding >>> bdrv_dirty_bitmap_next_dirty, which is more comfortable to use than >>> bitmap iterators in some cases. >>> >>> For test modify test_hbitmap_next_zero_check_range to check both >>> next_zero and next_dirty and add some new checks. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >>> --- >>> include/block/dirty-bitmap.h | 2 + >>> include/qemu/hbitmap.h | 13 ++++ >>> block/dirty-bitmap.c | 6 ++ >>> tests/test-hbitmap.c | 130 ++++++++++++++++++++--------------- >>> util/hbitmap.c | 60 ++++++++-------- >>> 5 files changed, 126 insertions(+), 85 deletions(-) >> >> [...] >> >>> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h >>> index b6e85f3d5d..a4b032b270 100644 >>> --- a/include/qemu/hbitmap.h >>> +++ b/include/qemu/hbitmap.h >>> @@ -297,6 +297,19 @@ void hbitmap_free(HBitmap *hb); >>> */ >>> void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t >>> first); >>> +/* >>> + * hbitmap_next_dirty: >>> + * >>> + * Find next dirty bit within selected range. If not found, return -1. >>> + * >>> + * @hb: The HBitmap to operate on >>> + * @start: The bit to start from. >>> + * @count: Number of bits to proceed. If @start+@count > bitmap size, the >>> whole >>> + * bitmap is looked through. You can use UINT64_MAX as @count to search up >>> to >> >> I would’ve said s/looked through/scanned/, but it matches >> hbitmap_next_zero()’s documentation, so it’s fine. >> >> But definitely s/UINT64_MAX/INT64_MAX/. >> >>> + * the bitmap end. >>> + */ >>> +int64_t hbitmap_next_dirty(const HBitmap *hb, int64_t start, int64_t >>> count); >>> + >>> /* hbitmap_next_zero: >>> * >>> * Find next not dirty bit within selected range. If not found, return -1. >> >> [...] >> >>> diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c >>> index 0e1e5c64dd..e3f1b3f361 100644 >>> --- a/tests/test-hbitmap.c >>> +++ b/tests/test-hbitmap.c >>> @@ -816,92 +816,108 @@ static void >>> test_hbitmap_iter_and_reset(TestHBitmapData *data, >>> hbitmap_iter_next(&hbi); >>> } >>> -static void test_hbitmap_next_zero_check_range(TestHBitmapData *data, >>> - uint64_t start, >>> - uint64_t count) >>> +static void test_hbitmap_next_x_check_range(TestHBitmapData *data, >>> + uint64_t start, >>> + uint64_t count) >> >> Why not change the parameters to be signed while we’re already here?
Now I think that better to do it in previous patch. >> >> [...] >> >>> diff --git a/util/hbitmap.c b/util/hbitmap.c >>> index df22f06be6..d23f4b9678 100644 >>> --- a/util/hbitmap.c >>> +++ b/util/hbitmap.c >>> @@ -193,6 +193,30 @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap >>> *hb, uint64_t first) >>> } >>> } >>> +int64_t hbitmap_next_dirty(const HBitmap *hb, int64_t start, int64_t count) >>> +{ >>> + HBitmapIter hbi; >>> + int64_t firt_dirty_off; >> >> Pre-existing, but isn’t this just a typo that you could fix here? (i.e. >> s/firt/first/) >> >> Apart from this minor things: > > Agree with them. > >> >> Reviewed-by: Max Reitz <mre...@redhat.com> >> >>> + uint64_t end; >>> + >>> + assert(start >= 0 && count >= 0); >>> + >>> + if (start >= hb->orig_size || count == 0) { >>> + return -1; >>> + } >>> + >>> + end = count > hb->orig_size - start ? >> > > -- Best regards, Vladimir