[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/5] nbd/server: fix sparse read
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/5] nbd/server: fix sparse read |
Date: |
Fri, 9 Mar 2018 13:55:10 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 03/08/2018 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:
In case of io error in nbd_co_send_sparse_read we should not
"goto reply:", as it is fatal error and common behavior is
disconnect in this case. We should not try to send client an
s/send/send the/
error reply, representing channel-io error on previous try to
send a reply.
s/representing .../since we already hit a channel-io error on our
previous attempt to send a reply/
Fix this by handle block-status error in nbd_co_send_sparse_read,
s/handle/handling/
so nbd_co_send_sparse_read fails only on io error. Then just skip
common "reply:" code path in nbd_trip.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
v2: code movement splitted to 01
remove error_report
fix indent
fix commit message subject line
add comment to nbd_co_send_sparse_read (be free to adjust it)
Not a problem, I know English is not your native tongue, so I don't mind
touching it up, and I already admire your efforts at programming in a
second language (a skill I lack).
nbd/server.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index c406b0656d..e0de431e10 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1365,6 +1365,10 @@ static int coroutine_fn
nbd_co_send_structured_error(NBDClient *client,
return nbd_co_send_iov(client, iov, 1 + !!iov[1].iov_len, errp);
}
+/* Do sparse read and send structured reply to the client.
s/Do/Do a/
+ * Returns -errno if sending fails. bdrv_block_status_above() fail is not
s/fail/failure/
+ * considered as an error, but reported to the client instead (as a structured
+ * error). */
The comment tail */ usually gets its own line.
Here's what I used:
/* Do a sparse read and send the structured reply to the client.
* Returns -errno if sending fails. bdrv_block_status_above() failure is
* reported to the client, at which point this function succeeds.
*/
static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
uint64_t handle,
uint64_t offset,
@@ -1385,8 +1389,13 @@ static int coroutine_fn
nbd_co_send_sparse_read(NBDClient *client,
bool final;
if (status < 0) {
- error_setg_errno(errp, -status, "unable to check for holes");
- return status;
+ char *msg = g_strdup_printf("unable to check for holes: %s",
+ strerror(-status));
I had to double-check block/io.c, but it looks like
bdrv_block-status_above() does indeed document a return of negative
errno on failure. (My suspicion is that the documentation might be
wrong, because I did not audit error returns when doing my byte-based
work; but a quick audit of a couple of random drivers shows that at
least iscsi.c and qed.c appear to comply.)
+
+ ret = nbd_co_send_structured_error(client, handle, -status, msg,
+ errp);
+ g_free(msg);
+ return ret;
I wonder if a separate patch should clean this up to let
nbd_co_send_structured_error() do message formatting (making it varargs)
- but doesn't affect this patch.
With grammar tweaks,
Reviewed-by: Eric Blake <address@hidden
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [Qemu-devel] [PATCH 0/5] nbd server fixing and refactoring before BLOCK_STATUS, Vladimir Sementsov-Ogievskiy, 2018/03/08
- [Qemu-devel] [PATCH v2 2/5] nbd/server: fix sparse read, Vladimir Sementsov-Ogievskiy, 2018/03/08
- Re: [Qemu-devel] [PATCH v2 2/5] nbd/server: fix sparse read,
Eric Blake <=
- [Qemu-devel] [PATCH 4/5] nbd/server: refactor nbd_trip: cmd_read and generic reply, Vladimir Sementsov-Ogievskiy, 2018/03/08
- [Qemu-devel] [PATCH 5/5] nbd/server: refactor nbd_trip: split out nbd_handle_request, Vladimir Sementsov-Ogievskiy, 2018/03/08
- [Qemu-devel] [PATCH 3/5] nbd/server: fix: check client->closing before reply sending, Vladimir Sementsov-Ogievskiy, 2018/03/08
- [Qemu-devel] [PATCH 1/5] nbd/server: move nbd_co_send_structured_error up, Vladimir Sementsov-Ogievskiy, 2018/03/08
- Re: [Qemu-devel] [PATCH 0/5] nbd server fixing and refactoring before BLOCK_STATUS, Eric Blake, 2018/03/09