On 03/30/2016 01:33 AM, Wouter Verhelst wrote: > Morning, > > On Wed, Mar 30, 2016 at 07:59:15AM +0100, Alex Bligh wrote: >> On 30 Mar 2016, at 00:17, Eric Blake <ebl...@redhat.com> wrote: >>>> >>>> -The server replies with: >>>> +Replies take one of two forms. They may either be structured replies, >>> >>> Hmm, you put your strawman directly in the 'transmission phase' section, >>> while mine is deferred to the 'Experimental Extensions' section, at >>> least until we have a reference client and server implementation. >> >> Yeah, my wording was straw man but I think it should go into the main >> body of the work. Obviously in that case it wouldn't be *merged* until >> we had a working implementation. >> >> The SELECT stuff is a bit different as I am not sure it was intended >> to be standardized short term (i.e. it was a longer term experiment >> IIRC). > > Actually, I plan to implement that when I get around to doing STARTTLS > (which I've started work on, but is far from ready).
On that tangent, I found SELECT slightly ambiguous (particularly since I'm also considering a proposal to modify NBD_REP_SERVER to expose alignment details, so it would have to play nicely with SELECT): Based on normative text alone, we would have: S: 64 bits, 0x3e889045565a9 S: 32 bits, NBD_OPT_SELECT (6) S: 32 bits, NBD_REP_SERVER (2) S: 32 bits, length S: 32 bits, name length S: 'name length' bytes, UTF-8 encoded name (is it NUL-terminated?) S: 'length - name length' bytes, which is UTF-8 encoded message to display to human This implies that 'name length' <= 'length - 4', although we don't actually state that the server MUST NOT send a name length longer than that. It might not hurt to also require that length include space for a NUL terminator (in both the name, and in the optional human-readable information field), but only if that matches existing practice (if it does not, then we should document that the client and server are NOT dealing with NUL-terminated UTF-8 strings, but relying on length instead). The SELECT text then refines this: S: 64 bits, 0x3e889045565a9 S: 32 bits, NBD_OPT_SELECT (6) S: 32 bits, NBD_REP_SERVER (2) S: 32 bits, length S: 32 bits, name length S: 'name length' bytes, UTF-8 encoded name (is it NUL-terminated?) S: 64 bits, size S: 16 bits, export flags but doesn't mention what happens if 'length' > 'name length + 14'. Presumably, if the server wants to include the same UTF-8 human information as before, it would go AFTER the 16 bits for the export field (in other words, the SELECT extension is carving out fields in the MIDDLE of the payload). So, once I know for sure about the handling of NUL bytes, I'll probably try my hand at a patch to clarify the wording in both the normative and in the SELECT extension. >> I guess Wouter should be the arbiter of whether he'd prefer to merge >> it only when we have an implementation. My concern is that it may >> hang around in 'experimental', when it needs properly merging, which >> will require re-wordsmithing. > > I'll merge whatever the outcome of this discussion is in the > experimental section; that way, it won't get lost. I can reformulate > your text to fit there myself if needs be, although obviously having > something that doesn't require such work is preferable. However, I'm > leaning more towards Eric's proposal at this time, because it feels more > mature. Okay, I'll do my best to word things in the experimental section so that we can minimize edits when moving text. Maybe I'll even go so far as to post a 2-patch series; one with the text in the experimental section for committing now; the other for moving the text (but NOT to be committed) to the normative section, for demonstrating the level of effort required. > > I now realize, after having slept over it, that you guys probably meant > for an error-with-offset to only mark bytes that were part of *that* > particular reply chunk to be marked as faulty, which makes more sense > than "the whole request range from that point on", as I was interpreting > it... Indeed, anything I can do to make the wording more clear is welcome. Obviously, a server can mark an entire data chunk invalid by setting the offset to the same value as the offset used in that chunk; the real power is that an offset that is > chunk-offset but < 'chunk-offset + chunk-length' then lets the client know that the first half of chunk was valid, the second half of chunk is bogus, and no other chunk is impacted. And when a lazy server sends error-without-offset, the client is forced to treat ALL chunks as invalid; which is why I state that the server SHOULD NOT mix error-with-offset and error-without-offset in the same reply (SHOULD NOT, but not MUST NOT, because I can't predict every possible error scenario, and there may be some fatal chain of events where the server is forced to mix after all). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
