11.11.2017 01:52, John Snow wrote:
On 10/30/2017 12:32 PM, Vladimir Sementsov-Ogievskiy wrote:
It is needed to realize bdrv_dirty_bitmap_release_successor in
the following patch.
OK, but...
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
block/dirty-bitmap.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 81adbeb6d4..981f99d362 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -326,13 +326,13 @@ static bool bdrv_dirty_bitmap_has_name(BdrvDirtyBitmap
*bitmap)
return !!bdrv_dirty_bitmap_name(bitmap);
}
-/* Called with BQL taken. */
-static void bdrv_do_release_matching_dirty_bitmap(
+/* Called within bdrv_dirty_bitmap_lock..unlock */
...Add this so it will compile:
how do you compile to get an error? and what is unused?
__attribute__((__unused__))
+static void bdrv_do_release_matching_dirty_bitmap_locked(
BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
bool (*cond)(BdrvDirtyBitmap *bitmap))
{
BdrvDirtyBitmap *bm, *next;
- bdrv_dirty_bitmaps_lock(bs);
+
QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
if ((!bitmap || bm == bitmap) && (!cond || cond(bm))) {
assert(!bm->active_iterators);
@@ -344,18 +344,33 @@ static void bdrv_do_release_matching_dirty_bitmap(
g_free(bm);
if (bitmap) {
- goto out;
+ return;
}
}
}
+
if (bitmap) {
abort();
}
Do we have any style guide rules on using abort() instead of assert()?
The rest of this function uses assert, and it'd be less lines to simply
write:
assert(!bitmap);
which I think might also carry better semantic information for coverity
beyond an actual runtime conditional branch.
(I think. Please correct me if I am wrong, I'm a little hazy on this.)
+}
-out:
+/* Called with BQL taken. */
+static void bdrv_do_release_matching_dirty_bitmap(
+ BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ bool (*cond)(BdrvDirtyBitmap *bitmap))
+{
+ bdrv_dirty_bitmaps_lock(bs);
+ bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, cond);
bdrv_dirty_bitmaps_unlock(bs);
}
+/* Called within bdrv_dirty_bitmap_lock..unlock */
+static void bdrv_release_dirty_bitmap_locked(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap)
+{
+ bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, NULL);
+}
+
/* Called with BQL taken. */
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
If you agree with those two changes, you may add:
Reviewed-by: John Snow <js...@redhat.com>
--
Best regards,
Vladimir