On 14/06/2016 17:02, Alex Bligh wrote: > > On 14 Jun 2016, at 14:32, Paolo Bonzini <pbonz...@redhat.com> wrote: > >> >> On 13/06/2016 23:41, Alex Bligh wrote: >>> That's one of the reasons that there is a proposal to add >>> STRUCTURED_READ to the spec (although I still haven't had time to >>> implement that for qemu), so that we have a newer approach that allows >>> for proper error handling without ambiguity on whether bogus bytes must >>> be sent on a failed read. But you'd have to convince me that ALL >>> existing NBD server and client implementations expect to handle a read >>> error without read payload, otherwise, I will stick with the notion that >>> the current spec wording is correct, and that read errors CANNOT be >>> gracefully recovered from unless BOTH sides transfer (possibly bogus) >>> bytes along with the error message, and which is why BOTH sides of the >>> protocol are warned that read errors usually result in a disconnection >>> rather than clean continuation, without the addition of STRUCTURED_READ. >> >> I suspect that there are exactly two client implementations, > > My understanding is that there are more than 2 client implementations. > A quick google found at least one BSD client. I bet read error handling > is a mess in all of them.
Found it, it is exactly the same as Linux and QEMU: https://github.com/bitrig/bitrig/blob/418985278/sys/dev/nbd.c#L577 >> namely >> Linux and QEMU's, and both do the right thing. > > This depends what you mean by 'right'. Both appear to be non-compliant > with the standard. I mean "what makes sense". > Note the standard is not defined by the client implementation, but > by the protocol document. > > IMHO the 'right thing' is what is in the spec. Servers can't send an > error in any other way if they don't buffer the entire read first, as the > read may error towards the end. > > To illustrate the problem, look consider what qemu itself would do as > a server if it can't buffer the entire read issued to it. Return ENOMEM? > The spec originally was not clear on how errors on reads should be > handled, leading to any read causing a protocol drop. The spec is > now clear. Unfortunately it is not possible to make a back compatible > fix. Hence the real fix here is to implement structured replies, > which is what Eric and I have been working on. I agree that structured replies are better. However, it looks like the de facto status prior to structured replies is that the error is in the spec, and this patch introduces a regression. Paolo
