[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 05/14] nbd: Share common reply-sending code i
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v5 05/14] nbd: Share common reply-sending code in server |
Date: |
Wed, 20 Jul 2016 12:39:38 +0800 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Tue, 07/19 08:52, Eric Blake wrote:
> 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?
No, I didn't like it because "kill connection" logic is actually in the caller
and not a functionality of this function. I'd say "Return -errno if an error
occured, otherwise return 0".
Fam
- [Qemu-devel] [PATCH v5 14/14] nbd: Implement NBD_CMD_WRITE_ZEROES on client, (continued)
- [Qemu-devel] [PATCH v5 02/14] nbd: Add qemu-nbd -D for human-readable description, Eric Blake, 2016/07/19
- [Qemu-devel] [PATCH v5 04/14] nbd: Treat flags vs. command type as separate fields, Eric Blake, 2016/07/19
- [Qemu-devel] [PATCH v5 12/14] nbd: Improve server handling of shutdown requests, Eric Blake, 2016/07/19
- [Qemu-devel] [PATCH v5 10/14] nbd: Less allocation during NBD_OPT_LIST, Eric Blake, 2016/07/19
- Re: [Qemu-devel] [PATCH for-2.7 v5 00/14] nbd: efficient write zeroes, Fam Zheng, 2016/07/19
- Re: [Qemu-devel] [PATCH for-2.7 v5 00/14] nbd: efficient write zeroes, Paolo Bonzini, 2016/07/19