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

[Marcelo Tosatti]

On Thu, Apr 19, 2012 at 09:38:29AM +0200, Kevin Wolf wrote:
> 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?

address@hidden kvm]# qemu-img info
/root/git/kvm-autotest/client/tests/kvm/images/fc8-64.qcow2 
image: /root/git/kvm-autotest/client/tests/kvm/images/fc8-64.qcow2
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 4.0G
cluster_size: 65536
address@hidden kvm]# qemu-img check
/root/git/kvm-autotest/client/tests/kvm/images/fc8-64.qcow2 
ERROR OFLAG_COPIED: offset=8000000080000000 refcount=2
ERROR refcount block 1 refcount=2

2 errors were found on the image.
Data may be corrupted, or further writes to the image may corrupt it.

> 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.