qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: Eric Blake <eblake@redhat.com>,
	qemu-block@nongnu.org, Alberto Garcia <berto@igalia.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 3/9] block: Add BDRV_REQ_WRITE_UNCHANGED flag
Date: Mon, 30 Apr 2018 10:41:16 +0200	[thread overview]
Message-ID: <20180430084116.GA6488@localhost.localdomain> (raw)
In-Reply-To: <7632df03-6884-7533-f2b1-cdd8f293bea1@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]

Am 28.04.2018 um 13:19 hat Max Reitz geschrieben:
> 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.

I think even a WRITE_UNCHANGED request is supposed to allocate a
currently unmappen block, so "very plain storage" probably has to mean
at least "supports neither backing files nor thin provisioning". Or you
would have to check the current allocation status first...

But I don't see a reason anyway why a WRITE_UNCHANGED request should end
up in the protocol layer.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

  reply	other threads:[~2018-04-30  8:41 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-21 13:29 [Qemu-devel] [PATCH v2 0/9] block: Add COR filter driver Max Reitz
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 1/9] " Max Reitz
2018-04-24 15:08   ` Alberto Garcia
2018-04-25 11:18     ` Max Reitz
2018-04-25 11:35       ` Alberto Garcia
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 2/9] block: BLK_PERM_WRITE includes ..._UNCHANGED Max Reitz
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 3/9] block: Add BDRV_REQ_WRITE_UNCHANGED flag Max Reitz
2018-04-25 14:33   ` Eric Blake
2018-04-25 15:08     ` Max Reitz
2018-04-26  2:12       ` Eric Blake
2018-04-26  8:58         ` Kevin Wolf
2018-04-28 11:19         ` Max Reitz
2018-04-30  8:41           ` Kevin Wolf [this message]
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 4/9] block: Set BDRV_REQ_WRITE_UNCHANGED for COR writes Max Reitz
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 5/9] block/quorum: Support BDRV_REQ_WRITE_UNCHANGED Max Reitz
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 6/9] block: Support BDRV_REQ_WRITE_UNCHANGED in filters Max Reitz
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 7/9] iotests: Clean up wrap image in 197 Max Reitz
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 8/9] iotests: Copy 197 for COR filter driver Max Reitz
2018-04-24 15:17   ` Alberto Garcia
2018-04-21 13:29 ` [Qemu-devel] [PATCH v2 9/9] iotests: Add test for COR across nodes Max Reitz
2018-04-24 16:50   ` Kevin Wolf
2018-04-24 16:51 ` [Qemu-devel] [PATCH v2 0/9] block: Add COR filter driver Kevin Wolf
2018-04-25 12:18 ` Max Reitz
2018-04-25 14:36   ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180430084116.GA6488@localhost.localdomain \
    --to=kwolf@redhat.com \
    --cc=berto@igalia.com \
    --cc=eblake@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).