On 16.07.26 14:22, Denis V. Lunev wrote:
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 <[email protected]>
CC: Eric Blake <[email protected]>
CC: Vladimir Sementsov-Ogievskiy <[email protected]>
CC: John Snow <[email protected]>
CC: Andrey Drobyshev <[email protected]>
Could it be a scenario, when user intentionally create bitmap in
read-only image, and expect, that it would become writeable soon?
For example before block-commit.
I don't think that useful.
For usual backup scenario, user will just move bitmaps from top to
base, no need to make a bitmap, dirtied by the block-commit operation
itself.
If, for some reason we don't have bitmap in top image, but have in
base (top is a kind of temporary snapshot), bitmaps in base would
become writeable and accept new dirty bits as expected.
Creating a new bitmap, to become actually a block-status bitmap of
merged top base seems a strange case.. A kind of combination of
snapshot-based and backup-based backup processes?
Anyway, I think, it's quite correct immediately store an empty
bitmap to the qcow2 image when creating it with persistent=true,
which of course fails if image is read-only. So, I think the change
is correct:
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
--
Best regards,
Vladimir
