On 2017-05-03 14:25, Vladimir Sementsov-Ogievskiy wrote: > It will be needed in following commits for persistent bitmaps. > If bitmap is loaded from read-only storage (and we can't mark it > "in use" in this storage) corresponding BdrvDirtyBitmap should be > read-only. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > block/dirty-bitmap.c | 16 ++++++++++++++++ > include/block/dirty-bitmap.h | 3 +++ > 2 files changed, 19 insertions(+)
Revisiting this again after the whole series: So you never really make sure that the read-only bitmaps are not written to (except for these assertions). The idea is that you only set it for read-only BDS and read-only BDS are never written to. But that assumption is not true, generally, and can be broken e.g. using a commit job: $ ./qemu-img create -f qcow2 backing.qcow2 64M Formatting 'backing.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ ./qemu-img create -f qcow2 -b backing.qcow2 top.qcow2 Formatting 'top.qcow2', fmt=qcow2 size=67108864 backing_file=backing.qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ x86_64-softmmu/qemu-system-x86_64 -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 9, "major": 2}, "package": " (v2.9.0-632-g4a52d43-dirty)"}, "capabilities": []}} {'execute': 'qmp_capabilities'} {"return": {}} {'execute': 'blockdev-add', 'arguments': {'node-name': 'backing-node', 'driver': 'qcow2', 'file': {'driver': 'file', 'filename': 'backing.qcow2'}}} {"return": {}} {'execute': 'block-dirty-bitmap-add', 'arguments': {'node': 'backing-node', 'name': 'foo', 'persistent': true, 'autoload': true}} {"return": {}} {'execute': 'blockdev-del', 'arguments': {'node-name': 'backing-node'}} {"return": {}} {'execute': 'blockdev-add', 'arguments': {'node-name': 'top-node', 'driver': 'qcow2', 'file': {'driver': 'file', 'filename': 'top.qcow2'}}} {"return": {}} {'execute': 'human-monitor-command', 'arguments': {'command-line': 'qemu-io top-node "write 0 64k"'}} wrote 65536/65536 bytes at offset 0 64 KiB, 1 ops; 0.0079 sec (7.852 MiB/sec and 125.6281 ops/sec) {"return": ""} {'execute': 'block-commit', 'arguments': {'device': 'top-node', 'job-id': 'commit-job'}} {"return": {}} qemu-system-x86_64: block/dirty-bitmap.c:571: bdrv_set_dirty: Assertion `!bdrv_dirty_bitmap_readonly(bitmap)' failed. [1] 10872 abort (core dumped) x86_64-softmmu/qemu-system-x86_64 -qmp stdio So there needs to be something else than just assertions to make sure that read-only bitmaps are never written to. Max
signature.asc
Description: OpenPGP digital signature