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

[Max Reitz]

On 2018-04-25 16:33, Eric Blake wrote:
> On 04/21/2018 08:29 AM, Max Reitz wrote:
>> This flag signifies that a write request will not change the visible
>> disk content.  With this flag set, it is sufficient to have the
>> BLK_PERM_WRITE_UNCHANGED permission instead of BLK_PERM_WRITE.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> Reviewed-by: Stefan Hajnoczi <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
>>  include/block/block.h | 6 +++++-
>>  block/io.c            | 6 +++++-
>>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> Since patch 5 adds an instance of a driver setting supported_write_flags
> = BDRV_REQ_WRITE_UNCHANGED, I think this patch should tweak the
> documentation of supported_write_flags (and supported_zero_flags?) in
> block_int.h to mention that drivers can opt-in to supporting this
> particular flag (rather than it being handled completely at the block
> layer).

Ah, right, I just didn't know we had a list of supported flags there.

And the flag isn't handled by the block layer if drivers don't support
it -- it just is cleared then and thus treated like a normal write.

Maybe I should make a note that drivers should support the flag when
they support writes to their children, but don't explicitly request a
WRITE permission (so they rely on their parent to request the proper
permission, which might be just WRITE_UNCHANGED and not the full WRITE).

I'll send a patch.

> 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.

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.

Max

signature.asc
Description: OpenPGP digital signature