Re: [Qemu-block] [PATCH v8 8/9] qcow2: skip writing zero buffers to empty COW areas

[Alberto Garcia]

On Mon 12 Mar 2018 11:16:57 AM CET, Anton Nefedov wrote:
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2584,6 +2584,8 @@
>  #
>  # @cor_write: a write due to copy-on-read (since 2.11)
>  #
> +# @cluster_alloc_space: an allocation of a cluster file space (since 2.12)

That doesn't sound like correct English to me.

> +static bool is_unallocated(BlockDriverState *bs, int64_t offset, int64_t 
> bytes)
> +{
> +    int64_t nr;
> +    return !bytes ||
> +        (!bdrv_is_allocated_above(bs, NULL, offset, bytes, &nr) && nr == 
> bytes);
> +}
> +

Is this really more efficient than the previous is_zero() call ?

It seems that in both cases the code ends up calling
bdrv_common_block_status_above().

> +static bool is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
> +{
> +    /* content with false negatives, giving is_allocated() is faster than
> +     * a proper zero detection with possible actual image seeks, which is
> +     * performed by is_zero() */

It took me a bit to understand this sentence, maybe some native English
speaker can suggest an alternate wording?

  "is_allocated() is not as accurate as is_zero() and can give us some
   false negatives, but it is much more efficient so let's use it here"

> +    return is_unallocated(bs, m->offset + m->cow_start.offset,
> +                          m->cow_start.nb_bytes) &&
> +           is_unallocated(bs, m->offset + m->cow_end.offset,
> +                          m->cow_end.nb_bytes);
> +}

Berto