28.12.2017 08:24, Fam Zheng wrote:
On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
include/block/dirty-bitmap.h | 3 +++
block/dirty-bitmap.c | 25 ++++++++++++++++---------
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 93d4336505..caf1f3d861 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -92,5 +92,8 @@ bool bdrv_has_changed_persistent_bitmaps(BlockDriverState
*bs);
BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap);
char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp);
#endif
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index d83da077d5..fe27ddfb83 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -327,15 +327,11 @@ BdrvDirtyBitmap
*bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
* The merged parent will be un-frozen, but not explicitly re-enabled.
* Called with BQL taken.
Maybe update the comment as
s/Called with BQL taken/Called within bdrv_dirty_bitmap_lock..unlock/
and ...
we have the following comment:
/* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex.
* Reading from the list can be done with either the BQL or the
* dirty_bitmap_mutex. Modifying a bitmap only requires
* dirty_bitmap_mutex. */
QemuMutex dirty_bitmap_mutex;
(I don't understand well the logic, why is it so. Paolo introduced the
lock, but didn't update some functions..)
so, actually, here we need both BQL and mutex.
*/
-BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
- BdrvDirtyBitmap *parent,
- Error **errp)
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent,
+ Error **errp)
{
- BdrvDirtyBitmap *successor;
-
- qemu_mutex_lock(parent->mutex);
-
- successor = parent->successor;
+ BdrvDirtyBitmap *successor = parent->successor;
if (!successor) {
error_setg(errp, "Cannot reclaim a successor when none is present");
@@ -349,9 +345,20 @@ BdrvDirtyBitmap
*bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
bdrv_release_dirty_bitmap_locked(bs, successor);
parent->successor = NULL;
+ return parent;
+}
+
... move the "Called with BQL taken" comment here?
and here BQL.
Ok, I'll add/fix comments.
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent,
+ Error **errp)
+{
+ BdrvDirtyBitmap *ret;
+
+ qemu_mutex_lock(parent->mutex);
+ ret = bdrv_reclaim_dirty_bitmap_locked(bs, parent, errp);
qemu_mutex_unlock(parent->mutex);
- return parent;
+ return ret;
}
/**
--
2.11.1
--
Best regards,
Vladimir
