Re: [Qemu-devel] [PATCH 14/20] qcow2: Factor out count_cow_clusters

[Stefan Hajnoczi]

On Fri, Apr 20, 2012 at 1:27 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Fri, Apr 20, 2012 at 9:24 AM, Kevin Wolf <address@hidden> wrote:
>> Am 19.04.2012 23:18, schrieb Marcelo Tosatti:
>>> On Thu, Apr 19, 2012 at 05:14:20PM -0300, Marcelo Tosatti wrote:
>>>>> There is one intended change in functionality in this patch, which is
>>>>> that it allocates new clusters even when it could satisfy the first part
>>>>> of the request with already allocated clusters. In order to check if
>>>>> there is a problem with this scenario, the following patch should revert
>>>>> to the old behaviour:
>>>>>
>>>>> --- a/block/qcow2-cluster.c
>>>>> +++ b/block/qcow2-cluster.c
>>>>> @@ -847,7 +847,7 @@ again:
>>>>>          keep_clusters = count_contiguous_clusters(nb_clusters,
>>>>> s->cluster_size,
>>>>>                                                    &l2_table[l2_index],
>>>>> 0, 0);
>>>>>          assert(keep_clusters <= nb_clusters);
>>>>> -        nb_clusters -= keep_clusters;
>>>>> +        nb_clusters = 0;
>>>>>      } else {
>>>>>          /* For the moment, overwrite compressed clusters one by one */
>>>>>          if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
>>>>>
>>>>> The rest is meant to be a functionally equivalent rewrite of the old
>>>>> code that was required in order to allow this change.
>>>>
>>>> Testing.
>>>
>>> Corruption gone with patch above.
>>
>> Okay, so something must be wrong only with the new code paths, which
>> makes things a bit easier. I'll have another closer look. Stefan, can
>> you re-review 250196f1 as well?
>
> I'm taking a look.

Just looking at the qemu-img check output that Marcelo posted I'm
interpreting that as OFLAG_COPIED was set on the offset but its
refcount was 2.

The refcount shouldn't be 2 unless internal snapshots were used.

So we probably incremented the refcount either too often or for the
wrong block (which is more likely since the guest sees corruption).

Stefan