Re: [PATCH] oxenstored: fix short-write issue

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 28/10/15 13:34, David Scott wrote:
>
>> On 27 Oct 2015, at 17:31, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> On 27/10/15 17:28, Samuel Thibault wrote:
>>> Andrew Cooper, le Tue 27 Oct 2015 17:21:39 +0000, a écrit :
>>>> as the second attempted write could return short as well.
>>> That is fine. The second attempt will only return a short write if there
>>> was really not enough room for it, which is what we want.
>> Then surely the bug is that Xs_ring.write returns short when it shouldn’t?
>
>> On 27 Oct 2015, at 17:31, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
>>
>> Another way would be to make ml_interface_write write both pieces
>> instead of just a contiguous one.  The caml code would then look less
>> suspicious :)
>>
> I agree with both Andrew and Samuel that it would be a better fix if Xs_ring.write (== “ml_interface_write”) wrote both pieces at once.
>
> However I believe the ‘suspicious’ OCaml patch fixes the specific issue (— or have I missed something?).

Actually the suspicious OCaml is still buggy in the case that there was
a genuine short write followed by a false short write from wrapping the
ring.

>
> Last time I meddled with the C-level ring reading/writing code I didn’t get it quite right. Does anyone have time to prototype what a C-level fix would look like? If we’re short of time I could live with the OCaml-level fix, especially since Wei has done some stress-testing (assuming everyone believes it fixes the issue)

There are several bugs in that function (the remaining size calculations
are plain wrong), and Samuels patch sadly doesn't address all of them. 

I will throw a different patch together addressing all the issues I can
spot.

~Andrew