I'm surprised it didn't come up sooner, but sometimes we have a +busy
bitmap as a source. This is dangerous from the QMP API, but if we are
the owner that marked the bitmap busy, it's safe to merge it using it as
a read only source.
It is not safe in the general case to allow users to read from in-use
bitmaps, so create an internal variant that foregoes the safety
checking.
Signed-off-by: John Snow <js...@redhat.com>
Reviewed-by: Max Reitz <mre...@redhat.com>
Message-id: 20190709232550.10724-10-js...@redhat.com
Signed-off-by: John Snow <js...@redhat.com>
---
block/dirty-bitmap.c | 54 +++++++++++++++++++++++++++++++++++----
include/block/block_int.h | 3 +++
2 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 95a9c2a5d8a..7881fea684b 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -810,6 +810,12 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap
*bitmap,
return hbitmap_next_dirty_area(bitmap->bitmap, offset, bytes);
}
+/**
+ * bdrv_merge_dirty_bitmap: merge src into dest.
+ * Ensures permissions on bitmaps are reasonable; use for public API.
+ *
+ * @backup: If provided, make a copy of dest here prior to merge.
+ */
void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
HBitmap **backup, Error **errp)
{
@@ -833,6 +839,42 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const
BdrvDirtyBitmap *src,
goto out;
}
+ ret = bdrv_dirty_bitmap_merge_internal(dest, src, backup, false);
+ assert(ret);
+
+out:
+ qemu_mutex_unlock(dest->mutex);
+ if (src->mutex != dest->mutex) {
+ qemu_mutex_unlock(src->mutex);
+ }
+}
+
+/**
+ * bdrv_dirty_bitmap_merge_internal: merge src into dest.
+ * Does NOT check bitmap permissions; not suitable for use as public API.
+ *
+ * @backup: If provided, make a copy of dest here prior to merge.
+ * @lock: If true, lock and unlock bitmaps on the way in/out.
+ * returns true if the merge succeeded; false if unattempted.
+ */
+bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
+ const BdrvDirtyBitmap *src,
+ HBitmap **backup,
+ bool lock)
+{
+ bool ret;
+
+ assert(!bdrv_dirty_bitmap_readonly(dest));
+ assert(!bdrv_dirty_bitmap_inconsistent(dest));
+ assert(!bdrv_dirty_bitmap_inconsistent(src));
+
+ if (lock) {
+ qemu_mutex_lock(dest->mutex);
+ if (src->mutex != dest->mutex) {
+ qemu_mutex_lock(src->mutex);
+ }
+ }
+
if (backup) {
*backup = dest->bitmap;
dest->bitmap = hbitmap_alloc(dest->size, hbitmap_granularity(*backup));
@@ -840,11 +882,13 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const
BdrvDirtyBitmap *src,
} else {
ret = hbitmap_merge(dest->bitmap, src->bitmap, dest->bitmap);
}
- assert(ret);
-out:
- qemu_mutex_unlock(dest->mutex);
- if (src->mutex != dest->mutex) {
- qemu_mutex_unlock(src->mutex);
+ if (lock) {
+ qemu_mutex_unlock(dest->mutex);
+ if (src->mutex != dest->mutex) {
+ qemu_mutex_unlock(src->mutex);
+ }
}
+
+ return ret;
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 80953ac8aeb..aa697f1f694 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1253,6 +1253,9 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t offset,
int64_t bytes);
void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup);
+bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
+ const BdrvDirtyBitmap *src,
+ HBitmap **backup, bool lock);
void bdrv_inc_in_flight(BlockDriverState *bs);
void bdrv_dec_in_flight(BlockDriverState *bs);
--
2.21.0
