On Wed, 04/13 13:18, Denis V. Lunev wrote: > On 04/13/2016 12:09 PM, Fam Zheng wrote: > >Too many troubles have been caused by two processes writing to the same image > >unexpectedly. This series introduces automatical image locking into QEMU to > >avoid such tragedy. With this, the user won't be able to open the image from > >two processes (e.g. using qemu-img when the image is attached to the guest). > > > >Underneath is the fcntl syscall that locks the local file, similar to what is > >already used in libvirt virtlockd. Also because of that, we cannot directly > >apply fcntl lock on the image file itself, instead we open and lock > >"/var/tmp/.qemu-$sha1.lock", where $sha1 is derived from the image's full > >path > >as in realpath(3). This mechanism should be equally useful for the single > >host > >case, and it doesn't conflict with virtlockd when managed by libvirt. > > > >The alternative file locking API on Linux, flock(2), cannot protect host NFS > >mount points, so it's not used. > > > >Gluster locking is also implemented wrapping glfs_posix_lock in patch 6. It's > >only lightly tested. > > > >All other drivers that don't implement .bdrv_lockf are always permissive and > >does no checking. > > > >In the future, the intention is that image format drivers that introduce > >locking mechanisms could also fit into this API. > > > >The first 6 patches define the internal and external interfaces, and > >implement > >the image locking. > > > >Patch 7 adds an option in qemu-io to allow disabling the lock, for testing > >purpose. > > > >Patches 8 - 14 fixes the potential failures of test cases where multiple > >processes may open the image concurrently. > > > >Finally the default behavior is switched to on in patch 15. > > > >Fam Zheng (15): > > block: Add BDRV_O_NO_LOCK > > qapi: Add lock-image in blockdev-add options > > blockdev: Add and parse "lock-image" option for block devices > > block: Introduce image file locking interface > > raw-posix: Implement .bdrv_lockf > > gluster: Implement .bdrv_lockf > > qemu-io: Add "-L" option for BDRV_O_NO_LOCK > > qemu-iotests: 140: Disable image lock for qemu-io access > > qemu-iotests: 046: Move version detection out from verify_io > > qemu-iotests: Fix lock-image for shared disk in test case 091 > > qemu-iotests: Disable image lock when checking test image > > qemu-iotests: 051: Disable image lock in the command line > > ahci-test: Specify "lock-image=off" in CLI > > ide-test: Specify "lock-image=off" in command lines > > block: Turn on image locking by default > > > > block.c | 25 +++++++++++ > > block/gluster.c | 34 +++++++++++++++ > > block/raw-posix.c | 97 > > +++++++++++++++++++++++++++++++++++++++++++ > > blockdev.c | 8 ++++ > > include/block/block.h | 9 ++++ > > include/block/block_int.h | 5 +++ > > qapi/block-core.json | 6 ++- > > qemu-io.c | 22 +++++++++- > > tests/ahci-test.c | 16 +++++-- > > tests/ide-test.c | 5 ++- > > tests/qemu-iotests/030 | 2 +- > > tests/qemu-iotests/046 | 22 +++++----- > > tests/qemu-iotests/051 | 2 +- > > tests/qemu-iotests/051.out | 10 ++--- > > tests/qemu-iotests/051.pc.out | 10 ++--- > > tests/qemu-iotests/091 | 4 +- > > tests/qemu-iotests/140 | 2 +- > > 17 files changed, 245 insertions(+), 34 deletions(-) > > > First of all, I like the approach as we have discussed :) Then, > in general, I support Daniel with the point that > the locking should be done on the image file > directly. > > Also, it looks like this will break migration with the shared storage. > For me it seems that we will have lock the image from both ends
There should already be state handling logic between source and destinition ends, I'll take a look and see if we can add lock/unlock calls there to keep it working. Fam
