[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/6] qapi: add block-dirty-bitmap-merge
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/6] qapi: add block-dirty-bitmap-merge |
Date: |
Thu, 20 Sep 2018 14:40:18 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
[reviving an old patch]
On 1/16/18 6:54 AM, Vladimir Sementsov-Ogievskiy wrote:
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
qapi/block-core.json | 38 ++++++++++++++++++++++++++++++++++++++
include/block/dirty-bitmap.h | 2 ++
block/dirty-bitmap.c | 18 ++++++++++++++++++
blockdev.c | 30 ++++++++++++++++++++++++++++++
4 files changed, 88 insertions(+)
We've already hit a couple issues with this patch mentioned in other
threads:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b3851ee760..9f9cfa0a44 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1604,6 +1604,20 @@
'*persistent': 'bool', '*autoload': 'bool' } }
##
+# @BlockDirtyBitmapMerge:
+#
+# @node: name of device/node which the bitmap is tracking
+#
+# @dst_name: name of the destination dirty bitmap
+#
+# @src_name: name of the source dirty bitmap
Naming choice (prefer - over _):
https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg02352.html
+++ b/block/dirty-bitmap.c
@@ -699,3 +699,21 @@ char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap
*bitmap, Error **errp)
{
return hbitmap_sha256(bitmap->bitmap, errp);
}
+
+void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
+ Error **errp)
+{
+ /* only bitmaps from one bds are supported */
+ assert(dest->mutex == src->mutex);
+
+ qemu_mutex_lock(dest->mutex);
+
+ assert(bdrv_dirty_bitmap_enabled(dest));
Merging to a disabled bitmap is desirable:
https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg02358.html
+ assert(!bdrv_dirty_bitmap_readonly(dest));
+
+ if (!hbitmap_merge(dest->bitmap, src->bitmap)) {
And here's another one I ran into. hbitmap_merge() does NOT properly
update hbitmap_count(), which means statistics about the number of bits
set in the bitmap, as visible through QMP "query-block", are inaccurate
after a merge.
Check out hbitmap_set() and hbitmap_reset(), which carefully recompute
hb->count using hb_count_between() as part of flipping bits. But since
hbitmap_merge() fails to recompute bits, various other aspects of the
code go haywire (for example, code that uses hbitmap_empty() or
hbitmap_count()==0 to short-circuit operations [and hbitmap_merge() is
one such caller] makes the wrong choice on a bitmap that started life
empty, and had a non-empty bitmap merged in, because hb->count is still 0).
I think that x-debug-block-dirty-bitmap-sha256 should be enhanced to
output count as well as the checksum of the bitmap contents, and then
that we need iotests coverage of x-block-dirty-bitmap-merge, complete
with a check that the count is properly updated. iotests 165, 169, 176,
199 all perform bitmap checksum validations, but so far none of them
perform a bitmap merge. Be careful in writing any such test that we
don't re-introduce any sensitivities on 32- vs. 64-bit or endianness in
our checksum computation (see commit 2807746f for comparison).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 5/6] qapi: add block-dirty-bitmap-merge,
Eric Blake <=