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