Am 22.04.2020 um 17:33 hat Eric Blake geschrieben:
> On 4/22/20 10:21 AM, Kevin Wolf wrote:
> > If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling
> > qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't
> > undo any previous preallocation, but just adds the zero flag to all
> > relevant L2 entries. If an external data file is in use, a write_zeroes
> > request to the data file is made instead.
> >
> > Signed-off-by: Kevin Wolf <kw...@redhat.com>
> > ---
> > block/qcow2.c | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
>
> > @@ -4214,6 +4215,35 @@ static int coroutine_fn
> > qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> > g_assert_not_reached();
> > }
> > + if ((flags & BDRV_REQ_ZERO_WRITE) && offset > old_length) {
> > + uint64_t zero_start = QEMU_ALIGN_UP(old_length, s->cluster_size);
> > + uint64_t zero_end = QEMU_ALIGN_UP(offset, s->cluster_size);
>
> This rounds up beyond the new size...
>
> > +
> > + /* Use zero clusters as much as we can */
> > + ret = qcow2_cluster_zeroize(bs, zero_start, zero_end - zero_start,
> > 0);
>
> and then requests that the extra be zeroed. Does that always work, even
> when it results in pdrv_co_pwrite_zeroes beyond the end of s->data_file?
You mean the data_file_is_raw() path in qcow2_cluster_zeroize()? It's
currently not a code path that is run because we only set
BDRV_REQ_ZERO_WRITE for truncate if the image has a backing file, and
data_file_is_raw() doesn't work with backing files.
But hypothetically, if someone called truncate with BDRV_REQ_ZERO_WRITE
for such a file, I think it would fail.
> If so,
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
> otherwise, you may have to treat the tail specially, the same way you
> treated an unaligned head.
Actually, do I even need to round the tail?
/* Caller must pass aligned values, except at image end */
assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
end_offset == bs->total_sectors << BDRV_SECTOR_BITS);
So qcow2_cluster_zeroize() seems to accept the unaligned tail. It would
still set the zero flag for the partial last cluster and for the
external data file, bdrv_co_pwrite_zeroes() would have the correct size.
Kevin
