[Qemu-devel] qcow2 corruption caused by r5006

[Qemu-devel] qcow2 corruption caused by r5006

[Nolan]

While working on a semi-related bug fix (to be sent later) I've been
experimenting with the following hack:

Index: qemu-img.c
===================================================================
--- qemu-img.c  (revision 6929)
+++ qemu-img.c  (working copy)
@@ -596,7 +596,7 @@
                assume that sectors which are unallocated in the input image
                are present in both the output's and input's base images (no
                need to copy them). */
-            if (out_baseimg) {
+            if (1 || out_baseimg) {
                if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset, n, &n1)) {
                   sector_num += n1;
                   continue;

My theory being that there is no point making is_allocated_sectors()
memcmp unallocated blocks that we already know will bdrv_read as all
'\0's.  In practice, this results in a nice (2-10x) speed up when doing
a "qemu-img convert" from a sparse format.

The problem is that this breaks "qemu-img convert" when the source is
qcow2 (other formats are fine).  The symptom is that the destination
image has zeroed sectors where the source had actual data!

A binary search narrowed it down to r5006, same as this report:
http://www.mail-archive.com/kvm@vger.kernel.org/msg10416.html

Reverting out that patch fixes it, as does this simpler partial
reversion:
Index: block-qcow2.c
===================================================================
--- block-qcow2.c       (revision 6929)
+++ block-qcow2.c       (working copy)
@@ -1101,11 +1101,19 @@
 static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num,
                              int nb_sectors, int *pnum)
 {
+    BDRVQcowState *s = bs->opaque;
+    int index_in_cluster, n;
     uint64_t cluster_offset;
 
     *pnum = nb_sectors;
     cluster_offset = get_cluster_offset(bs, sector_num << 9, pnum);
-
+#if 0
+    index_in_cluster = sector_num & (s->cluster_sectors - 1);
+    n = s->cluster_sectors - index_in_cluster;
+    if (n > nb_sectors)
+        n = nb_sectors;
+    *pnum = n;
+#endif
     return (cluster_offset != 0);
 }

Though I doubt this partial reversion will fix the issue reported in the
link above.

On the plus side, this (unlike many of the qcow2 corruption reports) is
a trivially and consistently reproducible case.