From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSlKz-0006Tu-3Y for qemu-devel@nongnu.org; Mon, 08 Feb 2016 07:54:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aSlKy-000866-3h for qemu-devel@nongnu.org; Mon, 08 Feb 2016 07:54:41 -0500 References: <1454637630-10585-1-git-send-email-famz@redhat.com> <1454637630-10585-2-git-send-email-famz@redhat.com> <56B5F3F9.2030509@redhat.com> <20160207124620.GA24411@ad.usersys.redhat.com> From: Max Reitz Message-ID: <56B89006.3040108@redhat.com> Date: Mon, 8 Feb 2016 13:54:30 +0100 MIME-Version: 1.0 In-Reply-To: <20160207124620.GA24411@ad.usersys.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NldUso4c5PRNM1IiRV7TElHiGcCl1kMw7" Subject: Re: [Qemu-devel] [PATCH v12 1/2] mirror: Rewrite mirror_iteration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , pbonzini@redhat.com, Jeff Cody , qemu-devel@nongnu.org, qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --NldUso4c5PRNM1IiRV7TElHiGcCl1kMw7 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 07.02.2016 13:46, Fam Zheng wrote: > On Sat, 02/06 14:24, Max Reitz wrote: >> On 05.02.2016 03:00, Fam Zheng wrote: >>> The "pnum < nb_sectors" condition in deciding whether to actually cop= y >>> data is unnecessarily strict, and the qiov initialization is >>> unnecessarily for bdrv_aio_write_zeroes and bdrv_aio_discard. >>> >>> Rewrite mirror_iteration to fix both flaws. >>> >>> The output of iotests 109 is updated because we now report the offset= >>> and len slightly differently in mirroring progress. >>> >>> Signed-off-by: Fam Zheng >>> --- >>> block/mirror.c | 335 +++++++++++++++++++++++++++--------= ---------- >>> tests/qemu-iotests/109.out | 80 +++++------ >>> trace-events | 1 - >>> 3 files changed, 243 insertions(+), 173 deletions(-) >>> >>> diff --git a/block/mirror.c b/block/mirror.c >>> index 2c0edfa..48cd0b3 100644 >>> --- a/block/mirror.c >>> +++ b/block/mirror.c >> >> [...] >> >>> @@ -449,16 +520,16 @@ static void coroutine_fn mirror_run(void *opaqu= e) >>> */ >>> bdrv_get_backing_filename(s->target, backing_filename, >>> sizeof(backing_filename)); >>> - if (backing_filename[0] && !s->target->backing) { >>> - ret =3D bdrv_get_info(s->target, &bdi); >>> - if (ret < 0) { >>> - goto immediate_exit; >>> - } >>> - if (s->granularity < bdi.cluster_size) { >>> - s->buf_size =3D MAX(s->buf_size, bdi.cluster_size); >>> - s->cow_bitmap =3D bitmap_new(length); >>> - } >>> + if (!bdrv_get_info(s->target, &bdi) && bdi.cluster_size) { >> >> This should be bdi.has_cluster_size... >=20 > has_cluster_size is a member of ImageInfo not BlockDriverInfo, and is d= erived > from (bdi.cluster_size !=3D 0). You're right, my bad. >>> + target_cluster_size =3D bdi.cluster_size; >> >> ...and maybe we want an explicit minimum of BDRV_SECTOR_SIZE here; but= I >> guess this is already assumed all over the block layer, so it may be >> fine without. >=20 > Okay, it doesn't hurt to add an assert here. I'd be happy to take the patch without, too (although I wouldn't decline a follow-up adding an assertion). Reviewed-by: Max Reitz >>> } >>> + if (backing_filename[0] && !s->target->backing >>> + && s->granularity < target_cluster_size) { >>> + s->buf_size =3D MAX(s->buf_size, target_cluster_size); >>> + s->cow_bitmap =3D bitmap_new(length); >>> + } >>> + s->target_cluster_sectors =3D target_cluster_size >> BDRV_SECTOR= _BITS; >> >> (this shouldn't be 0, so target_cluster_size needs to be at least >> BDRV_SECTOR_SIZE) >> >> Max >> >=20 >=20 --NldUso4c5PRNM1IiRV7TElHiGcCl1kMw7 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWuJAGAAoJEDuxQgLoOKytJtgH/2/yYxTNDezVuGCTHn6Wb9cg FUNNDiYOh8kMKpErmU4ki7n48ujEYiItpI+KaNgf5fMmqCx4zOirVXp5gNYjqdiv EFV0EIKCP4q3jIc0A2PPPdsd6EZiky0nHednLkstYGu1rRqX4ceksHHi7rYqgnx5 bAvrZryrWmZ6Oh/0rtkBq0r9JVO60dxwWPD9oz4tCdZGlsZnopeg+VxiZ6NQMkCF xMF4A97LTFVnvFTkhMNT+4w9uw6jED3GvQQxHWHvrPQRJHtMNRXpUaKPj4K6LiUu 6xPMEIt5zI6OnuRphKOkV4QfcjkqUXr/nGXOKzAbl38vMyC2nTO5SZqLoY08ZUU= =aiEh -----END PGP SIGNATURE----- --NldUso4c5PRNM1IiRV7TElHiGcCl1kMw7--