From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42073) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aR1Ir-0002ou-0I for qemu-devel@nongnu.org; Wed, 03 Feb 2016 12:33:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aR1Iq-0005bU-0b for qemu-devel@nongnu.org; Wed, 03 Feb 2016 12:33:16 -0500 References: <1454464341-9494-1-git-send-email-famz@redhat.com> <1454464341-9494-2-git-send-email-famz@redhat.com> From: Max Reitz Message-ID: <56B239D1.7010007@redhat.com> Date: Wed, 3 Feb 2016 18:33:05 +0100 MIME-Version: 1.0 In-Reply-To: <1454464341-9494-2-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="33HcbKAH9OD20SkVuAc6IHglWrqJhmLx9" Subject: Re: [Qemu-devel] [PATCH v11 1/2] mirror: Rewrite mirror_iteration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , pbonzini@redhat.com, Jeff Cody , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --33HcbKAH9OD20SkVuAc6IHglWrqJhmLx9 Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 03.02.2016 02:52, Fam Zheng wrote: > The "pnum < nb_sectors" condition in deciding whether to actually copy > data is unnecessarily strict, and the qiov initialization is > unnecessarily for bdrv_aio_write_zeroes and bdrv_aio_discard. >=20 > Rewrite mirror_iteration to fix both flaws. >=20 > The output of iotests 109 is updated because we now report the offset > and len slightly differently in mirroring progress. >=20 > 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(-) >=20 > diff --git a/block/mirror.c b/block/mirror.c > index 2c0edfa..8b06a31 100644 > --- a/block/mirror.c > +++ b/block/mirror.c [...] > @@ -449,16 +519,17 @@ static void coroutine_fn mirror_run(void *opaque)= > */ > 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); > - } > + ret =3D bdrv_get_info(s->target, &bdi); > + if (ret < 0) { > + goto immediate_exit; > } > + if (backing_filename[0] && !s->target->backing > + && s->granularity < bdi.cluster_size) { > + s->buf_size =3D MAX(s->buf_size, bdi.cluster_size); > + s->cow_bitmap =3D bitmap_new(length); > + } > + s->target_cluster_sectors =3D bdi.cluster_size >> BDRV_SECTOR_BITS= ; > + s->max_iov =3D MIN(s->common.bs->bl.max_iov, s->target->bl.max_iov= ); Sorry for noticing this so late: bdi.cluster_size is optional. I think we should default to BDRV_SECTOR_SIZE if bdi.has_cluster_size is false, and maybe even if bdrv_get_info() failed. (I noticed this because iotest 94 fails because nbd does not support bdrv_get_info().) Max --33HcbKAH9OD20SkVuAc6IHglWrqJhmLx9 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 iQEcBAEBCAAGBQJWsjnRAAoJEDuxQgLoOKyt2O4H/3RbnQuGeLfOTlbZOhCtWEzN 40unjrMUy/MimyRku84aJCxGROCCge0WMfFBqFhCslBYr0ghs3SLOKKHJrDq7N5F qvTj4dh/YsMolpWXfH7/1EzuWPyA6sejgtBhR4YbJKaKd0k0GlCvktg6d/l7KKsI SXvdFatr/cyUTSdN5TJUT1eAmAz536ygUg0cTlP92+fOJpaqJTFwtiteTxNexg+Z oq0iFuZem4xP+I6OnMM3zk4EL6jL35R4g+MrzMkBXhPtGZCnCQbvxSDvq2ZXvelW mIhQYCuwO3s1xh5T9UPuFmS+BeubHOC201/kM5Q1O6VHDc0u7OU5qSf/tpU0MyM= =VqTd -----END PGP SIGNATURE----- --33HcbKAH9OD20SkVuAc6IHglWrqJhmLx9--