[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v7 0/3] block: Fix unaligned bdrv_aio_write_zero

From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v7 0/3] block: Fix unaligned bdrv_aio_write_zeroes
Date: Mon, 18 May 2015 11:05:05 +0100
User-agent: Mutt/1.5.23 (2014-03-12)

On Wed, May 13, 2015 at 01:11:58PM +0000, Fam Zheng wrote:
> v7: Add Kevin's rev-by in patch 1 and 3.
>     Address Stefan's and Kevin's comments on patch 2:
>     - Don't duplicate tracked_request_begin and tracked_request_end;
>     - Don't forget to remove debug printf;
>     - Call qemu_vfree unconditionally;
>     - Don't serialize aligned part of the zero write req;
> An unaligned zero write causes NULL deferencing in bdrv_co_do_pwritev. That
> path is reachable from bdrv_co_write_zeroes and bdrv_aio_write_zeroes.
> You can easily trigger through the former with qemu-io, as the test case added
> by 61815d6e0aa. For bdrv_aio_write_zeroes, in common cases there's always a
> format driver (which uses 512 alignment), so it would be much rarer to have
> unaligned requests (only concerning top level here, when the request goes down
> to bs->file, where for example the alignment is 4k, it would then be calling
> bdrv_co_write_zeroes because it's in a coroutine).
> fc3959e4669a1c fixed bdrv_co_write_zeroes but not bdrv_aio_write_zeroes.  The
> lattern is the actually used one by device model. Revert the previous fix, do
> it in bdrv_co_do_pwritev, to cover both paths.
> Fam Zheng (3):
>   Revert "block: Fix unaligned zero write"
>   block: Fix NULL deference for unaligned write if qiov is NULL
>   qemu-iotests: Test unaligned sub-block zero write
>  block/io.c                 | 142 
> ++++++++++++++++++++++++++++++++-------------
>  tests/qemu-iotests/033     |  13 +++++
>  tests/qemu-iotests/033.out |  30 ++++++++++
>  3 files changed, 144 insertions(+), 41 deletions(-)

By the way, gcc 4.9.2 says:

block/io.c: In function ‘bdrv_co_do_pwritev’:
block/io.c:1193:9: error: ‘ret’ may be used uninitialized in this function 
     int ret;

I have squashed a fix.


Attachment: pgpdRxahuqQBl.pgp
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]