From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59158) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5GqG-000387-Bp for qemu-devel@nongnu.org; Tue, 24 May 2016 14:14:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b5GqD-0006LT-3l for qemu-devel@nongnu.org; Tue, 24 May 2016 14:14:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59125) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5GqC-0006LN-Rb for qemu-devel@nongnu.org; Tue, 24 May 2016 14:14:05 -0400 References: <1463476543-3087-1-git-send-email-den@openvz.org> <1463476543-3087-6-git-send-email-den@openvz.org> From: Eric Blake Message-ID: <574499EB.6050905@redhat.com> Date: Tue, 24 May 2016 12:14:03 -0600 MIME-Version: 1.0 In-Reply-To: <1463476543-3087-6-git-send-email-den@openvz.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9EThEQfuGU27pt32CH4aPkWtpIV67sukG" Subject: Re: [Qemu-devel] [PATCH 5/5] qcow2: merge is_zero_cluster helpers into qcow2_co_write_zeroes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" , qemu-devel@nongnu.org Cc: Kevin Wolf This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9EThEQfuGU27pt32CH4aPkWtpIV67sukG Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/17/2016 03:15 AM, Denis V. Lunev wrote: > They are used once only. This makes code more compact. >=20 > The patch also improves comments in the code. >=20 > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > --- > block/qcow2.c | 39 +++++++++++++++------------------------ > 1 file changed, 15 insertions(+), 24 deletions(-) >=20 > @@ -2439,20 +2418,32 @@ static coroutine_fn int qcow2_co_write_zeroes(B= lockDriverState *bs, > nb_sectors); > =20 > if (head !=3D 0 || tail !=3D 0) { > - int64_t cl_start =3D sector_num - head; > + BlockDriverState *file; > + uint64_t off; > + int nr; > + > + int64_t cl_start =3D sector_num - head, res; > =20 > assert(cl_start + s->cluster_sectors >=3D sector_num + nb_sect= ors); > =20 > sector_num =3D cl_start; > nb_sectors =3D s->cluster_sectors; > =20 > - if (!is_zero_cluster(bs, sector_num)) { > + /* check that the cluster is zeroed taking into account entire= > + backing chain */ > + nr =3D s->cluster_sectors; > + res =3D bdrv_get_block_status_above(bs, NULL, cl_start, > + s->cluster_sectors, &nr, &fi= le); > + if (res < 0 || !(res & BDRV_BLOCK_ZERO)) { > return -ENOTSUP; > } This is somewhat pessimistic in certain cases. Suppose I have the following cluster (borrowing from Kevin's nice notation in 154): 4k: -- -- XX -- and issue a 'write -z 2k 1k' As written, the code will see that bdrv_get_block_status_above() does NOT have all ZERO for the cluster, so it will return -ENOTSUP; we will then call into the fallbacks and write explicit zeroes, so that the top file now has an allocated cluster full of zero. But if we were a bit smarter, we would only check the allocation status of the backing sectors that do not overlap with our write_zeroes request, because the remaining sectors are about to be overwritten; in this case, we can specifically optimize to write a zero cluster without allocation. You'll also need to rebase this series on top of my pending work to rewrite bdrv_co_write_zeroes to instead be bdrv_co_pwrite_zeroes with a byte interface (patches to be posted later today), if mine gets reviewed first. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --9EThEQfuGU27pt32CH4aPkWtpIV67sukG 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/ iQEcBAEBCAAGBQJXRJnrAAoJEKeha0olJ0NqK8QIAK/a4UZVW8sIfrzjQyZe+GZX LjL81BObSn8258URqBhCU5SEK2UVnOTNC4vpwpDCVz9ryWjlROrxffWFDdioBdvC k0l7QcUtoNZtDkGM3ThnkBI1Yx/2+lg1y0BP4A3S9XSFpMFHTfoMwAgHvCrvgW4+ HbI/zreTW1cD/Eru+3ZL5k1ynVSwDtDk6zsdjAoe/f9iY/GO4gkHjax7wXz4dCVe poHvoEQ4x93swCrpZiBeJFKn6t1oaAGCKKmbVErgmuvm9s8VF3BjkO5Ft2bIdqyM EqQBQuikk85G6nPgCN1P5lV0oaG+foZJl3xDZne3toB8KBbKhkf+Z7BB9YzGvu0= =kOTt -----END PGP SIGNATURE----- --9EThEQfuGU27pt32CH4aPkWtpIV67sukG--