From c6d619cc12813acb2b1198f28cf2256ea9a30107 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 18 Jan 2019 14:40:36 +0100 Subject: qcow2: Don't assume 0 is an invalid cluster offset The cluster allocation code uses 0 as an invalid offset that is used in case of errors or as "offset not yet determined". With external data files, a host cluster offset of 0 becomes valid, though. Define a constant INV_OFFSET (which is not cluster aligned and will therefore never be a valid offset) that can be used for such purposes. This removes the additional host_offset == 0 check that commit ff52aab2df5 introduced; the confusion between an invalid offset and (erroneous) allocation at offset 0 is removed with this change. Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c | 59 +++++++++++++++++++++++---------------------------- block/qcow2.h | 2 ++ 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 660161b..1d09f54 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1109,9 +1109,9 @@ static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset, /* * Checks how many already allocated clusters that don't require a copy on - * write there are at the given guest_offset (up to *bytes). If - * *host_offset is not zero, only physically contiguous clusters beginning at - * this host offset are counted. + * write there are at the given guest_offset (up to *bytes). If *host_offset is + * not INV_OFFSET, only physically contiguous clusters beginning at this host + * offset are counted. * * Note that guest_offset may not be cluster aligned. In this case, the * returned *host_offset points to exact byte referenced by guest_offset and @@ -1143,8 +1143,8 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset, trace_qcow2_handle_copied(qemu_coroutine_self(), guest_offset, *host_offset, *bytes); - assert(*host_offset == 0 || offset_into_cluster(s, guest_offset) - == offset_into_cluster(s, *host_offset)); + assert(*host_offset == INV_OFFSET || offset_into_cluster(s, guest_offset) + == offset_into_cluster(s, *host_offset)); /* * Calculate the number of clusters to look for. We stop at L2 slice @@ -1182,7 +1182,7 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset, goto out; } - if (*host_offset != 0 && !offset_matches) { + if (*host_offset != INV_OFFSET && !offset_matches) { *bytes = 0; ret = 0; goto out; @@ -1225,10 +1225,10 @@ out: * contain the number of clusters that have been allocated and are contiguous * in the image file. * - * If *host_offset is non-zero, it specifies the offset in the image file at - * which the new clusters must start. *nb_clusters can be 0 on return in this - * case if the cluster at host_offset is already in use. If *host_offset is - * zero, the clusters can be allocated anywhere in the image file. + * If *host_offset is not INV_OFFSET, it specifies the offset in the image file + * at which the new clusters must start. *nb_clusters can be 0 on return in + * this case if the cluster at host_offset is already in use. If *host_offset + * is INV_OFFSET, the clusters can be allocated anywhere in the image file. * * *host_offset is updated to contain the offset into the image file at which * the first allocated cluster starts. @@ -1247,7 +1247,7 @@ static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset, /* Allocate new clusters */ trace_qcow2_cluster_alloc_phys(qemu_coroutine_self()); - if (*host_offset == 0) { + if (*host_offset == INV_OFFSET) { int64_t cluster_offset = qcow2_alloc_clusters(bs, *nb_clusters * s->cluster_size); if (cluster_offset < 0) { @@ -1267,8 +1267,8 @@ static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset, /* * Allocates new clusters for an area that either is yet unallocated or needs a - * copy on write. If *host_offset is non-zero, clusters are only allocated if - * the new allocation can match the specified host offset. + * copy on write. If *host_offset is not INV_OFFSET, clusters are only + * allocated if the new allocation can match the specified host offset. * * Note that guest_offset may not be cluster aligned. In this case, the * returned *host_offset points to exact byte referenced by guest_offset and @@ -1296,7 +1296,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset, int ret; bool keep_old_clusters = false; - uint64_t alloc_cluster_offset = 0; + uint64_t alloc_cluster_offset = INV_OFFSET; trace_qcow2_handle_alloc(qemu_coroutine_self(), guest_offset, *host_offset, *bytes); @@ -1335,7 +1335,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset, if (qcow2_get_cluster_type(bs, entry) == QCOW2_CLUSTER_ZERO_ALLOC && (entry & QCOW_OFLAG_COPIED) && - (!*host_offset || + (*host_offset == INV_OFFSET || start_of_cluster(s, *host_offset) == (entry & L2E_OFFSET_MASK))) { int preallocated_nb_clusters; @@ -1367,9 +1367,10 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset, qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice); - if (!alloc_cluster_offset) { + if (alloc_cluster_offset == INV_OFFSET) { /* Allocate, if necessary at a given offset in the image file */ - alloc_cluster_offset = start_of_cluster(s, *host_offset); + alloc_cluster_offset = *host_offset == INV_OFFSET ? INV_OFFSET : + start_of_cluster(s, *host_offset); ret = do_alloc_cluster_offset(bs, guest_offset, &alloc_cluster_offset, &nb_clusters); if (ret < 0) { @@ -1382,16 +1383,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset, return 0; } - /* !*host_offset would overwrite the image header and is reserved for - * "no host offset preferred". If 0 was a valid host offset, it'd - * trigger the following overlap check; do that now to avoid having an - * invalid value in *host_offset. */ - if (!alloc_cluster_offset) { - ret = qcow2_pre_write_overlap_check(bs, 0, alloc_cluster_offset, - nb_clusters * s->cluster_size); - assert(ret < 0); - goto fail; - } + assert(alloc_cluster_offset != INV_OFFSET); } /* @@ -1483,14 +1475,14 @@ int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset, again: start = offset; remaining = *bytes; - cluster_offset = 0; - *host_offset = 0; + cluster_offset = INV_OFFSET; + *host_offset = INV_OFFSET; cur_bytes = 0; *m = NULL; while (true) { - if (!*host_offset) { + if (*host_offset == INV_OFFSET && cluster_offset != INV_OFFSET) { *host_offset = start_of_cluster(s, cluster_offset); } @@ -1498,7 +1490,10 @@ again: start += cur_bytes; remaining -= cur_bytes; - cluster_offset += cur_bytes; + + if (cluster_offset != INV_OFFSET) { + cluster_offset += cur_bytes; + } if (remaining == 0) { break; @@ -1570,7 +1565,7 @@ again: *bytes -= remaining; assert(*bytes > 0); - assert(*host_offset != 0); + assert(*host_offset != INV_OFFSET); return 0; } diff --git a/block/qcow2.h b/block/qcow2.h index 8fe2d55..e3bf322 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -463,6 +463,8 @@ typedef enum QCow2MetadataOverlap { #define REFT_OFFSET_MASK 0xfffffffffffffe00ULL +#define INV_OFFSET (-1ULL) + static inline bool has_data_file(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque; -- cgit v1.1