From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37561) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b835y-0001BE-D4 for qemu-devel@nongnu.org; Wed, 01 Jun 2016 06:09:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b835t-0005NU-7w for qemu-devel@nongnu.org; Wed, 01 Jun 2016 06:09:49 -0400 Received: from mail-db5eur01on0109.outbound.protection.outlook.com ([104.47.2.109]:16516 helo=EUR01-DB5-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b835s-0005N1-CJ for qemu-devel@nongnu.org; Wed, 01 Jun 2016 06:09:45 -0400 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> <574DFB7D.6010205@openvz.org> <574DFE80.9030802@redhat.com> From: Pavel Butsykin Message-ID: <574EB0A1.1000404@virtuozzo.com> Date: Wed, 1 Jun 2016 12:53:37 +0300 MIME-Version: 1.0 In-Reply-To: <574DFE80.9030802@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit 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 , "Denis V. Lunev" , Stefan Hajnoczi Cc: Kevin Wolf , Jeff Cody , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , John Snow On 01.06.2016 00:13, Eric Blake wrote: > On 05/31/2016 03:00 PM, Denis V. Lunev wrote: >> On 05/31/2016 09:42 PM, Eric Blake wrote: >>> On 05/30/2016 06:58 AM, Pavel Butsykin wrote: >>> >>>> Sorry, but it seems this will never happen, because the second write >>>> will not pass this check: >>>> >>>> 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 = 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; >>>> } >>>> ... >>>> >>>> As you can see we can't do the compressed write in the already allocated >>>> cluster. >>> 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. >>> >> AFAIK the file will be sparse in that unused areas > > IIRC, on the NTFS file system, the minimum hole size is 64k. If you also > have 64k clusters, then you don't have a sparse file - every tail of > zero sectors will be explicit in the filesystem, if you are using 1:1 > clusters for compression. Other file systems may have finer granularity > for holes, but it's still rather awkward to be relying on sparseness > when a better solution is to pack compressed sectors consecutively. > Maybe you're right, I agree that the above mentioned issues are taking place and it would be good to solve them, but it looks like the next step. The solution that you proposed has a direct relation to the improvement of the format driver that goes beyond the goals of this patch-set, where the main goal is to provide opportunities compression from the format drivers for the backup function.