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

Re: [Nbd] [PATCH v2 3/3] doc: Propose Structured Read extension



On Wed, Mar 30, 2016 at 03:16:02PM -0600, Eric Blake wrote:
> On 03/30/2016 02:44 PM, Wouter Verhelst wrote:
> >>
> >> @@ -579,7 +602,7 @@ To remedy this, a `SELECT` extension is envisioned. This extension adds
> >>  two option requests and one error reply type, and extends one existing
> >>  option reply type.
> >>
> >> -* `NBD_OPT_SELECT`
> >> +* `NBD_OPT_SELECT` (6)
> > 
> > NAK. The spec is currently consistent in associating numbers to
> > constants in only *one* place. This is no accident, and I'd like to keep
> > it that way.
> > 
> > (at least I think it is; if not, that's a bug)
> 
> I didn't realize it was intentional; I was trying to be consistent in
> the text I add, but I can be consistent in the opposite manner (only tie
> it to a value in one place).

Thanks.

> The spec does mention that the "server will set bit 0"; that might read
> better as "server will set the global flag `NBD_FLAG_FIXED_NEWSTYLE`"

Ah, yes, that's an oversight indeed. Thanks for spotting it :-)

[...]
> So to recap, IF the client negotiates structured replies, then the
> server MUST set the export flag NBD_FLAG_SEND_DF. The user-space client
> then passes this flag to the kernel, and the kernel instantly knows that
> structured replies will be arriving, and that the kernel may use
> NBD_CMD_FLAG_DF on reads.

Right.

> > Thinking about this, I suppose it makes sense to rename the "global"
> > flags field as the "negotiation flags field" which signals incompatible
> 
> Maybe "handshake flags" instead, since we document
> handshake/transmission phases?

That works better, indeed (although the document *also* mentions negotiation
explicitly... ah well).

> > changes in the negotiation phase, and the "per-export" flags field as
> > the "transmission flags field" which signals features the client can use
> > during transmission, or something along those lines. Thoughts?
> 
> The rename doesn't hurt anything (it is a document artifact only), and
> makes enough sense that we could do it as a pre-req patch.

Right.

I've changed it like that, and pushed all of today's changes.

Regards,

-- 
< 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



Reply to: