Re: [Nbd] [PATCH] Add time out for receiving data.

[Wouter Verhelst <w@...112...>]

Hi,

First of all, please try to avoid sending patches as attachment if at
all possible; doing so makes it much harder than necessary to review
and/or merge them.

On Wed, Jun 10, 2015 at 03:12:06PM +0900, Yoshinori Sato wrote:
> Hello.
> 
> I found the problem of falling into recovery fail by network failure.
> That can't return from a temporary failure.
>
> When it was investigated, I found out that it's the waiting state in readit.
> So I decided to add time-out and make them recover.

I'm not sure I understand which problem you're trying to solve here. Can
you explain why we need a timeout?

Also, I'm afraid this patch introduces more problems than it solves: I
don't think the consume() call should time out. If it does, you'll
either just have to go again, or drop the connection. At any rate, a
timeout in consume() currently gets the internal state of nbd-server out
of sync with what's going over the wire, and that's just not an option.

In that light, why doesn't a timeout just kill the connection instead?
If we time out, we're assuming the other side is dead. So why bother
trying to read more data from the socket? If the other side is indeed
dead, that will just fail again. If the other side is not dead, and it
is indeed a transitional failure, then TCP should take care of things
eventually recovering. If not, you've got a bug in your TCP stack and no
amount of timing out will fix it.

Unless I'm missing something?

Because of the possible getting-out-of-sync bug, I'm not going to merge
your patch as is; but even if that's fixed, I'm going to need some
convincing before merging a patch that introduces this kind of
behaviour.

Regards,

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26