Re: [PATCH for-4.2 v2 0/3] qcow2: Fix data corruption on XFS

[Stefan Hajnoczi]

On Fri, Nov 01, 2019 at 04:25:07PM +0100, Max Reitz wrote:
> Hi,
> 
> As I reasoned here:
> https://lists.nongnu.org/archive/html/qemu-block/2019-11/msg00027.html
> I’m no longer convinced of reverting c8bb23cbdbe.  I could see a
> significant performance improvement from it on ext4 with aio=native in a
> guest that does random 4k writes, and as such I feel it would be a
> regression to revert it for 4.2.
> 
> To work around the XFS corruption, we still need the other three patches
> from the series, of course.  We cannot restrict the workaround to just
> XFS, because maybe the file is on a remote filesystem and then we don’t
> know anything about the host configuration.
> 
> The performance impact should still be minimal because this just
> serializes post-EOF write-zeroes and data writes, and that just doesn’t
> happen very often, even with handle_alloc_space() in qcow2.
> 
> 
> I would love to have more time to make a decision, but there simply
> isn’t any.  Patches for 4.1.1 are to be collected on Monday, AFAIU.
> 
> 
> v2:
> - Dropped patch 1
> - Forgot a “coroutine_fn” in patch 2 (it isn’t really important,
>   qemu_coroutine_self() works in non-coroutine functions, too, but
>   calling bdrv_(co_)get_self_request() from a non-coroutine just doesn’t
>   make any sense)
> - Patch 3:
>   - Reverted the commit message to the RFC state to reflect that this is
>     specifically because of handle_alloc_space()
>   - Dropped the two lines that said there’d be no performance impact at
>     all because no driver would submit zero writes beyond the EOF
>     (because qcow2 now still does)
> 
> 
> 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: Make wait/mark serialising requests public'
> 002/3:[0004] [FC] 'block: Add bdrv_co_get_self_request()'
> 003/3:[0002] [FC] 'block/file-posix: Let post-EOF fallocate serialize'
> 
> 
> Max Reitz (3):
>   block: Make wait/mark serialising requests public
>   block: Add bdrv_co_get_self_request()
>   block/file-posix: Let post-EOF fallocate serialize
> 
>  include/block/block_int.h |  4 ++++
>  block/file-posix.c        | 36 +++++++++++++++++++++++++++++++++
>  block/io.c                | 42 ++++++++++++++++++++++++++++-----------
>  3 files changed, 70 insertions(+), 12 deletions(-)
> 
> -- 
> 2.21.0
> 

Reviewed-by: Stefan Hajnoczi <address@hidden>

signature.asc
Description: PGP signature