From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8S0F-0007oY-Be for qemu-devel@nongnu.org; Thu, 02 Jun 2016 08:45:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8S0D-0000yp-9D for qemu-devel@nongnu.org; Thu, 02 Jun 2016 08:45:34 -0400 Date: Thu, 2 Jun 2016 14:45:18 +0200 From: Kevin Wolf Message-ID: <20160602124518.GF6867@noname.redhat.com> References: <1464815413-613-1-git-send-email-eblake@redhat.com> <1464815413-613-10-git-send-email-eblake@redhat.com> <20160602111647.GD6867@noname.redhat.com> <5750294F.6030102@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="DKU6Jbt7q3WqK7+M" Content-Disposition: inline In-Reply-To: <5750294F.6030102@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 09/13] qed: Convert to bdrv_co_pwrite_zeroes() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Stefan Hajnoczi , Max Reitz --DKU6Jbt7q3WqK7+M Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 02.06.2016 um 14:40 hat Eric Blake geschrieben: > On 06/02/2016 05:16 AM, Kevin Wolf wrote: > > Am 01.06.2016 um 23:10 hat Eric Blake geschrieben: > >> Another step on our continuing quest to switch to byte-based > >> interfaces. > >> > >> Kill an abuse of the comma operator while at it (fortunately, > >> the semantics were still right). Also, the test for requests > >> not aligned to clusters should be applied always, not just > >> when a backing file is present. > >> > >> Signed-off-by: Eric Blake > >> --- > >> block/qed.c | 33 +++++++++++++++------------------ > >> 1 file changed, 15 insertions(+), 18 deletions(-) >=20 > >> - } > >> + /* Fall back if the request is not */ > >=20 > > ...aligned? >=20 > Yes, thanks. >=20 > >=20 > >> + if (qed_offset_into_cluster(s, offset) || > >> + qed_offset_into_cluster(s, count)) { > >> + return -ENOTSUP; > >> } > >=20 > > This is obviously correct and almost as obviously suboptimal compared to > > the original version (we need cluster alignment with a backing file, but > > without a backing file, sector alignment would be enough). >=20 > Does QED support per-sector zeroing, or is it only per-cluster zero like > qcow2? >=20 > /me checks the spec >=20 > only per-cluster zeroing >=20 > >=20 > > But as this is QED, which is only supported for compatibility these > > days, simpler if slightly suboptimal code is okay with me. >=20 > Widening a request to cluster boundaries is only possible if you know > the cluster is otherwise unallocated or already reads as zeroes, and > unless qed tracks zero sectors (as opposed to zero clusters), I argue > that this was actually a bug fix, even when the request was > sector-aligned, since we lack the check for cluster remains > unallocated/zero before widening, the way qcow2 does it. Sub-optimal > but safe is better than incorrectly optimized. It actually checks whether the cluster is already allocated; in that case, qed implements a fallback itself. So I think it was working. Just as usual with the callback based qed code, it's not very easy to read. Kevin --DKU6Jbt7q3WqK7+M Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXUCpeAAoJEH8JsnLIjy/W9BYP/2AZkfyECeWRm34f4PW/nYYP h3pDKPcJCFMtEYFsHzni7Y33mPauhDHJUIu4koBXpzyJ4xbjtbzLErjjx2xH1w/+ qy7ZNqr9R+032YyY/adJFeuWce/5dXR7/bTcxv4M1w4nfuzxRhaZ3ruF3ESMHNdV b+DkqwEP/Z+UbMt9kSk5extUnAhPR7pSqfN2sb8GrSyQLj2xt0aiVVwgxrCfELtG UjZwOs/N8SU0lpfI7jHNKuBydkhWNr1ZgYWyBtSSPdImoXgXSIyaF2gG+j1Cimlz EkePveFvZJyfbyX57myLz2iaPFXZovCbYsEXJwU06ESksuOhRXynISA/JScYD64r zezi2MHJOtpCYyqFKJdaN5aiVBn9atOtA+WHxOXsPYAeykX6gcrP9zVDvAHvC2JD Cwn9X8br7qxdaxNe+QbQ02+s5NCjkwNT/iup/hvKAYvxfeKpKBY5TyCli98E2wAT lBTCaNgT8kL4EtMzm26+K6uUJra5jb7DyZIjAF4Tq3BA1Y+R/YSE9xsT4EaAsAP5 RY/1V7O+eC0R+ya4HlbfRcFNYGOIoAgGcfqBd1Seohes/SBD2l2iUtIDtvuPj9E2 bx9+LSEx0Q7rkpckmNW8mO2xQ9tVrmo0HmAfLVXGNLWyi04REuPMKh7DPc3tiIXt Zcn9UDatoaIti7o/LV5Z =c4C4 -----END PGP SIGNATURE----- --DKU6Jbt7q3WqK7+M--