Re: [Qemu-devel] [PATCH 02/10] qcow2: add qcow2_co_write_compressed

[Kevin Wolf]

Am 31.05.2016 um 20:42 hat Eric Blake geschrieben:
> 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.

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

pgpIh7QLaULTZ.pgp
Description: PGP signature