Commit a0d7ce20 authored Mar 08, 2018 by Vladimir Sementsov-Ogievskiy Committed by Eric Blake Mar 13, 2018

nbd/server: fix: check client->closing before sending reply



Since the unchanged code has just set client->recv_coroutine to
NULL before calling nbd_client_receive_next_request(), we are
spawning a new coroutine unconditionally, but the first thing
that coroutine will do is check for client->closing, making it
a no-op if we have already detected that the client is going
away.  Furthermore, for any error other than EIO (where we
disconnect, which itself sets client->closing), if the client
has already gone away, we'll probably encounter EIO later
in the function and attempt disconnect at that point.  Logically,
as soon as we know the connection is closing, there is no need
to try a likely-to-fail a response or spawn a no-op coroutine.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20180308184636.178534-4-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: squash in further reordering: hoist check before spawning
next coroutine, and document rationale in commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>

parent 37e02aeb

nbd/server.c

+9 −8

Original line number	Diff line number	Diff line
		@@ -1543,14 +1543,6 @@ static coroutine_fn void nbd_trip(void *opaque)
		req = nbd_request_get(client);
		ret = nbd_co_receive_request(req, &request, &local_err);
		client->recv_coroutine = NULL;
		nbd_client_receive_next_request(client);
		if (ret == -EIO) {
		goto disconnect;
		}

		if (ret < 0) {
		goto reply;
		}

		if (client->closing) {
		/*
		@@ -1560,6 +1552,15 @@ static coroutine_fn void nbd_trip(void *opaque)
		goto done;
		}

		nbd_client_receive_next_request(client);
		if (ret == -EIO) {
		goto disconnect;
		}

		if (ret < 0) {
		goto reply;
		}

		switch (request.type) {
		case NBD_CMD_READ:
		/* XXX: NBD Protocol only documents use of FUA with WRITE */