[Qemu-devel] [PATCH v2 0/2] qcow2: count_contiguous_clusters and compression

[Qemu-devel] [PATCH v2 0/2] qcow2: count_contiguous_clusters and compression

[Max Reitz]

Compressed clusters can never be contiguous and count_contiguous_clusters
will not even work correctly for them. Therefore, those cluster should
always be treated as non-contiguous.

This series makes count_contiguous_clusters always stop at compressed
clusters and removes the corresponding flag from its function calls.

v2:
 - added patch 2 (following Kevin's proposal)
 - patch 1 remains unmodified

Max Reitz (2):
  qcow2: count_contiguous_clusters and compression
  qcow2: COMPRESSED on count_contiguous_clusters

 block/qcow2-cluster.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/2] qcow2: count_contiguous_clusters and compression

[Max Reitz]

The function is not intended to be used on compressed clusters and will
not work correctly, if used anyway, since L2E_OFFSET_MASK is not the
right mask for determining the offset of compressed clusters. Therefore,
assert that the first cluster is not compressed and always include the
compression flag in the mask of significant flags, i.e., stop the search
as soon as a compressed cluster occurs.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index cab5f2e..a62ef42 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -281,12 +281,15 @@ static int count_contiguous_clusters(uint64_t nb_clusters, int cluster_size,
         uint64_t *l2_table, uint64_t start, uint64_t stop_flags)
 {
     int i;
-    uint64_t mask = stop_flags | L2E_OFFSET_MASK;
-    uint64_t offset = be64_to_cpu(l2_table[0]) & mask;
+    uint64_t mask = stop_flags | L2E_OFFSET_MASK | QCOW2_CLUSTER_COMPRESSED;
+    uint64_t first_entry = be64_to_cpu(l2_table[0]);
+    uint64_t offset = first_entry & mask;
 
     if (!offset)
         return 0;
 
+    assert(qcow2_get_cluster_type(first_entry) != QCOW2_CLUSTER_COMPRESSED);
+
     for (i = start; i < start + nb_clusters; i++) {
         uint64_t l2_entry = be64_to_cpu(l2_table[i]) & mask;
         if (offset + (uint64_t) i * cluster_size != l2_entry) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/2] qcow2: COMPRESSED on count_contiguous_clusters

[Max Reitz]

Compressed clusters can never be contiguous, therefore the corresponding
flag does not need to be given explicitly to count_contiguous_clusters.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a62ef42..145fa18 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -490,8 +490,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
             return -EIO;
         }
         c = count_contiguous_clusters(nb_clusters, s->cluster_size,
-                &l2_table[l2_index], 0,
-                QCOW_OFLAG_COMPRESSED | QCOW_OFLAG_ZERO);
+                &l2_table[l2_index], 0, QCOW_OFLAG_ZERO);
         *cluster_offset = 0;
         break;
     case QCOW2_CLUSTER_UNALLOCATED:
@@ -502,8 +501,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
     case QCOW2_CLUSTER_NORMAL:
         /* how many allocated clusters ? */
         c = count_contiguous_clusters(nb_clusters, s->cluster_size,
-                &l2_table[l2_index], 0,
-                QCOW_OFLAG_COMPRESSED | QCOW_OFLAG_ZERO);
+                &l2_table[l2_index], 0, QCOW_OFLAG_ZERO);
         *cluster_offset &= L2E_OFFSET_MASK;
         break;
     default:
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/2] qcow2: count_contiguous_clusters and compression

[Kevin Wolf]

Am 27.09.2013 um 12:14 hat Max Reitz geschrieben:
> Compressed clusters can never be contiguous and count_contiguous_clusters
> will not even work correctly for them. Therefore, those cluster should
> always be treated as non-contiguous.
> 
> This series makes count_contiguous_clusters always stop at compressed
> clusters and removes the corresponding flag from its function calls.
> 
> v2:
>  - added patch 2 (following Kevin's proposal)
>  - patch 1 remains unmodified

Thanks, applied to the block branch.

Kevin