qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 07/19] block/dirty-bitmap: introduce bdrv_dirty_bitmap_sta


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v3 07/19] block/dirty-bitmap: introduce bdrv_dirty_bitmap_status()
Date: Wed, 26 Jan 2022 13:56:05 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]