diff mbox series

[v5,20/31] qcow2: Add subcluster support to qcow2_get_host_offset()

Message ID d4b41accb4c52f31dc7faa3b58084062ac4407f4.1588699789.git.berto@igalia.com (mailing list archive)
State New, archived
Headers show
Series Add subcluster allocation to qcow2 | expand

Commit Message

Alberto Garcia May 5, 2020, 5:38 p.m. UTC 
The logic of this function remains pretty much the same, except that
it uses count_contiguous_subclusters(), which combines the logic of
count_contiguous_clusters() / count_contiguous_clusters_unallocated()
and checks individual subclusters.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2.h         |  38 +++++-------
 block/qcow2-cluster.c | 141 ++++++++++++++++++++----------------------
 2 files changed, 82 insertions(+), 97 deletions(-)

Comments

Eric Blake May 6, 2020, 5:55 p.m. UTC | #1 
On 5/5/20 12:38 PM, Alberto Garcia wrote:
> The logic of this function remains pretty much the same, except that
> it uses count_contiguous_subclusters(), which combines the logic of
> count_contiguous_clusters() / count_contiguous_clusters_unallocated()
> and checks individual subclusters.
> 

Maybe mention that qcow2_cluster_to_subcluster_type() is now inlined 
into its lone remaining caller.

> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>   block/qcow2.h         |  38 +++++-------
>   block/qcow2-cluster.c | 141 ++++++++++++++++++++----------------------
>   2 files changed, 82 insertions(+), 97 deletions(-)
> 

> +++ b/block/qcow2-cluster.c
> @@ -376,66 +376,58 @@ fail:

