Re: [Qemu-block] [Qemu-devel] [RFC PATCH 18/18] nbd: Implement NBD_CMD_WRITE_ZEROES on client

[Eric Blake]

On 04/09/2016 04:57 AM, Alex Bligh wrote:
> 
> On 8 Apr 2016, at 23:05, Eric Blake <address@hidden> 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

signature.asc
Description: OpenPGP digital signature