On 27.04.2017 03:46, Eric Blake wrote:
> We were throwing away the preallocation information associated with
> zero clusters. But we should be matching the well-defined semantics
> in bdrv_get_block_status(), where (BDRV_BLOCK_ZERO |
> BDRV_BLOCK_OFFSET_VALID) informs the user which offset is reserved,
> while still reminding the user that reading from that offset is
> likely to read garbage.
>
> Making this change lets us see which portions of an image are zero
> but preallocated, when using qemu-img map --output=json. The
> --output=human side intentionally ignores all zero clusters, whether
> or not they are preallocated.
>
> The fact that there is no change to qemu-iotests './check -qcow2'
> merely means that we aren't yet testing this aspect of qemu-img;
> a later patch will add a test.
>
> Signed-off-by: Eric Blake
>
> ---
> v10: new patch
> ---
> block/qcow2-cluster.c | 32 +++-
> 1 file changed, 27 insertions(+), 5 deletions(-)
I'd propose you split the qcow2 changes off of this series.
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 100398c..d1063df 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -328,6 +328,10 @@ static int count_contiguous_clusters(int nb_clusters,
> int cluster_size,
> return i;
> }
>
> +/*
> + * Checks how many consecutive clusters in a given L2 table have the same
> + * cluster type with no corresponding allocation.
> + */
> static int count_contiguous_clusters_by_type(int nb_clusters,
> uint64_t *l2_table,
> int wanted_type)
> @@ -335,9 +339,10 @@ static int count_contiguous_clusters_by_type(int
> nb_clusters,
> int i;
>
> for (i = 0; i < nb_clusters; i++) {
> -int type = qcow2_get_cluster_type(be64_to_cpu(l2_table[i]));
> +uint64_t entry = be64_to_cpu(l2_table[i]);
> +int type = qcow2_get_cluster_type(entry);
>
> -if (type != wanted_type) {
> +if (type != wanted_type || entry & L2E_OFFSET_MASK) {
This works only for wanted_type \in \{ ZERO, UNALLOCATED \} -- which is
what the comment you added describes, but this may still warrant a
renaming of this function (count_contiguous_unallocated_clusters?) and
probably an assertion that wanted_type is ZERO or UNALLOCATED.
> break;
> }
> }
> @@ -559,9 +564,26 @@ int qcow2_get_cluster_offset(BlockDriverState *bs,
> uint64_t offset,
> ret = -EIO;
> goto fail;
> }
> -c = count_contiguous_clusters_by_type(nb_clusters,
> &l2_table[l2_index],
> - QCOW2_CLUSTER_ZERO);
> -*cluster_offset = 0;
> +/* Distinguish between pure zero clusters and pre-allocated ones */
> +if (*cluster_offset & L2E_OFFSET_MASK) {
> +c = count_contiguous_clusters(nb_clusters, s->cluster_size,
> + &l2_table[l2_index],
> QCOW_OFLAG_ZERO);
You should probably switch this patch and the next one -- or I just send
my patches myself and you base your series on top of it...
Because before the next patch, this happens:
$ ./qemu-img create -f qcow2 foo.qcow2 64M
Formatting 'foo.qcow2', fmt=qcow2 size=67108864 encryption=off
cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ ./qemu-io -c 'write 0 64M' -c 'write -z 0 64M' foo.qcow2
wrote 67108864/67108864 bytes at offset 0
64 MiB, 1 ops; 0.1846 sec (346.590 MiB/sec and 5.4155 ops/sec)
wrote 67108864/67108864 bytes at offset 0
64 MiB, 1 ops; 0.0004 sec (127.551 GiB/sec and 2040.8163 ops/sec)
$ ./qemu-img map foo.qcow2
Offset Length Mapped to File
qemu-img: block/qcow2-cluster.c:319: count_contiguous_clusters:
Assertion `qcow2_get_cluster_type(first_entry) == QCOW2_CLUSTER_NORMAL'
failed.
[1]4970 abort (core dumped) ./qemu-img map foo.qcow2
Max
> +*cluster_offset &= L2E_OFFSET_MASK;
> +if (offset_into_cluster(s, *cluster_offset)) {
> +qcow2_signal_corruption(bs, true, -1, -1,
> +"Preallocated zero cluster offset %#"
> +PRIx64 " unaligned (L2 offset: %#"
> +PRIx64 ", L2 index: %#x)",
> +*cluster_offset, l2_offset,
> l2_index);
> +ret = -EIO;
> +goto fail;
> +}
> +} else {
> +c = count_contiguous_clusters_by_type(nb_clusters,
> + &l2_table[l2_index],
> + QCOW2_CLUSTER_ZERO);
> +*cluster_offset = 0;
> +}
> break;
> case QCOW2_CLUSTER_UNALLOCATED:
> /* how many empty clusters ? */
>
signature.asc
Description: OpenPGP digital signature