@samitouri / QOSamiQemu / commits / 59ecd8a1ac

block/monitor: reject persistent bitmap add on a read-only node

qmp_block_dirty_bitmap_add() marks a new bitmap persistent without checking write access to its node. bdrv_create_dirty_bitmap() always creates bitmaps writable, so a persistent bitmap added to an already read-only node stays writable in memory on a node that can never store it, and the next global inactivation fails: Lost persistent bitmaps during inactivation of node '<node>': No write access migration_block_inactivate: bdrv_inactivate_all() failed: -22 Forcing it read-only instead does not help: it was never stored, so it stays unpromotable on the next reopen to read-write and can trip bdrv_set_dirty()'s readonly assert on the first write. Reject the add instead, for both read-only and inactive nodes -- an already-inactive node skips qcow2_inactivate() on close, so a bitmap added during that window would never get stored either. Wrapped in a transaction, this denies the whole transaction, since qmp_transaction() is already all-or-none. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Eric Blake <eblake@redhat.com> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> CC: John Snow <jsnow@redhat.com> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> Message-ID: <20260716112242.3000035-2-den@openvz.org> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Denis V. Lunev committed Jul 16, 2026 at 13:22 UTC 59ecd8a1ac3f05f572155a2d7a81fbeec397e2d3
4 files changed +62 -6
block/monitor/bitmap-qmp-cmds.c
+11 -4
@@ -125,10 +125,17 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
125 disabled = false;
126 }
127
128 - if (persistent &&
129 - !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
130 - {
131 - return;
128 + if (persistent) {
129 + if (!bdrv_is_writable(bs)) {
130 + error_setg(errp, "Cannot add a persistent bitmap to "
131 + "read-only or inactive node '%s'",
132 + bdrv_get_node_name(bs));
133 + return;
134 + }
135 +
136 + if (!bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp)) {
137 + return;
138 + }
139 }
140
141 bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
qapi/block-core.json
+3 -1
@@ -2353,7 +2353,9 @@
2353 # @persistent: the bitmap is persistent, i.e. it will be saved to the
2354 # corresponding block device image file on its close. For now
2355 # only Qcow2 disks support persistent bitmaps. Default is false
2356 -# for `block-dirty-bitmap-add`. (Since: 2.10)
2356 +# for `block-dirty-bitmap-add`. This fails if the node is
2357 +# read-only or inactive, since such a bitmap could never be
2358 +# stored. (Since: 2.10)
2359 #
2360 # @disabled: the bitmap is created in the disabled state, which means
2361 # that it will not track drive changes. The bitmap may be enabled
tests/qemu-iotests/tests/remove-bitmap-from-backing
+28 -1
@@ -35,7 +35,7 @@ qemu_img('bitmap', '--add', base, 'bitmap0')
35 # Just assert that our method of checking bitmaps in the image works.
36 assert 'bitmaps' in qemu_img_info(base)['format-specific']['data']
37
38 -vm = iotests.VM().add_drive(top, 'backing.node-name=base')
38 +vm = iotests.VM().add_drive(top, 'node-name=top,backing.node-name=base')
39 vm.launch()
40
41 log('Trying to remove persistent bitmap from r-o base node, should fail:')
@@ -66,6 +66,33 @@ result = vm.qmp('blockdev-reopen', **new_base_opts)
66 if result != {'return': {}}:
67 log('Failed to reopen: ' + str(result))
68
69 +log('Adding a persistent bitmap to the r-o base node, should fail:')
70 +vm.qmp_log('block-dirty-bitmap-add', node='base', name='bitmap1',
71 + persistent=True)
72 +
73 +log('Same add inside a transaction, preceded by an otherwise valid')
74 +log('action: the whole transaction must fail and roll back the')
75 +log('already-succeeded first action too:')
76 +vm.qmp_log('transaction', actions=[
77 + {'type': 'block-dirty-bitmap-add',
78 + 'data': {'node': 'top', 'name': 'bitmap2', 'persistent': True}},
79 + {'type': 'block-dirty-bitmap-add',
80 + 'data': {'node': 'base', 'name': 'bitmap1', 'persistent': True}},
81 +])
82 +
83 +log('bitmap2 on the rw top node must not have survived the rollback:')
84 +vm.qmp_log('block-dirty-bitmap-remove', node='top', name='bitmap2')
85 +
86 +log('Marking the rw top node inactive:')
87 +vm.qmp_log('blockdev-set-active', **{'node-name': 'top', 'active': False})
88 +
89 +log('Adding a persistent bitmap to a rw but inactive node, should fail:')
90 +vm.qmp_log('block-dirty-bitmap-add', node='top', name='bitmap3',
91 + persistent=True)
92 +
93 +log('Reactivating the top node:')
94 +vm.qmp_log('blockdev-set-active', **{'node-name': 'top', 'active': True})
95 +
96 vm.shutdown()
97
98 if 'bitmaps' in qemu_img_info(base)['format-specific']['data']:
tests/qemu-iotests/tests/remove-bitmap-from-backing.out
+20
@@ -4,3 +4,23 @@ Trying to remove persistent bitmap from r-o base node, should fail:
4 Remove persistent bitmap from base node reopened to RW:
5 {"execute": "block-dirty-bitmap-remove", "arguments": {"name": "bitmap0", "node": "base"}}
6 {"return": {}}
7 +Adding a persistent bitmap to the r-o base node, should fail:
8 +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap1", "node": "base", "persistent": true}}
9 +{"error": {"class": "GenericError", "desc": "Cannot add a persistent bitmap to read-only or inactive node 'base'"}}
10 +Same add inside a transaction, preceded by an otherwise valid
11 +action: the whole transaction must fail and roll back the
12 +already-succeeded first action too:
13 +{"execute": "transaction", "arguments": {"actions": [{"data": {"name": "bitmap2", "node": "top", "persistent": true}, "type": "block-dirty-bitmap-add"}, {"data": {"name": "bitmap1", "node": "base", "persistent": true}, "type": "block-dirty-bitmap-add"}]}}
14 +{"error": {"class": "GenericError", "desc": "Cannot add a persistent bitmap to read-only or inactive node 'base'"}}
15 +bitmap2 on the rw top node must not have survived the rollback:
16 +{"execute": "block-dirty-bitmap-remove", "arguments": {"name": "bitmap2", "node": "top"}}
17 +{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap2' not found"}}
18 +Marking the rw top node inactive:
19 +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "top"}}
20 +{"return": {}}
21 +Adding a persistent bitmap to a rw but inactive node, should fail:
22 +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap3", "node": "top", "persistent": true}}
23 +{"error": {"class": "GenericError", "desc": "Cannot add a persistent bitmap to read-only or inactive node 'top'"}}
24 +Reactivating the top node:
25 +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "top"}}
26 +{"return": {}}