qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Kevin Wolf <kwolf@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"open list:Block layer core" <qemu-block@nongnu.org>
Subject: Re: [Qemu-devel] [RFC PATCH 18/18] nbd: Implement NBD_CMD_WRITE_ZEROES on client
Date: Sat, 9 Apr 2016 17:17:23 -0600	[thread overview]
Message-ID: <57098D83.1080503@redhat.com> (raw)
In-Reply-To: <D6B96403-5EF5-49D5-B734-55D29EC27DCB@alex.org.uk>

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

On 04/09/2016 04:57 AM, Alex Bligh wrote:
> 
> On 8 Apr 2016, at 23:05, Eric Blake <eblake@redhat.com> wrote:
> 
>> RFC because there is still discussion on the NBD list about
>> adding an NBD_OPT_ to let the client suggest server defaults
>> related to scanning for zeroes during NBD_CMD_WRITE, which may
>> tweak this patch.
>>
>> Upstream NBD protocol recently added the ability to efficiently
>> write zeroes without having to send the zeroes over the wire,
>> along with a flag to control whether the client wants a hole.
>>
>> The generic block code takes care of falling back to the obvious
>> write lots of zeroes if we return -ENOTSUP because the server
>> does not have WRITE_ZEROES.
>>

>> +    ret = nbd_client_co_write_zeroes(bs, sector_num, nb_sectors, &flags);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    /* The flag wasn't sent to the server, so we need to emulate it with an
>> +     * explicit flush */
> 
> Surely you only need to do this is the flag wasn't sent to the server,
> i.e. if !(client->nbdflags & NBD_FLAG_SEND_FUA)
> 
> If you've sent a FUA request, no need to flush the whole thing.
> 
> nbd_co_writev_flags seems to have the same issue, which is where I guess
> you got that from.

No, the code is correct.  In both functions, the logic is that if the
lower-level knows that the server respects FUA, then it clears the flag
before returning (flags is passed by reference, not value).  Then at
this higher level, if FUA is still set, the server is too old, so we do
a fallback flush to get the same semantics for the write in question,
but at higher cost of a full flush.

> 
>> +    if (flags & BDRV_REQ_FUA) {
>> +        ret = nbd_client_co_flush(bs);
>> +    }
>> +

It's also this higher level that knows how to fall back to NBD_CMD_WRITE
if the lower-level returned -ENOTSUP because the server doesn't support
WRITE_ZEROES.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  parent reply	other threads:[~2016-04-09 23:17 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-08 22:05 [Qemu-devel] [RFC PATCH 00/18] NBD protocol additions Eric Blake
2016-04-08 22:05 ` [Qemu-devel] [PATCH 01/18] nbd: Don't kill server on client that doesn't request TLS Eric Blake
2016-04-09 10:28   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 02/18] nbd: Don't fail handshake on NBD_OPT_LIST descriptions Eric Blake
2016-04-09 10:30   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 03/18] nbd: More debug typo fixes, use correct formats Eric Blake
2016-04-09 10:30   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 04/18] nbd: Detect servers that send unexpected error values Eric Blake
2016-04-09 10:31   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 05/18] nbd: Reject unknown request flags Eric Blake
2016-04-09 10:32   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 06/18] nbd: Avoid magic number for NBD max name size Eric Blake
2016-04-09 10:35   ` Alex Bligh
2016-04-09 22:07     ` Eric Blake
2016-04-08 22:05 ` [Qemu-devel] [PATCH 07/18] nbd: Treat flags vs. command type as separate fields Eric Blake
2016-04-09 10:37   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 08/18] nbd: Limit nbdflags to 16 bits Eric Blake
2016-04-09 10:37   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 09/18] nbd: Share common reply-sending code in server Eric Blake
2016-04-09 10:38   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 10/18] nbd: Share common option-sending code in client Eric Blake
2016-04-09 10:38   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 11/18] nbd: Let client skip portions of server reply Eric Blake
2016-04-09 10:39   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 12/18] nbd: Less allocation during NBD_OPT_LIST Eric Blake
2016-04-09 10:41   ` Alex Bligh
2016-04-09 22:24     ` Eric Blake
2016-04-08 22:05 ` [Qemu-devel] [PATCH 13/18] nbd: Support shorter handshake Eric Blake
2016-04-09 10:42   ` Alex Bligh
2016-04-09 22:27     ` Eric Blake
2016-04-08 22:05 ` [Qemu-devel] [PATCH 14/18] nbd: Implement NBD_OPT_GO on client Eric Blake
2016-04-09 10:47   ` Alex Bligh
2016-04-09 22:38     ` Eric Blake
2016-04-08 22:05 ` [Qemu-devel] [PATCH 15/18] nbd: Implement NBD_OPT_GO on server Eric Blake
2016-04-09 10:48   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [PATCH 16/18] nbd: Support NBD_CMD_CLOSE Eric Blake
2016-04-09 10:50   ` Alex Bligh
2016-04-09 23:12     ` Eric Blake
2016-04-10  5:28       ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [RFC PATCH 17/18] nbd: Implement NBD_CMD_WRITE_ZEROES on server Eric Blake
2016-04-09  9:39   ` Pavel Borzenkov
2016-04-09 10:54   ` Alex Bligh
2016-04-08 22:05 ` [Qemu-devel] [RFC PATCH 18/18] nbd: Implement NBD_CMD_WRITE_ZEROES on client Eric Blake
2016-04-09 10:57   ` Alex Bligh
2016-04-09 11:52     ` Pavel Borzenkov
2016-04-09 23:17     ` Eric Blake [this message]
2016-04-10  5:27       ` Alex Bligh
2016-04-09 10:21 ` [Qemu-devel] [Nbd] [RFC PATCH 00/18] NBD protocol additions Wouter Verhelst

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=57098D83.1080503@redhat.com \
    --to=eblake@redhat.com \
    --cc=alex@alex.org.uk \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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).