From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57583) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b82VC-0007pc-TA for qemu-devel@nongnu.org; Wed, 01 Jun 2016 05:31:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b82V8-0003bU-Rs for qemu-devel@nongnu.org; Wed, 01 Jun 2016 05:31:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59044) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b82V8-0003bO-Jf for qemu-devel@nongnu.org; Wed, 01 Jun 2016 05:31:46 -0400 Date: Wed, 1 Jun 2016 11:31:43 +0200 From: Kevin Wolf Message-ID: <20160601093143.GC5356@noname.str.redhat.com> References: <1463229957-14253-1-git-send-email-den@openvz.org> <1463229957-14253-3-git-send-email-den@openvz.org> <20160527173335.GF27946@stefanha-x1.localdomain> <574C0404.9040507@virtuozzo.com> <574C38D9.8020709@virtuozzo.com> <574DDB2E.9070801@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rS8CxjVDS/+yyDmU" Content-Disposition: inline In-Reply-To: <574DDB2E.9070801@redhat.com> Subject: Re: [Qemu-devel] [PATCH 02/10] qcow2: add qcow2_co_write_compressed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Pavel Butsykin , Stefan Hajnoczi , "Denis V. Lunev" , Jeff Cody , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , John Snow --rS8CxjVDS/+yyDmU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 31.05.2016 um 20:42 hat Eric Blake geschrieben: > On 05/30/2016 06:58 AM, Pavel Butsykin wrote: >=20 > >=20 > > Sorry, but it seems this will never happen, because the second write > > will not pass this check: > >=20 > > uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs, > > uint64_t offset, > > int compressed_size) > > { > > ... > > /* Compression can't overwrite anything. Fail if the cluster was > > already > > * allocated. */ > > cluster_offset =3D be64_to_cpu(l2_table[l2_index]); > > if (cluster_offset & L2E_OFFSET_MASK) { > > qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table); > > return 0; > > } > > ... > >=20 > > As you can see we can't do the compressed write in the already allocated > > cluster. >=20 > Umm, doesn't that defeat the point of compression, if every compressed > cluster becomes the head of a new cluster? The whole goal of > compression is to be able to fit multiple clusters within one. With compression, be careful what kind of clusters you're looking at. The clusters in the code above are guest clusters and you can't overwrite a guest cluster that has already been written (I'm not sure why, though - didn't matter so far because the only writer is qemu-img convert, which writes the whole image sequentially and doesn't split clusters across requests, but if we want to allow compressed writes in a running VM, we may want to change this). However, the compressed data of multiple guest clusters may end up in the same host cluster, and that's indeed the whole point of compression. Kevin --rS8CxjVDS/+yyDmU Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXTqt/AAoJEH8JsnLIjy/W0CoQAKTOFRNN4a+dbVYpIvzyJUX+ ffrMpcepE1SC4s5xseruQ/riR6G1zA92e4ouIsQI4LyfZ+YBjwkrgyR4i1yylIxx RQj1bEH1Vc9WGIuPlUXYzit89udsA8xnwVQVfw7e/ZZl7qJSz5uhc2AjH/cdP98d fdQHcw28OrF0Owa2xfd9qRdepFYBP+9t4Cr76+xGQjfsPR+jhmXQKeQ6rO4jA/eB K72y7Fh9jia7kAWYZX6lQdNGeGSabI4NfPpqf/D78DUajKjojHHubeHZwY9boq1B IUeHbwH+QF7cxYrQVRkPE2TzDwS/KxTLmNtHvp4wH5gScujgCBZWZr/qne8m+uUw 5mbGmuYiWX5xUNv/5P7qvtbj8CHeKKPlMj6xDQnxLHATZxIPq0zN3/nz0CPREm8b py+LNB28wi98IA8HaP9v82BL900OQFpnNpTYH5RgNu728Z01SSaSsL2BHeWK0E0v nKo82YFFru9gi+Vfj+z7Huf26jow10zFQL0Ge3KViOYq7hl81rSAOLmb7fswF+2P 1+seTXKEAlDn50BW/wpvw8GjAYwMmiu1A+GTeYcSCZouxReWH/EyUjGOt6UXC5oD uQtOM7nDmNJ/R89xQJ4Lr0hVWehiz5oGTHUfg4Sl80y5xIukHW5lckv+A2LvTXrX 7b6OOK6NUrJ7jANZLup3 =vyrF -----END PGP SIGNATURE----- --rS8CxjVDS/+yyDmU--