Re: [PATCH v5 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

[Eric Blake]

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 <address@hidden>
> >
> > 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