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 ------------------------------------------------------------------------------ _______________________________________________ Nbd-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/nbd-general
