qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"open list:Network Block Dev..." <qemu-block@nongnu.org>
Subject: [Qemu-devel] [PULL 04/17] nbd/server: fix: check client->closing before sending reply
Date: Tue, 13 Mar 2018 12:13:32 -0500	[thread overview]
Message-ID: <20180313171345.659672-5-eblake@redhat.com> (raw)
In-Reply-To: <20180313171345.659672-1-eblake@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

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>
---
 nbd/server.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 5f292064af0..b230ecb4fb8 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -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 */
-- 
2.14.3

  parent reply	other threads:[~2018-03-13 17:13 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-13 17:13 [Qemu-devel] [PULL 00/17] NBD patches for 2018-03-13 (2.12 softfreeze) Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 01/17] iotests: Fix stuck NBD process on 33 Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 02/17] nbd/server: move nbd_co_send_structured_error up Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 03/17] nbd/server: fix sparse read Eric Blake
2018-03-13 17:13 ` Eric Blake [this message]
2018-03-13 17:13 ` [Qemu-devel] [PULL 05/17] nbd/server: refactor nbd_trip: cmd_read and generic reply Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 06/17] nbd/server: refactor nbd_trip: split out nbd_handle_request Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 07/17] nbd/server: Honor FUA request on NBD_CMD_TRIM Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 08/17] block: let blk_add/remove_aio_context_notifier() tolerate BDS changes Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 09/17] iotests: add 208 nbd-server + blockdev-snapshot-sync test case Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 10/17] nbd/server: add nbd_opt_invalid helper Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 11/17] nbd/server: add nbd_read_opt_name helper Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 12/17] nbd: BLOCK_STATUS for standard get_block_status function: server part Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 13/17] block/nbd-client: save first fatal error in nbd_iter_error Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 14/17] nbd: BLOCK_STATUS for standard get_block_status function: client part Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 15/17] iotests.py: tiny refactor: move system imports up Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 16/17] iotests: add file_path helper Eric Blake
2018-03-13 17:13 ` [Qemu-devel] [PULL 17/17] iotests: new test 209 for NBD BLOCK_STATUS Eric Blake
2018-03-13 18:17 ` [Qemu-devel] [PULL 00/17] NBD patches for 2018-03-13 (2.12 softfreeze) no-reply
2018-03-13 19:30   ` Eric Blake
2018-03-13 18:19 ` no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180313171345.659672-5-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).