Am 01.03.2017 um 09:15 hat Fam Zheng geschrieben: > In an ideal world, read-write access to an image is inherently > exclusive, because we cannot guarantee other readers and writers a > consistency view of the whole image at all point. That's what the > current permission system does, and it is okay as long as it is entirely > internal. But that would change with the coming image locking. In > practice, both end users and our test cases use tools like qemu-img and > qemu-io to peek at images while guest is running. > > Relax a bit and accept other writers in this case. > > Signed-off-by: Fam Zheng <f...@redhat.com>
Hm. On the one hand I think you're right that people are using things like this, on other hand it's also true that the result might not be consistent and therefore image locking is right about forbidding these actions. I think your patch is too permissive, it allows even launching a second long-running VM on the image, which will definitely see corrupted data sooner or later. Maybe what we can do is allow shared writers for read-only images if CONSISTENT_READ isn't requested. It's still not 100% correct because we can get inconsistent metadata and cause unexpected failure, but this is probably tolerable. We would then have to change the allowed actions to not request this permission. In qemu-img, we have these read-only users: * qemu-img check (without -r): Let's keep this blocked, it will only report lots of leaks and leads to invalid bug reports. I've had my share of them. * qemu-img compare: Not sure if this makes sense with an image that is in active use? * qemu-img convert source: Similarly to qemu-img compare, this doesn't make a whole lot of sense, with one exception: People are using -s to extract internal snapshots while the VM is still running, and this usually works because the snapshot doesn't change. However, I don't want to know what happens if you delete the snapshort from the qemu process while qemu-img convert is running... Doing 'qemu-img snapshot -s ...' with a running VM is more tolerated abuse than a supported feature. * qemu-img info: This one makes perfect sense even with a running VM * qemu-img map: Results are potentially meaningless with concurrent I/O, but there may be cases where it makes sense. * qemu-img snapshot -l: Somewhat similar to qemu-img convert -s, except that it's very quick and doesn't access actual data (could even be BDRV_O_NOIO, I think). Allowing this makes sense, I guess. * qemu-img rebase, safe mode, backing files: If we allowed concurrent writes, it wouldn't be safe. * qemu-img bench: Well... No. You don't need this on an image with an active VM. * qemu-img dd: Same as convert, except that there is no -s. So the list of subcommands where we want to support it is rather short. We can change blk_new_open() to clear CONSISTENT_READ for BDRV_O_NOIO, which could cover 'info' and 'snapshot -l'. That leaves us with qemu-io, 'convert -s' and 'map', all of which can be imagined to be useful even with a running VM, but all of which can also easily produce wrong results in this case. An explicit command line option to disable CONSISTENT_READ might be the right tool here. What do you think? Kevin
