On 9/26/19 1:54 PM, John Snow wrote:
On 9/16/19 10:19 AM, Vladimir Sementsov-Ogievskiy wrote:
bdrv_dirty_bitmap_next is always used in same pattern. So, split it
into _next and _first, instead of combining two functions into one and
add FOR_EACH_DIRTY_BITMAP macro.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
include/block/dirty-bitmap.h | 9 +++++++--
block.c | 4 +---
block/dirty-bitmap.c | 11 +++++++----
block/qcow2-bitmap.c | 8 ++------
migration/block-dirty-bitmap.c | 4 +---
5 files changed, 18 insertions(+), 18 deletions(-)
I'm not as sure that this is an improvement.
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
-BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap);
+
+BdrvDirtyBitmap *bdrv_dirty_bitmap_first(BlockDriverState *bs);
+BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BdrvDirtyBitmap *bitmap);
+#define FOR_EACH_DIRTY_BITMAP(bs, bitmap) \
+for (bitmap = bdrv_dirty_bitmap_first(bs); bitmap; \
+ bitmap = bdrv_dirty_bitmap_next(bitmap))
+
If you want the macro, you can do that without splitting the function in
two:
#define FOR_EACH_DIRTY_BITMAP(bs, bitmap) \
for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap; \
bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
Well, I guess explicit first and next functions is harder to mess up,
anyway.
Reviewed-by: John Snow <js...@redhat.com>
(Any other thoughts?)
I don't mind the macro as much (since we already use iteration macros
for QTAILQ_FOREACH and such throughout the codebase, and since it
somewhat isolates you from the calling conventions of passing NULL to
prime the iteration), but introducing the macro does not imply that we
need two functions. So I think this is, to some extent, a question of
the maintainer's sense of aesthetics, and not an actual problem in the code.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org