On 2018-05-09 23:53, Max Reitz wrote: > [Unchanged text from v1 ahead] > > Currently we do not take permissions on a file while it is being > created. That is a bit sad. The simplest way to test this is the > following: > > $ qemu-img create -f qcow2 foo.qcow2 64M > Formatting 'foo.qcow2', fmt=qcow2 size=67108864 cluster_size=65536 > lazy_refcounts=off refcount_bits=16 > $ qemu-img convert -f qcow2 -O qcow2 foo.qcow2 foo.qcow2 > qemu-img: foo.qcow2: error while converting qcow2: Failed to get "write" > lock > Is another process using the image? > $ qemu-img info foo.qcow2 > image: foo.qcow2 > file format: raw > virtual size: 0 (0 bytes) > disk size: 0 > > (See also https://bugzilla.redhat.com/show_bug.cgi?id=1519144) > > Here is what's happening: file-posix opens the file with > O_CREAT | O_TRUNC, thus truncating the file to 0. Then qcow2 tries to > format it, but fails because it needs the WRITE permission to do so. So > it cannot format it and the file stays empty. > > We should actually take a WRITE and a RESIZE permission before we > truncate the file, and this is what this series does. > > I personally consider the solution taken here a bit of a hack, but it > works and we don't have any locking in any drivers but file-posix > anyway, so it isn't lacking anything in that regard. Integrating it in > blockdev-create might be possible, but there are two issues: > (1) It would be harder. > (2) What would we do anyway? We'd advise protocol drivers to take WRITE > and RESIZE permissions before they truncate a file to be empty... > Well, and then they'd do exactly what file-posix is made to do in > this series. > > So basically what I consider a hack could be seen as exactly the right > way to tackle the issue: Protocol drivers have to ensure the correct > permissions (and they have to choose what those are) are applied before > changing any file -- which is what this series implements. > > > v2: > - Patch 2: [Fam] > - Add a note that while not sharing the RESIZE permission does protect > us from other block-layer-infused programs resizing the file while > raw_co_create() is active; it does not protect against anyone > resizing the file afterwards. > - Drop the unnecessary second raw_apply_lock_bytes() > > > git-backport-diff against v1: > > Key: > [----] : patches are identical > [####] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, > respectively > > 001/3:[----] [--] 'block/file-posix: Pass FD to locking helpers' > 002/3:[0016] [FC] 'block/file-posix: File locking during creation' > 003/3:[----] [--] 'iotests: Add creation test to 153' > > > Max Reitz (3): > block/file-posix: Pass FD to locking helpers > block/file-posix: File locking during creation > iotests: Add creation test to 153 > > block/file-posix.c | 64 > +++++++++++++++++++++++++++++++++++----------- > tests/qemu-iotests/153 | 18 +++++++++++++ > tests/qemu-iotests/153.out | 13 ++++++++++ > 3 files changed, 80 insertions(+), 15 deletions(-)
Applied to my block branch. Max
signature.asc
Description: OpenPGP digital signature
