From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56311) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuHv8-0004Ll-El for qemu-devel@nongnu.org; Mon, 18 May 2015 06:05:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YuHv7-00059y-DY for qemu-devel@nongnu.org; Mon, 18 May 2015 06:05:14 -0400 Date: Mon, 18 May 2015 11:05:05 +0100 From: Stefan Hajnoczi Message-ID: <20150518100505.GE2679@stefanha-thinkpad.redhat.com> References: <1431522721-3266-1-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6Vw0j8UKbyX0bfpA" Content-Disposition: inline In-Reply-To: <1431522721-3266-1-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 0/3] block: Fix unaligned bdrv_aio_write_zeroes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org, qemu-stable@nongnu.org --6Vw0j8UKbyX0bfpA Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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; >=20 > An unaligned zero write causes NULL deferencing in bdrv_co_do_pwritev. Th= at > path is reachable from bdrv_co_write_zeroes and bdrv_aio_write_zeroes. >=20 > 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 ha= ve > 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 call= ing > bdrv_co_write_zeroes because it's in a coroutine). >=20 > 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. >=20 >=20 >=20 > 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 >=20 > 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 =E2=80=98bdrv_co_do_pwritev=E2=80=99: block/io.c:1193:9: error: =E2=80=98ret=E2=80=99 may be used uninitialized i= n this function [-Werror=3Dmaybe-uninitialized] int ret; ^ I have squashed a fix. Stefan --6Vw0j8UKbyX0bfpA Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEbBAEBAgAGBQJVWblRAAoJEJykq7OBq3PIEmgH9ivFO4Rla4fZf/hbdRYKdbV8 q396F8PVB5pzTV5VyZwcZINH/wuvrHK2VjAM4aLuqhOoV3grPZDcybGdIca+nFor z2mvsQVcIXgyhZ/JBCDGgyMr6FGaH1uB9pbQeSBsm6khaXEaWp2eT0gTNLeN0Cqx Bl6cxFreWKENeLRohv0kdXvNb53xS4wiS43iEc9sJToCrjGPWg41CdZvNcjvGOig pxRXtabCFDhM48IUjcUjrGy7yILjbgRAhEiW32JAZds0MHphc3qK1RiIudbsWNlM 69L7FDXxumZrvRoQ/oqwntexVYNLc/+sy/5u5bo0S7wlkSqOMlz99wWVGr+m9g== =/RgI -----END PGP SIGNATURE----- --6Vw0j8UKbyX0bfpA--