> +static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
> +                                        unsigned sc_index, uint64_t *l2_slice,
> +                                        int l2_index)
>   {

> +
> +    assert(type != QCOW2_SUBCLUSTER_INVALID); /* The caller should check this */
> +    assert(l2_index + nb_clusters <= s->l2_size);
> +
> +    if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
> +        /* Compressed clusters are always processed one by one */
> +        return s->subclusters_per_cluster - sc_index;

Should this assert(sc_index == 0)?

>       for (i = 0; i < nb_clusters; i++) {
> -        uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
> -        QCow2ClusterType type = qcow2_get_cluster_type(bs, entry);
> -
> -        if (type != wanted_type) {
> -            break;
> +        l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
> +        l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
> +        if (check_offset && expected_offset != (l2_entry & L2E_OFFSET_MASK)) {
> +            return count;
> +        }
> +        for (j = (i == 0) ? sc_index : 0; j < s->subclusters_per_cluster; j++) {
> +            if (qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, j) != type) {
> +                return count;
> +            }

This really is checking that sub-clusters have the exact same type.

> @@ -604,24 +604,17 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
>               ret = -EIO;
>               goto fail;
>           }
> -        /* Compressed clusters can only be processed one by one */
> -        c = 1;
>           *host_offset = l2_entry & L2E_COMPRESSED_OFFSET_SIZE_MASK;
>           break;
> -    case QCOW2_CLUSTER_ZERO_PLAIN:
> -    case QCOW2_CLUSTER_UNALLOCATED:
> -        /* how many empty clusters ? */
> -        c = count_contiguous_clusters_unallocated(bs, nb_clusters,
> -                                                  l2_slice, l2_index, type);

The old code was counting how many contiguous clusters has similar 
types, coalescing ranges of two different cluster types into one 
nb_clusters result.

> +    case QCOW2_SUBCLUSTER_ZERO_PLAIN:
> +    case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
>           *host_offset = 0;
>           break;
> -    case QCOW2_CLUSTER_ZERO_ALLOC:
> -    case QCOW2_CLUSTER_NORMAL: {
> +    case QCOW2_SUBCLUSTER_ZERO_ALLOC:
> +    case QCOW2_SUBCLUSTER_NORMAL:
> +    case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC: {
>           uint64_t host_cluster_offset = l2_entry & L2E_OFFSET_MASK;
>           *host_offset = host_cluster_offset + offset_in_cluster;
> -        /* how many allocated clusters ? */
> -        c = count_contiguous_clusters(bs, nb_clusters, s->cluster_size,
> -                                      l2_slice, l2_index, QCOW_OFLAG_ZERO);

and here coalescing three different cluster types into one nb_clusters 
result.

>           if (offset_into_cluster(s, host_cluster_offset)) {
>               qcow2_signal_corruption(bs, true, -1, -1,
>                                       "Cluster allocation offset %#"
> @@ -647,9 +640,11 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
>           abort();
>       }
>   
> +    sc = count_contiguous_subclusters(bs, nb_clusters, sc_index,
> +                                      l2_slice, l2_index);

But the new code is stopping at the first different subcluster type, 
rather than trying to coalesce as many compatible types into one larger 
nb_clusters.  When coupled with patch 19, that factors into my concern 
over whether patch 19 needed to check for INVALID clusters in the 
middle, or whether its fail: label was unreachable.  But it also means 
that you are potentially fragmenting the write in more places (every 
time a subcluster status changes) rather than coalescing similar status, 
the way the old code used to operate.

I don't think the extra fragmentation causes any correctness issues, but 
it may cause performance issues.
Alberto Garcia May 8, 2020, 11:44 a.m. UTC | #2 
On Wed 06 May 2020 07:55:29 PM CEST, Eric Blake wrote:
> On 5/5/20 12:38 PM, Alberto Garcia wrote:
>> The logic of this function remains pretty much the same, except that
>> it uses count_contiguous_subclusters(), which combines the logic of
>> count_contiguous_clusters() / count_contiguous_clusters_unallocated()
>> and checks individual subclusters.
>
> Maybe mention that qcow2_cluster_to_subcluster_type() is now inlined 
> into its lone remaining caller.

Ok.

>> +static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
>> +                                        unsigned sc_index, uint64_t *l2_slice,
>> +                                        int l2_index)
>>   {
>
>> +
>> +    assert(type != QCOW2_SUBCLUSTER_INVALID); /* The caller should check this */
>> +    assert(l2_index + nb_clusters <= s->l2_size);
>> +
>> +    if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
>> +        /* Compressed clusters are always processed one by one */
>> +        return s->subclusters_per_cluster - sc_index;
>
> Should this assert(sc_index == 0)?

No. The documentation of the function says "Compressed clusters are
always processed one by one but for the purpose of this count they are
treated as if they were divided into subclusters of size
s->subcluster_size".

Let's say we have a compressed cluster at offset 0 (size 64k) and we
perform a read request with offset=32k, size=8k.

qcow2_co_preadv_part() calls qcow2_get_host_offset() and asks "How many
bytes up to 8k can we read in one go at offset 32k?".

The offset is 32k so the first subcluster is #16. And this function
(count_contiguous_subclusters()) is asked "how many contiguous
subclusters do we have starting at subcluster #16?" and the answer is
32 - 16 = 16 subclusters.

qcow2_get_host_offset() only needs 8k/2k = 4 subclusters, so it tells
the original caller (qcow2_co_preadv_part()) "you can read all 8k in one
go and the subcluster type is _COMPRESSED".

>>       for (i = 0; i < nb_clusters; i++) {
>> -        uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
>> -        QCow2ClusterType type = qcow2_get_cluster_type(bs, entry);
>> -
>> -        if (type != wanted_type) {
>> -            break;
>> +        l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
>> +        l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
>> +        if (check_offset && expected_offset != (l2_entry & L2E_OFFSET_MASK)) {
>> +            return count;
>> +        }
>> +        for (j = (i == 0) ? sc_index : 0; j < s->subclusters_per_cluster; j++) {
>> +            if (qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, j) != type) {
>> +                return count;
>> +            }
>
> This really is checking that sub-clusters have the exact same type.

Correct.

>> @@ -604,24 +604,17 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
>>               ret = -EIO;
>>               goto fail;
>>           }
>> -        /* Compressed clusters can only be processed one by one */
>> -        c = 1;
>>           *host_offset = l2_entry & L2E_COMPRESSED_OFFSET_SIZE_MASK;
>>           break;
>> -    case QCOW2_CLUSTER_ZERO_PLAIN:
>> -    case QCOW2_CLUSTER_UNALLOCATED:
>> -        /* how many empty clusters ? */
>> -        c = count_contiguous_clusters_unallocated(bs, nb_clusters,
>> -                                                  l2_slice, l2_index, type);
>
> The old code was counting how many contiguous clusters has similar 
> types, coalescing ranges of two different cluster types into one 
> nb_clusters result.

Not really. The old code had three different cases in the switch()
block:

- Compressed clusters: return always 1.

- Unallocated / zero_plain: count the number of clusters of the exact
  same type (one of the parameters was 'wanted_type').

- Normal / zero_alloc: count the number of clusters of type normal or
  zero_alloc that are contiguous on the image file **but stop** if the
  QCOW_OFLAG_ZERO flag changes. In other words, count contiguous
  clusters of the same type.

The new code simply merges all three cases in the same function. It
could be done even without having subclusters.

Plus, the old function was also returning the cluster type, so it had to
be the same one for the whole region.

>>           if (offset_into_cluster(s, host_cluster_offset)) {
>>               qcow2_signal_corruption(bs, true, -1, -1,
>>                                       "Cluster allocation offset %#"
>> @@ -647,9 +640,11 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
>>           abort();
>>       }
>>   
>> +    sc = count_contiguous_subclusters(bs, nb_clusters, sc_index,
>> +                                      l2_slice, l2_index);
>
> But the new code is stopping at the first different subcluster type, 
> rather than trying to coalesce as many compatible types into one larger 
> nb_clusters.  When coupled with patch 19, that factors into my concern 
> over whether patch 19 needed to check for INVALID clusters in the 
> middle, or whether its fail: label was unreachable.  But it also means 
> that you are potentially fragmenting the write in more places (every 
> time a subcluster status changes) rather than coalescing similar status, 
> the way the old code used to operate.

Note that the functions in this patch that you're reviewing are
concerned with read requests, not write requests.

Berto
diff mbox series

Patch

diff --git a/block/qcow2.h b/block/qcow2.h
index 1663b5359c..05e3ef0ece 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -692,29 +692,6 @@  static inline QCow2ClusterType qcow2_get_cluster_type(BlockDriverState *bs,
     }
 }
 
-/*
- * For an image without extended L2 entries, return the
- * QCow2SubclusterType equivalent of a given QCow2ClusterType.
- */
-static inline
-QCow2SubclusterType qcow2_cluster_to_subcluster_type(QCow2ClusterType type)
-{
-    switch (type) {
-    case QCOW2_CLUSTER_COMPRESSED:
-        return QCOW2_SUBCLUSTER_COMPRESSED;
-    case QCOW2_CLUSTER_ZERO_PLAIN:
-        return QCOW2_SUBCLUSTER_ZERO_PLAIN;
-    case QCOW2_CLUSTER_ZERO_ALLOC:
-        return QCOW2_SUBCLUSTER_ZERO_ALLOC;
-    case QCOW2_CLUSTER_NORMAL:
-        return QCOW2_SUBCLUSTER_NORMAL;
-    case QCOW2_CLUSTER_UNALLOCATED:
-        return QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN;
-    default:
-        g_assert_not_reached();
-    }
-}
-
 /*
  * In an image without subsclusters @l2_bitmap is ignored and
  * @sc_index must be 0.
@@ -759,7 +736,20 @@  QCow2SubclusterType qcow2_get_subcluster_type(BlockDriverState *bs,
             g_assert_not_reached();
         }
     } else {
-        return qcow2_cluster_to_subcluster_type(type);
+        switch (type) {
+        case QCOW2_CLUSTER_COMPRESSED:
+            return QCOW2_SUBCLUSTER_COMPRESSED;
+        case QCOW2_CLUSTER_ZERO_PLAIN:
+            return QCOW2_SUBCLUSTER_ZERO_PLAIN;
+        case QCOW2_CLUSTER_ZERO_ALLOC:
+            return QCOW2_SUBCLUSTER_ZERO_ALLOC;
+        case QCOW2_CLUSTER_NORMAL:
+            return QCOW2_SUBCLUSTER_NORMAL;
+        case QCOW2_CLUSTER_UNALLOCATED:
+            return QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN;
+        default:
+            g_assert_not_reached();
+        }
     }
 }
 
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ffcb11edda..f500cbfb8e 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -376,66 +376,58 @@  fail:
 }
 
 /*
- * Checks how many clusters in a given L2 slice are contiguous in the image
- * file. As soon as one of the flags in the bitmask stop_flags changes compared
- * to the first cluster, the search is stopped and the cluster is not counted
- * as contiguous. (This allows it, for example, to stop at the first compressed
- * cluster which may require a different handling)
+ * Return the number of contiguous subclusters of the exact same type
+ * in a given L2 slice, starting from cluster @l2_index, subcluster
+ * @sc_index. Allocated subclusters are required to be contiguous in
+ * the image file.
+ * At most @nb_clusters are checked (note that this means clusters,
+ * not subclusters).
+ * Compressed clusters are always processed one by one but for the
+ * purpose of this count they are treated as if they were divided into
+ * subclusters of size s->subcluster_size.
  */
-static int count_contiguous_clusters(BlockDriverState *bs, int nb_clusters,
-        int cluster_size, uint64_t *l2_slice, int l2_index, uint64_t stop_flags)
+static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
+                                        unsigned sc_index, uint64_t *l2_slice,
+                                        int l2_index)
 {
     BDRVQcow2State *s = bs->opaque;
-    int i;
-    QCow2ClusterType first_cluster_type;
-    uint64_t mask = stop_flags | L2E_OFFSET_MASK | QCOW_OFLAG_COMPRESSED;
-    uint64_t first_entry = get_l2_entry(s, l2_slice, l2_index);
-    uint64_t offset = first_entry & mask;
-
-    first_cluster_type = qcow2_get_cluster_type(bs, first_entry);
-    if (first_cluster_type == QCOW2_CLUSTER_UNALLOCATED) {
-        return 0;
+    int i, j, count = 0;
+    uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index);
+    uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
+    uint64_t expected_offset = l2_entry & L2E_OFFSET_MASK;
+    bool check_offset = true;
+    QCow2SubclusterType type =
+        qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc_index);
+
+    assert(type != QCOW2_SUBCLUSTER_INVALID); /* The caller should check this */
+    assert(l2_index + nb_clusters <= s->l2_size);
+
+    if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
+        /* Compressed clusters are always processed one by one */
+        return s->subclusters_per_cluster - sc_index;
     }
 
-    /* must be allocated */
-    assert(first_cluster_type == QCOW2_CLUSTER_NORMAL ||
-           first_cluster_type == QCOW2_CLUSTER_ZERO_ALLOC);
-
-    for (i = 0; i < nb_clusters; i++) {
-        uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index + i) & mask;
-        if (offset + (uint64_t) i * cluster_size != l2_entry) {
-            break;
-        }
+    if (type == QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN ||
+        type == QCOW2_SUBCLUSTER_ZERO_PLAIN) {
+        check_offset = false;
     }
 
-        return i;
-}
-
-/*
- * Checks how many consecutive unallocated clusters in a given L2
- * slice have the same cluster type.
- */
-static int count_contiguous_clusters_unallocated(BlockDriverState *bs,
-                                                 int nb_clusters,
-                                                 uint64_t *l2_slice,
-                                                 int l2_index,
-                                                 QCow2ClusterType wanted_type)
-{
-    BDRVQcow2State *s = bs->opaque;
-    int i;
-
-    assert(wanted_type == QCOW2_CLUSTER_ZERO_PLAIN ||
-           wanted_type == QCOW2_CLUSTER_UNALLOCATED);
     for (i = 0; i < nb_clusters; i++) {
-        uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
-        QCow2ClusterType type = qcow2_get_cluster_type(bs, entry);
-
-        if (type != wanted_type) {
-            break;
+        l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
+        l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
+        if (check_offset && expected_offset != (l2_entry & L2E_OFFSET_MASK)) {
+            return count;
+        }
+        for (j = (i == 0) ? sc_index : 0; j < s->subclusters_per_cluster; j++) {
+            if (qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, j) != type) {
+                return count;
+            }
+            count++;
         }
+        expected_offset += s->cluster_size;
     }
 
-    return i;
+    return count;
 }
 
 static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
@@ -524,12 +516,12 @@  int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
                           QCow2SubclusterType *subcluster_type)
 {
     BDRVQcow2State *s = bs->opaque;
-    unsigned int l2_index;
-    uint64_t l1_index, l2_offset, *l2_slice, l2_entry;
-    int c;
+    unsigned int l2_index, sc_index;
+    uint64_t l1_index, l2_offset, *l2_slice, l2_entry, l2_bitmap;
+    int sc;
     unsigned int offset_in_cluster;
     uint64_t bytes_available, bytes_needed, nb_clusters;
-    QCow2ClusterType type;
+    QCow2SubclusterType type;
     int ret;
 
     offset_in_cluster = offset_into_cluster(s, offset);
@@ -550,13 +542,13 @@  int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
 
     l1_index = offset_to_l1_index(s, offset);
     if (l1_index >= s->l1_size) {
-        type = QCOW2_CLUSTER_UNALLOCATED;
+        type = QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN;
         goto out;
     }
 
     l2_offset = s->l1_table[l1_index] & L1E_OFFSET_MASK;
     if (!l2_offset) {
-        type = QCOW2_CLUSTER_UNALLOCATED;
+        type = QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN;
         goto out;
     }
 
@@ -577,7 +569,9 @@  int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
     /* find the cluster offset for the given disk offset */
 
     l2_index = offset_to_l2_slice_index(s, offset);
+    sc_index = offset_to_sc_index(s, offset);
     l2_entry = get_l2_entry(s, l2_slice, l2_index);
+    l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
 
     nb_clusters = size_to_clusters(s, bytes_needed);
     /* bytes_needed <= *bytes + offset_in_cluster, both of which are unsigned
@@ -585,9 +579,9 @@  int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
      * true */
     assert(nb_clusters <= INT_MAX);
 
-    type = qcow2_get_cluster_type(bs, l2_entry);
-    if (s->qcow_version < 3 && (type == QCOW2_CLUSTER_ZERO_PLAIN ||
-                                type == QCOW2_CLUSTER_ZERO_ALLOC)) {
+    type = qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc_index);
+    if (s->qcow_version < 3 && (type == QCOW2_SUBCLUSTER_ZERO_PLAIN ||
+                                type == QCOW2_SUBCLUSTER_ZERO_ALLOC)) {
         qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry found"
                                 " in pre-v3 image (L2 offset: %#" PRIx64
                                 ", L2 index: %#x)", l2_offset, l2_index);
@@ -595,7 +589,13 @@  int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
         goto fail;
     }
     switch (type) {
-    case QCOW2_CLUSTER_COMPRESSED:
+    case QCOW2_SUBCLUSTER_INVALID:
+        qcow2_signal_corruption(bs, true, -1, -1, "Invalid cluster entry found "
+                                " (L2 offset: %#" PRIx64 ", L2 index: %#x)",
+                                l2_offset, l2_index);
+        ret = -EIO;
+        goto fail;
+    case QCOW2_SUBCLUSTER_COMPRESSED:
         if (has_data_file(bs)) {
             qcow2_signal_corruption(bs, true, -1, -1, "Compressed cluster "
                                     "entry found in image with external data "
@@ -604,24 +604,17 @@  int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
             ret = -EIO;
             goto fail;
         }
-        /* Compressed clusters can only be processed one by one */
-        c = 1;
         *host_offset = l2_entry & L2E_COMPRESSED_OFFSET_SIZE_MASK;
         break;
-    case QCOW2_CLUSTER_ZERO_PLAIN:
-    case QCOW2_CLUSTER_UNALLOCATED:
-        /* how many empty clusters ? */
-        c = count_contiguous_clusters_unallocated(bs, nb_clusters,
-                                                  l2_slice, l2_index, type);
+    case QCOW2_SUBCLUSTER_ZERO_PLAIN:
+    case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
         *host_offset = 0;
         break;
-    case QCOW2_CLUSTER_ZERO_ALLOC:
-    case QCOW2_CLUSTER_NORMAL: {
+    case QCOW2_SUBCLUSTER_ZERO_ALLOC:
+    case QCOW2_SUBCLUSTER_NORMAL:
+    case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC: {
         uint64_t host_cluster_offset = l2_entry & L2E_OFFSET_MASK;
         *host_offset = host_cluster_offset + offset_in_cluster;
-        /* how many allocated clusters ? */
-        c = count_contiguous_clusters(bs, nb_clusters, s->cluster_size,
-                                      l2_slice, l2_index, QCOW_OFLAG_ZERO);
         if (offset_into_cluster(s, host_cluster_offset)) {
             qcow2_signal_corruption(bs, true, -1, -1,
                                     "Cluster allocation offset %#"
@@ -647,9 +640,11 @@  int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
         abort();
     }
 
+    sc = count_contiguous_subclusters(bs, nb_clusters, sc_index,
+                                      l2_slice, l2_index);
     qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
 
-    bytes_available = (int64_t)c * s->cluster_size;
+    bytes_available = ((int64_t)sc + sc_index) << s->subcluster_bits;
 
 out:
     if (bytes_available > bytes_needed) {
@@ -662,7 +657,7 @@  out:
     assert(bytes_available - offset_in_cluster <= UINT_MAX);
     *bytes = bytes_available - offset_in_cluster;
 
-    *subcluster_type = qcow2_cluster_to_subcluster_type(type);
+    *subcluster_type = type;
 
     return 0;