[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] [PATCH/RFCv4] Remove NBD_OPT_BLOCK_SIZE; add specific requests to NBD_OPT_INFO



Wouter,

On 28 Apr 2016, at 09:06, Wouter Verhelst <w@...112...> wrote:

> On Tue, Apr 26, 2016 at 08:36:10PM +0100, Alex Bligh wrote:
>> Allow a client to requests specific bits of information using
>> NBD_OPT_INFO and thereby signal to the server that it supports them.
>> This makes it easier for the server to know whether the information
>> it is providing will be used / respected.
>> 
>> Now a server can determine whether a client supports block sizes by
>> the fact it uses NBD_OPT_INFO or NBD_OPT_GO with an NBD_INFO_BLOCK_SIZE,
>> request as these are part of the same extension, so there is no
>> need for NBD_OPT_BLOCK_SIZE.
> 
> I think we're close, except for this:
> 
> - I'd like to change things so that NBD_OPT_INFO doesn't commit a client
>  to following block sizes. The rationale for this is bifold:
>  - First and foremost, it will simplify the server, since it doesn't
>    need to retain as much state (it can simply look at the NBD_OPT_GO
>    command, and that one alone, to decide on whether it can do the
>    optimized O_DIRECT path, rather than having to do a lot of global
>    variables and ifs and buts etc).

>  - Second (and somewhat less likely), it allows a client to see if a
>    server allows for a block size that it too can support without
>    having to disconnect and reconnect to get more information. Let's
>    say a client exists which can support the default block size and
>    which can group and split requests if needs be, but which cannot
>    guarantee a minimum block size other than 512. Such a client might
>    want to check what the maximum block size is, but if the server
>    replies to that with "Oh jolly, I can go to my optimized path now!
>    Please use a minimum and preferred block size of 4K", it's pretty
>    much screwed. As said, this is somewhat less likely to happen, but
>    if we can support it, and in combination with the above, I think we
>    should do this.

So, to be clear, the client signals its intention to support block
sizes through the options passed to NBD_OPT_GO, rather than NBD_OPT_INFO
or NBD_OPT_GO?

I had thought of that, and would be prepared to go with it. In fact
I rather like the fact that INFO no longer changes state (after all
it's called ... INFO).

Minor challenges I see:

* We'd need to suggest that the client repeat any info blocks it
  might rely on. Arguably this introduces some pointless repetition
  into the protocol, but there we go.

* We currently have text saying the server shouldn't error an
  NBD_OPT_GO if it previously didn't error an NBD_OPT_INFO for
  that export with no intervening other options; we'd need to
  change that to say "and provided the requested information blocks
  are the same".

> - I think we should not allow a client to do OPT_INFO followed by
>  OPT_EXPORT_NAME. The latter is broken, and a client which can do
>  OPT_INFO can very much do OPT_GO too. This should be a MUST for the
>  client, and a SHOULD for the server to do a hard disconnect then. That
>  also makes it easier to implement the above (and is possibly related)

That's fine by me, but is unrelated to blocksizes.

We'd need to ensure that it didn't preclude a client falling
back to NBD_OPT_EXPORT_NAME if the server didn't actually
understand the NBD_OPT_INFO.

How about going further and adding that clients SHOULD use
NBD_OPT_GO and not NBD_OPT_EXPORT_NAME (even if they didn't
use NBD_OPT_INFO)? Effectively as and when this becomes
part of the main standard, we'd then be deprecating (but
still maintaining support for) NBD_OPT_EXPORT_NAME. Again
using it as a fallback would be fine.

> Other than that, LGTM.

I think we're nearly there then ...

--
Alex Bligh




Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail


Reply to: