Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/9] block: Add BDRV_REQ_WRITE_UNCHANGED flag

[Max Reitz]

On 2018-04-26 04:12, Eric Blake wrote:
> On 04/25/2018 10:08 AM, Max Reitz wrote:
> 
>>
>>> Also, that does raise the question of whether you have more work to
>>> support write-zero requests with WRITE_UNCHANGED (which indeed sounds
>>> like something plausible to support).
>>
>> I'm afraid I don't quite understand the question.
>> BDRV_REQ_WRITE_UNCHANGED support is usually rather simple because as
>> said above it is only needed by drivers that rely on their parent to
>> request the permissions, i.e. filter drivers.  Since filter drivers just
>> forward the requests, all they have to do is retain the
>> BDRV_REQ_WRITE_UNCHANGED flag, be it a zero write or a normal write.
> 
> I'm trying to figure out if BDRV_REQ_WRITE_UNCHANGED makes sense for
> bdrv_co_pwrite_zeroes as well as bdrv_co_pwrite. I think the answer is
> yes (if we know the guest already reads zeroes, but need to write to the
> protocol layer anyways because of a commit operation, then mixing both
> BDRV_REQ_WRITE_UNCHANGED and BDRV_REQ_ZERO_WRITE to the block layer
> makes sense, and supported_zero_flags should indeed pass
> BDRV_REQ_WRITE_UNCHANGED on to a driver.

Well, why not.

>> It would be more complicated for format drivers, because they would have
>> to verify that the incoming unchanged write actually ends up as an
>> unchanged write in their file.  But we have already recognized that that
>> would be too much to ask and that format drivers may want to generally
>> just write anything to their child if it's writable (even regardless of
>> whether the grandparent issues writes to the format driver node), so
>> they always grab a WRITE permission on their file if possible.
>> Therefore, they do not have to support this request flag.
> 
> So qcow2 doesn't have to support the flag, but file-posix.c might want
> to.  Or are you saying that only filter drivers need to advertise
> support for the flag?

It might make sense for file-posix, but when you think further, it
wouldn't do anything in practice.

First, if a protocol driver receives WRITE_UNCHANGED, there are two
things it can do: If it knows that it only has very plain storage, it
can just ignore the request because it won't do anything.

If a protocol driver supports backing files (gluster does, I think), it
cannot ignore them because just like qcow2 it needs to now make sure
that the data ends up in the top layer.  So if the protocol driver can
draw any benefits from advertising to the protocol that this is an
unchanging write (or that it just wants to bring data from a backing
file up to the top level, which it can infer from the WRITE_UNCHANGED
flag), then it would make sense for it to support the flag.

(Good point to add to the documentation.)

But from qemu's perspective, this is not required.  The protocol driver
will just be less efficient if it submits normal writes (and it can do
that, because qemu will refuse to grant WRITE_UNCHANGED on read-only
nodes, so the protocol driver must have write access).

OTOH, filter drivers (usually) *must* support this flag because they are
caught up in qemu's permission system.  They do not submit the writes to
some protocol, but to the block layer, and the block layer wants you to
have the proper permissions for that -- and that's not a simple RW/RO
decision, but we have read/write-unchanged in between.  So if a filter
driver that only forwards the permissions from its parent may only do
the very same requests the parent does -- and if the parent does a
WRITE_UNCHANGED request, the filter driver must do the same.

Max

signature.asc
Description: OpenPGP digital signature