Re: [Qemu-block] [PATCH v9 01/13] qcow2: Unallocate unmapped zero clusters if no backing file

[Kevin Wolf]

Am 11.04.2017 um 03:17 hat Eric Blake geschrieben:
> 'qemu-img map' already coalesces information about unallocated
> clusters (those with status 0) and pure zero clusters (those
> with status BDRV_BLOCK_ZERO and no offset).  Furthermore, all
> qcow2 images with no backing file already report all unallocated
> clusters (in the preallocation sense of clusters with no offset)
> as BDRV_BLOCK_ZERO, regardless of whether the QCOW_OFLAG_ZERO was
> set in that L2 entry (QCOW_OFLAG_ZERO also implies a return of
> BDRV_BLOCK_ALLOCATED, but we intentionally do not expose that bit
> to external users), thanks to generic block layer code in
> bdrv_co_get_block_status().
> 
> So, for an image with no backing file, having bdrv_pwrite_zeroes
> mark clusters as unallocated (defer to backing file) rather than
> reads-as-zero (regardless of backing file) makes no difference
> to normal behavior, but may potentially allow for fewer writes to
> the L2 table of a freshly-created image where the L2 table is
> initially written to all-zeroes (although I don't actually know
> if we skip an L2 update and flush when re-writing the same
> contents as already present).

I don't get this. Allocating a cluster always involves an L2 update, no
matter whether it was previously unallocated or a zero cluster.

> Furthermore, this matches the behavior of discard_single_l2(), in
> favoring an unallocated cluster over a zero cluster when full
> discard is requested.

The only use for "full discard" is qcow2_make_empty(). It explicitly
requests that the backing file becomes visible again. This is a
completely different case.

In other words, in order to stay consistent between discard and
write_zeroes from a guest POV, we need to leave this code alone.

> Meanwhile, version 2 qcow2 files (compat=0.10) lack support for an
> explicit zero cluster.  This minor tweak therefore allows us to turn
> write zeroes with unmap into an actual unallocation on those files,
> where they used to return -ENOTSUP and cause an allocation due to
> the fallback to explicitly written zeroes.

Okay, this is true.

But I doubt that making write_zeroes more efficient on v2 images without
a backing file is really worth any extra complexity at this point...

Kevin