Re: [Qemu-devel] [PATCH v2 3/7] block/nbd-client: refactor reading reply

[Paolo Bonzini]

On 19/09/2017 13:03, Vladimir Sementsov-Ogievskiy wrote:
>>>
>> I disagree that it is easier to extend it in the future.  If some
>> commands in the future need a different "how do we read it" (e.g. for
>> structured reply), nbd_read_reply_entry may not have all the information
>> it needs anymore.
> 
> how is it possible? all information is in requests[i].

If you are okay with violating information hiding, then it is.

>>
>> In fact, you're pushing knowledge of the commands from nbd_co_request to
>> nbd_read_reply_entry:
>>
>> +        if (s->reply.error == 0 &&
>> +            s->requests[i].request->type == NBD_CMD_READ)
>>
>> and knowledge of NBD_CMD_READ simply doesn't belong there.  So there is
>> an example of that right now, it is not some arbitrary bad thing that
>> could happen in the future.
> 
> I can't agree that my point of view is wrong, it's just different.
> You want commands, reading from socket, each knows what it should read.
> I want the receiver - one sequential reader, that can read all kinds of
> replies and just wake requesting coroutines when all reading is done.
> For me it looks simpler than switching.

It may look simpler, but I think it goes against the principle of
coroutines which is to have related code in the same function.  Here you
have the command function that takes care of sending the request payload
but not receiving the reply payload (except that for commands that
aren't as simple as read or write, it will have to _process_ the reply
payload).  What to do with the reply payload is in another function that
has to peek at the command in order to find out what to do.  It doesn't
seem like a better design, honestly.

Paolo