Re: [Nbd] NBD_OPT_GO

[Wouter Verhelst <w@...112...>]

On Tue, Apr 05, 2016 at 11:56:40AM -0600, Eric Blake wrote:
> On 04/05/2016 11:10 AM, Wouter Verhelst wrote:
> > On Tue, Apr 05, 2016 at 01:37:26PM +0100, Alex Bligh wrote:
> >> proto.md is unclear on what export is selected when you use NBD_OPT_GO. I take it
> >> that the selected export is the one previously selected using NBD_OPT_SELECT.
> >> If that's the case, it should say that, and it should also say that
> >> NBD_OPT_GO MUST NOT be used until NBD_OPT_SELECT has been used.
> >>
> >> But this got me thinking. Given the option has a length (which by default would
> >> be zero), why not allow NBD_OPT_GO (optionally) to carry an export name itself?
> >> Then it could be used instead of NBD_OPT_EXPORT_NAME completely. We could
> >> (eventually) deprecate NBD_OPT_EXPORT_NAME. IE if no name is specified, use
> >> the one from the last NBD_OPT_SELECT, but if one was specified, use that.
> > 
> > Mm. We could also drop NBD_OPT_SELECT completely.
> > 
> > The reason I added SELECT/GO was because EXPORT_NAME has no way for the
> > server to report error (other than to close the connection). The spec
> > for GO explicitly states that the server must reply with REP_ACK before
> > we move on to the transmission phase, so it does not have that problem.
> 
> And we just recently changed from REP_ACK to REP_SERVER on GO because of
> the situation with transmission flags that might be modified between
> SELECT/GO.
> 
> > 
> > In that light, SELECT is somewhat superfluous. That is, it could be
> > useful to have a SELECT-like option (so a client can query information
> > from the server without communicating intent to move on to the
> > transmission phase), but it might be best to rename NBD_OPT_SELECT to
> > NBD_OPT_INFO and drop its side effects. Then, a client who wishes to
> > move on to transmission without using the broken EXPORT_NAME can select
> > NBD_OPT_GO with (or without) a name, and *if* the server sends ACK,
> > we're in transmission.
> 
> Except that NBD_REP_ACK doesn't carry enough details; we _do_ need a
> successful reply to GO to be the modified NBD_REP_SERVER including
> transmission flags (if that's what you mean by ACK).

It wasn't, but that was mostly because I'd forgotten we changed the
proper reply for NBD_OPT_GO :-)

> I like the idea of not carrying any state.  You can query info about any
> export name (does it require TLS, what size is it, will it be
> read-only), and get more details than what NBD_OPT_LIST would give, but
> you don't have to. And NBD_OPT_GO does not do any magic to the empty
> string beyond what NBD_OPT_EXPORT_NAME already does, and does not care
> whether any earlier NBD_OPT_LIST or NBD_OPT_SELECT (or if we rename it
> NBD_OPT_INFO) took place.

Exactly.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12