From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51524) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ascLN-0004bh-Uy for qemu-devel@nongnu.org; Tue, 19 Apr 2016 16:33:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ascLM-0001bU-Qi for qemu-devel@nongnu.org; Tue, 19 Apr 2016 16:33:57 -0400 References: <1461060549-8667-1-git-send-email-famz@redhat.com> <1461060549-8667-2-git-send-email-famz@redhat.com> From: Max Reitz Message-ID: <57169629.2090404@redhat.com> Date: Tue, 19 Apr 2016 22:33:45 +0200 MIME-Version: 1.0 In-Reply-To: <1461060549-8667-2-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="nOHTFmVxKtmF5Nd38IsSGrGJqimbk7Wmb" Subject: Re: [Qemu-devel] [PATCH 1/3] mirror: Don't extend the last sub-chunk List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Jeff Cody , Kevin Wolf , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nOHTFmVxKtmF5Nd38IsSGrGJqimbk7Wmb Content-Type: multipart/mixed; boundary="cph20PvvKsks9BDQiMsqwRkUkoJtVdTpB" From: Max Reitz To: Fam Zheng , qemu-devel@nongnu.org Cc: Jeff Cody , Kevin Wolf , qemu-block@nongnu.org Message-ID: <57169629.2090404@redhat.com> Subject: Re: [PATCH 1/3] mirror: Don't extend the last sub-chunk References: <1461060549-8667-1-git-send-email-famz@redhat.com> <1461060549-8667-2-git-send-email-famz@redhat.com> In-Reply-To: <1461060549-8667-2-git-send-email-famz@redhat.com> --cph20PvvKsks9BDQiMsqwRkUkoJtVdTpB Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 19.04.2016 12:09, Fam Zheng wrote: > The last sub-chunk is rounded up to the copy granularity in the target > image, resulting in a larger size than the source. >=20 > Add a function to clip the copied sectors to the end. >=20 > This undoes the "wrong" changes to tests/qemu-iotests/109.out in > e5b43573e28. The remaining two offset changes are okay. >=20 > Reported-by: Kevin Wolf > Signed-off-by: Fam Zheng > --- > block/mirror.c | 10 ++++++++++ > tests/qemu-iotests/109.out | 44 ++++++++++++++++++++++----------------= ------ > 2 files changed, 32 insertions(+), 22 deletions(-) >=20 > diff --git a/block/mirror.c b/block/mirror.c > index c2cfc1a..b6387f1 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -205,6 +205,14 @@ static inline void mirror_wait_for_io(MirrorBlockJ= ob *s) > s->waiting_for_io =3D false; > } > =20 > +static inline void mirror_clip_sectors(MirrorBlockJob *s, > + int64_t sector_num, > + int *nb_sectors) > +{ > + *nb_sectors =3D MIN(*nb_sectors, > + s->bdev_length / BDRV_SECTOR_SIZE - sector_num);= > +} > + > /* Submit async read while handling COW. > * Returns: nb_sectors if no alignment is necessary, or > * (new_end - sector_num) if tail is rounded up or down due t= o > @@ -240,6 +248,7 @@ static int mirror_do_read(MirrorBlockJob *s, int64_= t sector_num, > mirror_wait_for_io(s); > } > =20 > + mirror_clip_sectors(s, sector_num, &nb_sectors); > /* Allocate a MirrorOp that is used as an AIO callback. */ > op =3D g_new(MirrorOp, 1); > op->s =3D s; I think you want to adjust the ret value, too. It doesn't really matter in practice (the caller just overshoots the end of the image instead of getting precisely to its end), but I wouldn't rely on this. > @@ -276,6 +285,7 @@ static void mirror_do_zero_or_discard(MirrorBlockJo= b *s, > { > MirrorOp *op; > =20 > + mirror_clip_sectors(s, sector_num, &nb_sectors); > /* Allocate a MirrorOp that is used as an AIO callback. The qiov i= s zeroed > * so the freeing in mirror_iteration_done is nop. */ > op =3D g_new0(MirrorOp, 1); I think it would be best to just pull out the mirror_clip_sectors() from these functions and put it above the "switch (mirror_method)" in mirror_iteration(). We'd just have to make sure that mirror_cow_align() will not increase nb_sectors such that it points beyond the image end. It can do that, because and image's size does not need to be aligned to its cluster size. But just putting a mirror_clip_sectors() below the bdrv_round_to_clusters() call in mirror_cow_align() should fix that. Then you wouldn't need to worry about fixing the ret value in mirror_do_read(). Max --cph20PvvKsks9BDQiMsqwRkUkoJtVdTpB-- --nOHTFmVxKtmF5Nd38IsSGrGJqimbk7Wmb 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 iQEcBAEBCAAGBQJXFpYpAAoJEDuxQgLoOKytaX0H/2K5RN98zyPNsCz5GuCY36sh AIMtVEnYIbL9bnfBUerbVoAXktXxus9e2JaBquRcRCm7Qx2z1yxu7AXn3hpOgzwK PIbFoUkLcYiaATVT4DNJYmzMolZUQVyKmmNuZbYpDPnBZCPmTsMAwWxnIQTc7kxM UdnMqLQqUdYucdPikWs9LS+aXiHpxPk29ASzNeNW76RBlut2N/1T8hPH8R+/p12T MlDkKpT2C4lVWRGyfosSoAv8Sk9w19PlctFGEblXUWN63I+6DHyck1FG2yUCwDCm /56bMoJBop/LmspH1aY4O2cURLoYMaZA7erecTaYDQc0zvzjQSViuMFKA9ZzKWE= =oQiV -----END PGP SIGNATURE----- --nOHTFmVxKtmF5Nd38IsSGrGJqimbk7Wmb--