[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands |
Date: |
Tue, 14 Jun 2016 17:11:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 |
On 14/06/2016 17:02, 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.
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
- Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Paolo Bonzini, 2016/06/13
- Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Eric Blake, 2016/06/13
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Wouter Verhelst, 2016/06/15
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Wouter Verhelst, 2016/06/15
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Alex Bligh, 2016/06/15
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Paolo Bonzini, 2016/06/15
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Alex Bligh, 2016/06/15
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Paolo Bonzini, 2016/06/15
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Wouter Verhelst, 2016/06/15
- Re: [Qemu-devel] [Nbd] [PATCH v4 04/11] nbd: Improve server handling of bogus commands, Wouter Verhelst, 2016/06/15