[Nbd] [PATCH] Only send one reply on oversize writes

[Alex Bligh <alex@...872...>]

--On 28 May 2011 01:25:13 +0200 Wouter Verhelst <w@...112...> wrote:

> Will have an in-depth review of your patch tomorrow (it's too late (or
> should I say, early?) right now to do that properly), and if it looks
> fine, will merge.

Rereading it, I think I have found a problem with the way we handle
oversize writes (both before and after my patch). NBD_CMD_WRITE is
handled liked this:

        while(len > 0) {
                readit(client->net, buf, currlen);
                ...
                if (expwrite(request.from, buf, len, client,
                             request.type & NBD_CMD_FLAG_FUA)) {
                        DEBUG("Write failed: %m" );
                        ERROR(client, reply, errno);
                        continue;
               }
               SEND(client->net, reply);
               len -= currlen;
               currlen = (len < BUFSIZE) ? len : BUFSIZE;
       }
       DEBUG("OK!\n");

Surely the SEND() should be outside the loop, or we will send multiple
replies to one NBD_CMD_READ (which will really upset the kernel as
the request will be freed).

Again, only tested with "make check", but I don't think we test
oversize writes (we probably should do in the oversize test).

Available from git.alex.org.uk as usual.

Signed-off-by: Alex Bligh <alex@...872...>
---
 nbd-server.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/nbd-server.c b/nbd-server.c
index a99c27b..6d734b3 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -1535,10 +1535,10 @@ int mainloop(CLIENT *client) {
 					ERROR(client, reply, errno);
 					continue;
 				}
-				SEND(client->net, reply);
 				len -= currlen;
 				currlen = (len < BUFSIZE) ? len : BUFSIZE;
 			}
+			SEND(client->net, reply);
 			DEBUG("OK!\n");
 			continue;
 
-- 
1.7.4.1