Re: [Qemu-devel] [PATCH for 2.7 1/1] qcow2: improve qcow2_co_write_zeroes()

[Kevin Wolf]

Am 25.04.2016 um 12:20 hat Denis V. Lunev geschrieben:
> On 04/25/2016 12:05 PM, Kevin Wolf wrote:
> >Am 23.04.2016 um 14:05 hat Denis V. Lunev geschrieben:
> >>Unfortunately Linux kernel could send non-aligned requests to qemu-nbd
> >>if the caller is using O_DIRECT and does not align in-memory data to
> >>page. Thus qemu-nbd will call block layer with non-aligned requests.
> >>
> >>qcow2_co_write_zeroes forcibly asks the caller to supply block-aligned
> >>data. In the other case it rejects with ENOTSUP which is properly
> >>handled on the upper level. The problem is that this grows the image.
> >>
> >>This could be optimized a bit:
> >>- particular request could be split to block aligned part and head/tail,
> >>   which could be handled separately
> >In fact, this is what bdrv_co_do_write_zeroes() is already supposed to
> >do. qcow2 exposes its cluster size as bs->bl.write_zeroes_alignment, so
> >block/io.c should split the request in three.
> >
> >If you see something different happening, we may have a bug there.
> >
> Pls look to the commit
> 
> commit 459b4e66129d091a11e9886ecc15a8bf9f7f3d92
> Author: Denis V. Lunev <address@hidden>
> Date:   Tue May 12 17:30:56 2015 +0300
> 
>     block: align bounce buffers to page
> 
> The situation is exactly like the described there. The user
> of the /dev/nbd0 writes with O_DIRECT and has unaligned
> to page buffers. Thus real operations on qemu-nbd
> layer becomes unaligned to block size.

I don't understand the connection to this patch. Unaligned buffers on
the NBD client shouldn't even be visible in the server, unless they
already result in the client requesting different things. If so, what is
the difference in the NBD requests? And can we reproduce the same
locally with qemu-io and no NBD involved?

> Thus bdrv_co_do_write_zeroes is helpless here unfortunately.

How can qcow2 fix something that bdrv_co_do_write_zeroes() can't
possibly fix? In particular, why does splitting the request in head,
tail and aligned part help when done by qcow2, but the same thing
doesn't help when done by bdrv_co_do_write_zeroes()?

I'd actually be interested in both parts of the answer, because I'm not
sure how _memory_ alignment on the client can possibly be fixed in
qcow2; but if it's about _disk_ alignment, I don't understand why it
can't be fixed in bdrv_co_do_write_zeroes().

> >>- writes could be omitted when we do know that the image already contains
> >>   zeroes at the offsets being written
> >I don't think this is a valid shortcut. The semantics of a write_zeroes
> >operation is that the zeros (literal or as flags) are stored in this
> >layer and that the backing file isn't involved any more for the given
> >sectors. For example, a streaming operation or qemu-img rebase may
> >involve write_zeroes operations, and relying on the backing file would
> >cause corruption there (because the whole point of the operation is that
> >the backing file can be removed).
> this is not a problem. The block will be abscent and thus it will be
> read as zeroes.

Removing a backing file doesn't mean that there won't still be another
backing file. You may have only removed one node in the backing file
chain, or in the case of rebase, you switch to another backing file.

Kevin