|
From: | Hanna Reitz |
Subject: | Re: [PATCH v3 07/19] block/dirty-bitmap: introduce bdrv_dirty_bitmap_status() |
Date: | Tue, 18 Jan 2022 14:31:54 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 |
On 22.12.21 18:40, Vladimir Sementsov-Ogievskiy wrote:
Add a convenient function similar with bdrv_block_status() to get status of dirty bitmap. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- include/block/dirty-bitmap.h | 2 ++ include/qemu/hbitmap.h | 11 +++++++++++ block/dirty-bitmap.c | 6 ++++++ util/hbitmap.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+)
[...]
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h index 5e71b6d6f7..845fda12db 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -340,6 +340,17 @@ bool hbitmap_next_dirty_area(const HBitmap *hb, int64_t start, int64_t end, int64_t max_dirty_count, int64_t *dirty_start, int64_t *dirty_count);+/*+ * bdrv_dirty_bitmap_status: + * @hb: The HBitmap to operate on + * @start: the offset to start from + * @end: end of requested area + * @is_dirty: is bitmap dirty at @offset + * @pnum: how many bits has same value starting from @offset + */ +void hbitmap_status(const HBitmap *hb, int64_t offset, int64_t bytes,
In addition to the comment not fitting the parameter names, I also don’t find it ideal that the parameter names here don’t match the ones in the function’s definition.
I don’t have a preference between `start` or `offset` (although most other bitmap functions seem to prefer `start`), but I do prefer `count` over `bytes`, because... Well, it’s a bit count, not a byte count, right? (And from the bitmap user’s perspective, those bits might stand for any arbitrary unit.)
Apart from that, looks nice to me. I am wondering a bit why this function doesn’t simply return the dirty bit status (like, well, the block-status functions do it), but I presume you simply found this interface to be better suited for its callers.
+ bool *is_dirty, int64_t *pnum); + /** * hbitmap_iter_next: * @hbi: HBitmapIter to operate on.
[Prev in Thread] | Current Thread | [Next in Thread] |