[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps from bdrv_move_feat
[Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps from bdrv_move_feature_fields()
Mon, 14 Mar 2016 16:44:49 +0100
This patch changes dirty bitmaps from following a BlockBackend in graph
changes to sticking with the node they were created at. For the full
discussion, read the following mailing list thread:
[Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()
In summary, the justification for this change is:
* When moving the dirty bitmap to the top of the tree was introduced in
bdrv_append() in commit a9fc4408, it didn't actually have any effect
because there could never be a bitmap in use when bdrv_append() was
called (op blockers would prevent this). This is still true today for
all internal uses of dirty bitmaps.
* Support for user-defined dirty bitmaps was introduced in 2.4, but we
discouraged users from using it because we didn't consider it ready
Moreover, in 2.5, the bdrv_swap() removal introduced a bug that left
dangling pointers if a dirty bitmap was present (the anchors of the
dirty bitmap were swapped, but the back link in the first element
wasn't updated), so it didn't even work correctly.
* block-dirty-bitmap-add takes an arbitrary node name, even if no
BlockBackend is attached. This suggests that it is a node level
operation and not a BlockBackend one. Consequently, there is no reason
for dirty bitmaps to stay with a BlockBackend that was attached to the
node they were created for.
* It was suggested that block-dirty-bitmap-add could track the node if a
node name was specified, and track the BlockBackend if the device name
was specified. This would however be inconsistent with other QMP
commands. Commands that accept both device and node names currently
interpret the device name just as an alias for the current root node
of that BlockBackend.
* Dirty bitmaps have a name that is only unique amongst the bitmaps in a
specific node. Moving bitmaps could lead to name clashes. Automatic
renaming would involve too much magic.
* Persistent bitmaps are stored in a specific node. Moving them around
automatically might be at least surprising, but it would probably also
become a real problem because that would have to happen atomically
without the management tool knowing of the operation.
At the end of the day it seems to be very clear that it was a mistake to
include dirty bitmaps in bdrv_move_feature_fields(). The functionality
of moving bitmaps and/or attaching them to a BlockBackend instead will
probably be needed, but it should be done with a new explicit QMP
command or option.
Signed-off-by: Kevin Wolf <address@hidden>
block.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/block.c b/block.c
index a75c4b3..698e2c7 100644
@@ -2284,9 +2284,6 @@ static void bdrv_move_feature_fields(BlockDriverState
/* dev info */
bs_dest->enable_write_cache = bs_src->enable_write_cache;
- /* dirty bitmap */
- bs_dest->dirty_bitmaps = bs_src->dirty_bitmaps;
static void change_parent_backing_link(BlockDriverState *from,