Re: [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry

[Eric Blake]

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