Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

[Wouter Verhelst]

On Tue, Jun 14, 2016 at 04:02:15PM +0100, Alex Bligh wrote:
> 
> On 14 Jun 2016, at 14:32, Paolo Bonzini <address@hidden> 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.
> 
> > 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.
> 
> Note the standard is not defined by the client implementation, but
> by the protocol document.

No, it isn't. At least not yet.

The standard document has only become formal a few months ago. It's
certainly possible that we made a mistake formalizing things, and if so,
we should fix the document rather than saying "whatever you've been
doing these years, even though it worked, is wrong".

There are more clients than the Linux and qemu ones, but I think it's
fair to say that those two are the most important ones. If they agree
that a read reply which errors should come without payload, then I think
we should update the standard to say that, too.

> > What servers do doesn't matter, if all the clients agree.
> 
> 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.

That much, at any rate, is true.

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