Re: [Qemu-devel] [PATCH v5 05/14] nbd: Share common reply-sending code in server

[Eric Blake]

On 07/18/2016 11:10 PM, Fam Zheng wrote:
> On Mon, 07/18 22:07, Eric Blake wrote:
>> Rather than open-coding NBD_REP_SERVER, reuse the code we
>> already have by adding a length parameter.  Additionally,
>> the refactoring will make adding NBD_OPT_GO in a later patch
>> easier.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> ---
>> v4: no change
>> v3: rebase to changes earlier in series
>> ---
>>  nbd/server.c | 48 +++++++++++++++++++++++-------------------------
>>  1 file changed, 23 insertions(+), 25 deletions(-)
>>
>> diff --git a/nbd/server.c b/nbd/server.c
>> index 85c4f5d..c8716f1 100644
>> --- a/nbd/server.c
>> +++ b/nbd/server.c
>> @@ -195,12 +195,15 @@ static ssize_t nbd_negotiate_drop_sync(QIOChannel 
>> *ioc, size_t size)
>>
>>  */
>>
>> -static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t 
>> opt)
>> +/* Send a reply header, including length, but no payload.
>> + * Return -errno to kill connection, 0 to continue negotiation. */
> 
> Not a show stopper but I'm not sure documenting the control logic of the
> outermost caller a few layers away is a good idea, the same question applies 
> to
> functions below as well.

The documentation here accurately describes this function.  Or is your
complaint that the outermost caller is lacking documentation, and
therefore I should first do a patch that uniformly adds documentation,
before changing behavior, so that this function doesn't end up with
details while the outermost caller remains undocumented?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature