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

[Kevin Wolf]

Am 19.04.2012 04:44, schrieb Marcelo Tosatti:
> On Mon, Mar 12, 2012 at 04:19:45PM +0100, Kevin Wolf wrote:
>> Signed-off-by: Kevin Wolf <address@hidden>
>> Reviewed-by: Stefan Hajnoczi <address@hidden>
>> ---
>>  block/qcow2-cluster.c |   55 
>> ++++++++++++++++++++++++++++++++-----------------
>>  1 files changed, 36 insertions(+), 19 deletions(-)
> 
> Kevin,
> 
> Autotest installed Fedora.8.64 guests boot with a corrupt disk 
> (see screenshot).
> 
> Reverting
> 
> qcow2: Reduce number of I/O requests
> qcow2: Add qcow2_alloc_clusters_at()
> qcow2: Factor out count_cow_clusters
> 
> Fixes the problem.

Do you really need to revert the latter two? They should be really
harmless, the first is adding yet dead code, the second one is trivial
code motion.

> Let me know if there is a fix available or you need further information.

No, this is the first report of such corruption, so any further
information would be very helpful. Does qemu-img check find any problems?

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.

Kevin