[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 18/30] qcow2: Add subcluster support to qcow2_get_host_off
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v4 18/30] qcow2: Add subcluster support to qcow2_get_host_offset() |
Date: |
Wed, 22 Apr 2020 11:07:30 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 |
17.03.2020 21:16, 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.
Signed-off-by: Alberto Garcia <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
[..]
+static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
+ unsigned sc_index, uint64_t *l2_slice,
+ int l2_index)
{
BDRVQcow2State *s = bs->opaque;
preexist, but, worth asserting that nb_clusters are all in this l2_slice?
[..]
+ 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)
{
+ goto out;
why not just return count from here? And then you don't need goto at all. Hmm,
may be out: code will be extended in further patches..
+ }
+ count++;
}
+ expected_offset += s->cluster_size;
}
- return i;
+out:
+ return count;
}
[..]
@@ -607,21 +607,20 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t
offset,
goto fail;
}
/* Compressed clusters can only be processed one by one */
- c = 1;
+ sc = s->subclusters_per_cluster - sc_index;
should not we assert here that sc_index == 0? Otherwise the caller definitely
doing something wrong.
*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:
+ sc = count_contiguous_subclusters(bs, nb_clusters, sc_index,
+ l2_slice, l2_index);
*host_offset = 0;
break;
- case QCOW2_CLUSTER_ZERO_ALLOC:
- case QCOW2_CLUSTER_NORMAL:
- /* how many allocated clusters ? */
- c = count_contiguous_clusters(bs, nb_clusters, s->cluster_size,
- l2_slice, l2_index, QCOW_OFLAG_ZERO);
+ case QCOW2_SUBCLUSTER_ZERO_ALLOC:
+ case QCOW2_SUBCLUSTER_NORMAL:
+ case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC:
+ sc = count_contiguous_subclusters(bs, nb_clusters, sc_index,
+ l2_slice, l2_index);
*host_offset = l2_entry & L2E_OFFSET_MASK;
if (offset_into_cluster(s, *host_offset)) {
Hmm, you may move "sc = count_contiguous_subclusters" to be after the
switch-block, as it is universal now. And keep only offset calculation and error checking
in the switch-block.
qcow2_signal_corruption(bs, true, -1, -1,
@@ -651,7 +650,7 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t
offset,
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) {
@@ -664,7 +663,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;
--
Best regards,
Vladimir