On 08/07/2017 11:09 AM, Vladimir Sementsov-Ogievskiy wrote: >>>>> On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: >>>>>> Set reply.handle to 0 on error path to prevent normal path of >>>>>> nbd_co_receive_reply.
>> >> The client still tried to send a flush request to the server, when it >> should REALLY quit talking to the server at all and just disconnect, >> because the moment the client recognizes that the server has sent >> garbage is the moment that the client can no longer assume that the >> server will react correctly to any further commands. >> >> So I don't think your patch is quite correct, even if you have >> identified a shortfall in our client code. >> > Why do you think so? It just does what it states in commit message. The commit message doesn't state much on the way of WHY. It it the subsequent discussion that says that the reason WHY we need to fix something is to make the client robustly hang up when it encounters a broken server - but once you couch it in those terms, this patch is NOT making the client hang up gracefully (it is making the client skip ONE invalid reply, but then immediately lets the client send another request and gets stuck waiting for a reply to that second request). A full fix for the issue would make sure the client hangs up as soon as it detects a bogus server. > > Patch 17 should fix the case (but I doubt that it can be taken separately). It's okay if it takes more than one patch to fix an issue, if the earlier patches document that more work is still needed. Or maybe we can squash the best parts of 17 and this one. I'm still playing with the best way to minimally address the issue for 2.10. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature