On 4/22/20 10:58 AM, Kevin Wolf wrote:
@@ -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.
Good point.
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.
Then I'm in favor of NOT rounding the tail. That's an easy enough
change and we've now justified that it does what we want, so R-b stands
with that one-line tweak.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
