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/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index f95d350b70..2ae7dc3d1d 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -115,6 +115,8 @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, int64_t offset,
bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
int64_t start, int64_t end, int64_t max_dirty_count,
int64_t *dirty_start, int64_t *dirty_count);
+void bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset,
+ int64_t bytes, bool *is_dirty, int64_t *count);
BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
Error **errp);
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,
+ bool *is_dirty, int64_t *pnum);
+
/**
* hbitmap_iter_next:
* @hbi: HBitmapIter to operate on.
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 94a0276833..e4a836749a 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -875,6 +875,12 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
dirty_start, dirty_count);
}
+void bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset,
+ int64_t bytes, bool *is_dirty, int64_t *count)
+{
+ hbitmap_status(bitmap->bitmap, offset, bytes, is_dirty, count);
+}
+
/**
* bdrv_merge_dirty_bitmap: merge src into dest.
* Ensures permissions on bitmaps are reasonable; use for public API.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 305b894a63..ae8d0eb4d2 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -301,6 +301,42 @@ bool hbitmap_next_dirty_area(const HBitmap *hb, int64_t start, int64_t end,
return true;
}
+void hbitmap_status(const HBitmap *hb, int64_t start, int64_t count,
+ bool *is_dirty, int64_t *pnum)
+{
+ int64_t next_dirty, next_zero;
+
+ assert(start >= 0);
+ assert(count > 0);
+ assert(start + count <= hb->orig_size);
+
+ next_dirty = hbitmap_next_dirty(hb, start, count);
+ if (next_dirty == -1) {
+ *pnum = count;
+ *is_dirty = false;
+ return;
+ }
+
+ if (next_dirty > start) {
+ *pnum = next_dirty - start;
+ *is_dirty = false;
+ return;
+ }
+
+ assert(next_dirty == start);
+
+ next_zero = hbitmap_next_zero(hb, start, count);
+ if (next_zero == -1) {
+ *pnum = count;
+ *is_dirty = true;
+ return;
+ }
+
+ assert(next_zero > start);
+ *pnum = next_zero - start;
+ *is_dirty = false;
+}
+
bool hbitmap_empty(const HBitmap *hb)
{
return hb->count == 0;
--
2.31.1
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.
18.01.2022 16:31, Hanna Reitz wrote: >> +/* >> + * 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. Hmm, seems, no reason for it actually. Will change to use normal return value. -- Best regards, Vladimir
On 12/22/21 20: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/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index f95d350b70..2ae7dc3d1d 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -115,6 +115,8 @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, int64_t offset,
> bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
> int64_t start, int64_t end, int64_t max_dirty_count,
> int64_t *dirty_start, int64_t *dirty_count);
> +void bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset,
> + int64_t bytes, bool *is_dirty, int64_t *count);
> BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
> Error **errp);
>
> 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,
> + bool *is_dirty, int64_t *pnum);
> +
I think description should be changed, there is no start and no end
arguments in function.
> /**
> * hbitmap_iter_next:
> * @hbi: HBitmapIter to operate on.
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 94a0276833..e4a836749a 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -875,6 +875,12 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
> dirty_start, dirty_count);
> }
>
> +void bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset,
> + int64_t bytes, bool *is_dirty, int64_t *count)
> +{
> + hbitmap_status(bitmap->bitmap, offset, bytes, is_dirty, count);
> +}
> +
> /**
> * bdrv_merge_dirty_bitmap: merge src into dest.
> * Ensures permissions on bitmaps are reasonable; use for public API.
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 305b894a63..ae8d0eb4d2 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -301,6 +301,42 @@ bool hbitmap_next_dirty_area(const HBitmap *hb, int64_t start, int64_t end,
> return true;
> }
>
> +void hbitmap_status(const HBitmap *hb, int64_t start, int64_t count,
> + bool *is_dirty, int64_t *pnum)
> +{
> + int64_t next_dirty, next_zero;
> +
> + assert(start >= 0);
> + assert(count > 0);
> + assert(start + count <= hb->orig_size);
> +
> + next_dirty = hbitmap_next_dirty(hb, start, count);
> + if (next_dirty == -1) {
> + *pnum = count;
> + *is_dirty = false;
> + return;
> + }
> +
> + if (next_dirty > start) {
> + *pnum = next_dirty - start;
> + *is_dirty = false;
> + return;
> + }
> +
> + assert(next_dirty == start);
> +
> + next_zero = hbitmap_next_zero(hb, start, count);
> + if (next_zero == -1) {
> + *pnum = count;
> + *is_dirty = true;
> + return;
> + }
> +
> + assert(next_zero > start);
> + *pnum = next_zero - start;
> + *is_dirty = false;
> +}
> +
This function finds if this bitmap is dirty and also counts first bits.
I don't think that this is a problem, but may be it should be divided?
> bool hbitmap_empty(const HBitmap *hb)
> {
> return hb->count == 0;
With corrected description
Reviewed-by: Nikita Lapshin<nikita.lapshin@virtuozzo.com>
17.01.2022 13:06, Nikta Lapshin wrote:
> On 12/22/21 20: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/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index f95d350b70..2ae7dc3d1d 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -115,6 +115,8 @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, int64_t offset,
>> bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
>> int64_t start, int64_t end, int64_t max_dirty_count,
>> int64_t *dirty_start, int64_t *dirty_count);
>> +void bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset,
>> + int64_t bytes, bool *is_dirty, int64_t *count);
>> BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>> Error **errp);
>>
>> 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,
>> + bool *is_dirty, int64_t *pnum);
>> +
>
> I think description should be changed, there is no start and no end
> arguments in function.
>
>> /**
>> * hbitmap_iter_next:
>> * @hbi: HBitmapIter to operate on.
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 94a0276833..e4a836749a 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -875,6 +875,12 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
>> dirty_start, dirty_count);
>> }
>>
>> +void bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset,
>> + int64_t bytes, bool *is_dirty, int64_t *count)
>> +{
>> + hbitmap_status(bitmap->bitmap, offset, bytes, is_dirty, count);
>> +}
>> +
>> /**
>> * bdrv_merge_dirty_bitmap: merge src into dest.
>> * Ensures permissions on bitmaps are reasonable; use for public API.
>> diff --git a/util/hbitmap.c b/util/hbitmap.c
>> index 305b894a63..ae8d0eb4d2 100644
>> --- a/util/hbitmap.c
>> +++ b/util/hbitmap.c
>> @@ -301,6 +301,42 @@ bool hbitmap_next_dirty_area(const HBitmap *hb, int64_t start, int64_t end,
>> return true;
>> }
>>
>> +void hbitmap_status(const HBitmap *hb, int64_t start, int64_t count,
>> + bool *is_dirty, int64_t *pnum)
>> +{
>> + int64_t next_dirty, next_zero;
>> +
>> + assert(start >= 0);
>> + assert(count > 0);
>> + assert(start + count <= hb->orig_size);
>> +
>> + next_dirty = hbitmap_next_dirty(hb, start, count);
>> + if (next_dirty == -1) {
>> + *pnum = count;
>> + *is_dirty = false;
>> + return;
>> + }
>> +
>> + if (next_dirty > start) {
>> + *pnum = next_dirty - start;
>> + *is_dirty = false;
>> + return;
>> + }
>> +
>> + assert(next_dirty == start);
>> +
>> + next_zero = hbitmap_next_zero(hb, start, count);
>> + if (next_zero == -1) {
>> + *pnum = count;
>> + *is_dirty = true;
>> + return;
>> + }
>> +
>> + assert(next_zero > start);
>> + *pnum = next_zero - start;
>> + *is_dirty = false;
>> +}
>> +
>
> This function finds if this bitmap is dirty and also counts first bits.
Not exactly.
The idea was to have one function, that works like block_status:
it return status of bit at offset and count how many bits are of the same status after it.
> I don't think that this is a problem, but may be it should be divided?
No, I need it as one function, for further commits.
>
>> bool hbitmap_empty(const HBitmap *hb)
>> {
>> return hb->count == 0;
>
> With corrected description
> Reviewed-by: Nikita Lapshin<nikita.lapshin@virtuozzo.com>
>
thanks!
--
Best regards,
Vladimir
© 2016 - 2026 Red Hat, Inc.