From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5wNy-00078G-62 for qemu-devel@nongnu.org; Thu, 26 May 2016 10:35:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b5wNt-0000IR-5I for qemu-devel@nongnu.org; Thu, 26 May 2016 10:35:41 -0400 References: <1464234529-13018-1-git-send-email-eblake@redhat.com> <1464234529-13018-6-git-send-email-eblake@redhat.com> <5746FCFC.2060708@openvz.org> From: Eric Blake Message-ID: <574709AD.4020805@redhat.com> Date: Thu, 26 May 2016 08:35:25 -0600 MIME-Version: 1.0 In-Reply-To: <5746FCFC.2060708@openvz.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="V2x7OjIuQSwxj8oQi6btd2BmDMFJ63Gku" Subject: Re: [Qemu-devel] [PATCH v3 5/5] qcow2: Catch more unaligned write_zero into zero cluster List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" , qemu-devel@nongnu.org Cc: qemu-block@nongnu.org, kwolf@redhat.com, Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --V2x7OjIuQSwxj8oQi6btd2BmDMFJ63Gku Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/26/2016 07:41 AM, Denis V. Lunev wrote: > On 05/26/2016 06:48 AM, Eric Blake wrote: >> is_zero_cluster() and is_zero_cluster_top_locked() are used only >> by qcow2_co_write_zeroes(). The former is too broad (we don't >> care if the sectors we are about to overwrite are non-zero, only >> that all other sectors in the cluster are zero), so it needs to >> be called up to twice but with smaller limits - rename it along >> with adding the neeeded parameter. The latter can be inlined for >> more compact code. >> >> The testsuite change shows that we now have a sparser top file >> when an unaligned write_zeroes overwrites the only portion of >> the backing file with data. >> >> Based on a patch proposal by Denis V. Lunev. >> >> - >> - if (!is_zero_cluster(bs, sector_num)) { >> + /* check whether remainder of cluster already reads as zero *= / >> + if (!(is_zero_sectors(bs, cl_start, head) && >> + is_zero_sectors(bs, sector_num + nb_sectors, >> + -tail & (s->cluster_sectors - 1)))) { >=20 > can we have cluster_sectors !=3D 2^n? No. cluster_sectors is an inherent property of the qcow2 file format: 20 - 23: cluster_bits Number of bits that are used for addressing an offset= within a cluster (1 << cluster_bits is the cluster size). Must not be less than 9 (i.e. 512 byte clusters). As the file format uses a bit shift value, you are guaranteed to have a power of two amount of sectors within a cluster. If you prefer, I could have written '-tail % s->cluster_sectors', but as % on a negative signed integer gives different results than what you get for an unsigned number, I felt that & was nicer than % for making it more obvious that I'm grabbing particular bits. If you can think of any cleaner expression that represents the number of sectors occurring after the tail until the next cluster boundary, I'm game; the hardest part is that when tail is 0, we want the number passed to is_zero_sectors() to also be 0, not s->cluster_sectors (so the naive 's->cluster_sectors - tail' is wrong). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --V2x7OjIuQSwxj8oQi6btd2BmDMFJ63Gku 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXRwmtAAoJEKeha0olJ0NqHhoIAKvfecyrmcrUtlK4QfCSp0uu 1ynlxk8YAgRmeCweg62cpQaSmMXKnGV6RNO9mpgG/bq87j5GjbNYW6LLO8PqgWaE h9K248XfDtmSd17bcjm99X1QPchFJOhQHWOmVeU6vFH/i745qwltDfUdILcArWkU wgT5MlPzNKn2eWOd6bWmV7YmP2RrKzchW1MwOYByxbvk45FXA9ImvzjV8rGCiHV4 V6OJA0q2OBogQuQfVHtZkZhjHarIUkl0JZww0l60Q5UWDF9B9oDZEfVXqfmbgPfB ebifXqpNY0p97pAzFuGYYgzEGXC2OigIxc6hlx+Djs4soNcxovfV+TaUtAJxyGo= =D6KU -----END PGP SIGNATURE----- --V2x7OjIuQSwxj8oQi6btd2BmDMFJ63Gku--