Re: [Qemu-block] [PATCH v2 0/3] block/file-posix: File locking during creation

[Max Reitz]

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