Re: [Qemu-block] [PATCH v4 7/8] nbd: Implement NBD_INFO_BLOCK_SIZE on server

[Eric Blake]

On 02/22/2017 10:51 AM, Paolo Bonzini wrote:
> 
> 
> On 21/02/2017 03:42, Eric Blake wrote:
>> +    /* Send NBD_INFO_BLOCK_SIZE always, but tweak the minimum size
>> +     * according to whether the client requested it, and according to
>> +     * whether this is OPT_INFO or OPT_GO. */
>> +    /* minimum - 1 for back-compat, or 512 if client is new enough.
>> +     * TODO: consult blk_bs(blk)->request_align? */
>> +    sizes[0] = (opt == NBD_OPT_INFO || blocksize) ? BDRV_SECTOR_SIZE : 1;
>> +    /* preferred - At least 4096, but larger as appropriate. */
>> +    sizes[1] = MAX(blk_get_opt_transfer(exp->blk), 4096);
> 
> Can we just say zero if the preferred transfer size is unknown?

The NBD specification requires a non-zero power-of-2 number if the
server transmits the block size at all; 1 is the ideal number, followed
by whatever actual size we learn from the request_align of the device.


I added blk_get_opt_transfer() in 3/8; maybe I should just respin the
series to also add blk_get_request_align() as well and get rid of the
TODO here.  In other words, my TODO is probably the result of me not
remembering to complete my rebase (since v3 of the series was posted so
many months ago).

> 
> Apart from this, it looks good.

Thanks; I'll get a respin out soon, with R-b added where they make
sense, and with the TODO removed.

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

signature.asc
Description: OpenPGP digital signature