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

Re: [Nbd] [PATCH v3 4/5] doc: Propose STRUCTURED_REPLY extension



On 03/31/2016 09:28 AM, Alex Bligh wrote:
>>
>> But having written that, I still see two variations within both option 1
>> and option 2:
>>

Call this #C1

>> - if a request has any mode that requires data in its reply, then ALL
>> replies to that request must be structured
>>

and #C2

>> - a reply that includes data MUST use a structured reply, but a request
>> (other than NBD_CMD_READ) can still have a simple reply in cases where
>> no data is required (such as when reporting an error like EINVAL on
>> improper flags
> 
> Please let's have the first of these!
> 
>> That is, Option #A1, #A2, and #A3 were concerned about what
>> NBD_CMD_WRITE can reply,
> 
> although as I indicate elsewhere, it would actually be useful if
> NBD_CMD_WRITE can return a structured reply so it can indicate an
> error offset.
> 
>> while my two variations are concerned about
>> what NBD_CMD_GET_LBA_STATUS can reply (I still think, for back-compat
>> reasons, the NBD_CMD_READ must be special-cased, and be all-or-none with
>> structured replies, even when reporting an error like EINVAL for an
>> unrecognized flag).
>>
>> If we go with option #A1 (CMD_WRITE never uses structured), then the
>> parallel argument is that commands with possibility of data must never
>> use simple.  If we go with option #A2 (CMD_WRITE may use structured),
>> then (other than CMD_READ), it makes a bit more sense to allow a simple
>> reply if the server is sending an error without data (then again, the
>> whole notion of the structured error with UTF-8 human-readable string
>> makes it less likely that we'd even have a reason to reply without data).

Or, as written above, the pairings (#A1 and #C1) and (#A2 and #C2) make
the most sense, the pairing (#A1 and #C2) is odd (if WRITE can't use
multiple reply types, then why should GET_LBA_STATUS be allowed to?);
the pairing (#A2 and #C1) is a little bit better (old commands can favor
either reply type, new commands should only use new replies) but more
complex to describe.  And while Wouter is not in favor of #A3, you are
correct that (#A3 and #C1) is easy (always use new reply) while (#A3 and
#C2) is wrong (why should GET_LBA_STATUS be allowed multiple reply types)
> 
> Hmm.. Well my favourite remains #A2 over #A1, but this is one reason I
> preferred #A3 - the simplicity of negotiating a single reply format
> and no complex rules about when one can be used versus the other! I
> suppose the second simplest route is "If structured replies have
> been negotiated, then the server MAY always send either structured
> or unstructured replies". Sigh.

That last sentence is summed up as (#A2 and #C2).

>> Yeah, the user space already has to ask the kernel if the kernel is
>> willing to receive structured replies before sending
>> NBD_OPT_STRUCTURED_REPLY to the server, but making the server echo back
>> the negotiation in the transmission flags saves the userspace from
>> needing an additional ioctl() to inform the kernel about whether the
>> negotiation with the server was successful.
> 
> Well it sounds to me like instead we need a NBD_FLAG_SEND_STRUCTURED_REPLY
> i.e. we need a flag to say "send structured replies", not a flag to say
> "send DF".
> 
> Or am I missing the point.

I don't care about the flag name; naming it NBD_FLAG_STRUCTURED_REPLY
instead of NBD_FLAG_SEND_DF may make it easier to describe why it is
useful for the case of a client split between user/kernel.  Later on, we
can still claim that a client MUST NOT set NBD_CMD_FLAG_DF if structured
replies are not negotiated (which is equivalent to the transmission flag
being set, whether that transmission flag is named NBD_FLAG_SEND_DF or
NBD_FLAG_STRUCTURED_REPLY, but by referring to the negotiation result
rather than the flag bit, the wording might come off cleaner).

>> And here, I was TRYING to be clever by using "data chunk" to mean
>> "either TYPE_OFFSET_DATA or TYPE_OFFSET_HOLE".  But that overloads the
>> term "data chunk", so maybe I can come up with a better term?
> 
> Doh. Hoist by my own terminology. Perhaps we need a different term.
> 
>> Again, I was trying to lump data and hole types both as "data chunks".
>> Maybe "content chunks" is a good unambiguous term?
> 
> Yes.

Will do, sounds like I post a v4 today, even though we are still waiting
for Markus to have a chance to respond.

> How about you define:
> 
> * content chunks (HOLE and DATA)
> * non-content chunks (NONE and ERROR)
> 
> and define everything explicitly as a chunk.

Reformulated slightly:

There are 5 chunk types currently defined.
Of those 5, 2 are content chunks (HOLE, DATA) (3 are non-content, but
other non-content may be defined later)
Of those 5, 2 are error chunks (ERROR, ERROR_OFFSET) (3 are non-error,
but other non-error may be defined later)
Of those 5, any type can be the final chunk
Of those 5, only the NONE chunk is not permitted to be the final chunk

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

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: