Re: [Qemu-devel] [PATCH for-2.12 v4] iotests: Test abnormally large size in compressed cluster descriptor

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4012 bytes --]

On 2018-03-28 19:58, Eric Blake wrote:
> On 03/28/2018 12:34 PM, Max Reitz wrote:

[...]

>> The OFLAG_COPIED repair looks a bit interesting, but, er, well.
>>
>> Max
>>
>> (Since a compressed cluster does not correspond 1:1 to a host cluster,
>> you cannot say that a compressed cluster has a refcount -- only host
>> clusters are refcounted.  So what is it supposed to mean that a
>> compressed cluster has a refcount of 1?
> 
> A compressed cluster may affect the refcount of multiple clusters: the
> cluster that contains the initial offset, and the cluster that contains
> any of the nb_sectors that did not fit in the first cluster.  So, if I
> have a 4k-cluster image (where each character is a sector), and where
> the compressed clusters are nicely sector-aligned:
> 
> |1-------|2-------|3-------|
> |AAAAAABB|BBBBCCCC|CC------|
> 
> Here, the L2 entry for A, B, and C each list nb_sectors of 5, as it
> takes 6 sectors to list the entire image, but nb_sectors does not
> include the sector that includes the original offset.  The refcount for
> cluster 1 is 2 (the full contents of compressed A and the first half of
> compressed B); for cluster 2 is 2 (the second half of compressed B and
> the first half of compressed C); and for cluster 3 is 1 (the second half
> of compressed C).
> 
> But what this patch is dealing with is when nb_sectors is larger than
> required.  With 4k sectors, qemu will never populate nb_clusters more
> than 8 (if the output is not nicely aligned, and 4096 bytes compresses
> down to only 4095, we can end up with 1 byte in the first sector, then 7
> complete sectors, then 510 bytes in a final sector, for 8 sectors beyond
> the initial offset).  But the qcow2 image is still valid even if the L2
> entry claims nb_sectors of 15; if that happens, then a compressed
> cluster can now affect the refcount of 3 clusters rather than the usual
> 1 or 2.
> 
>>
>> I'd argue from a point of usefulness.  In theory, you could modify
>> compressed clusters in-place, and then you'd need the information
>> whether you are allowed to.  But that doesn't really depend on whether
>> the host clusters touched by the compressed cluster have a refcount of
>> 1, instead if depends on how many times the compressed cluster itself is
>> referenced.  Unfortunately we have no refcounting structure for that.
>>
>> So all in all OFLAG_COPIED is just useless for compressed clusters.)
> 
> In general, because we intentionally allow multiple compressed clusters
> to (try to) share a single host cluster, the refcount for compressed
> clusters will usually be larger than 1.  And OFLAG_COPIED is only useful
> when the refcount is 1, right?

OFLAG_COPIED is only useful when it reflects the number of references to
the data cluster, because only then can we update it in-place without
having to worry about other users.  For normal clusters, one data
cluster is one host cluster, so we can get the number of references from
the refcount structures (which contain the reference counts for host
clusters, but *not* for data clusters).

But for compressed clusters, the data cluster may be part of a host
cluster, or it may even be multiple host clusters.  So the information
we get from the refcount structures is totally useless here, it doesn't
tell us how many references there are to that compressed data cluster.

There is only one exception: If all of the host clusters the compressed
cluster touches have a refcount of 1, we can be certain that there are
no other users, so then we know there is only one reference to the
compressed cluster.  But as soon as they have a higher refcount... We
don't know whether that's because of multiple references to this
compressed cluster or because of multiple compressed cluster sharing a
single host cluster.

In practice it doesn't really matter of course because modifying
compressed clusters in-place seems like a whole different can of worms.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]