|
 |
7711c0 |
From 436070bc736adc93d90292f5b5a3a2b75c36017d Mon Sep 17 00:00:00 2001
|
|
|
7711c0 |
From: John Snow <jsnow@redhat.com>
|
|
|
7711c0 |
Date: Mon, 6 May 2019 17:56:13 +0200
|
|
|
7711c0 |
Subject: [PATCH 03/53] nbd: Don't lose server's error to NBD_CMD_BLOCK_STATUS
|
|
|
7711c0 |
|
|
|
7711c0 |
RH-Author: John Snow <jsnow@redhat.com>
|
|
|
7711c0 |
Message-id: <20190506175629.11079-4-jsnow@redhat.com>
|
|
|
7711c0 |
Patchwork-id: 87189
|
|
|
7711c0 |
O-Subject: [RHEL-7.7 qemu-kvm-rhev PATCH 03/19] nbd: Don't lose server's error to NBD_CMD_BLOCK_STATUS
|
|
|
7711c0 |
Bugzilla: 1692018
|
|
|
7711c0 |
RH-Acked-by: Max Reitz <mreitz@redhat.com>
|
|
|
7711c0 |
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
|
|
|
7711c0 |
RH-Acked-by: Thomas Huth <thuth@redhat.com>
|
|
|
7711c0 |
|
|
|
7711c0 |
From: Eric Blake <eblake@redhat.com>
|
|
|
7711c0 |
|
|
|
7711c0 |
When the server replies with a (structured [*]) error to
|
|
|
7711c0 |
NBD_CMD_BLOCK_STATUS, without any extent information sent first, the
|
|
|
7711c0 |
client code was blindly throwing away the server's error code and
|
|
|
7711c0 |
instead telling the caller that EIO occurred. This has been broken
|
|
|
7711c0 |
since its introduction in 78a33ab5 (v2.12, where we should have called:
|
|
|
7711c0 |
error_setg(&local_err, "Server did not reply with any status extents");
|
|
|
7711c0 |
nbd_iter_error(&iter, false, -EIO, &local_err);
|
|
|
7711c0 |
to declare the situation as a non-fatal error if no earlier error had
|
|
|
7711c0 |
already been flagged, rather than just blindly slamming iter.err and
|
|
|
7711c0 |
iter.ret), although it is more noticeable since commit 7f86068d, which
|
|
|
7711c0 |
actually tries hard to preserve the server's code thanks to a separate
|
|
|
7711c0 |
iter.request_ret.
|
|
|
7711c0 |
|
|
|
7711c0 |
[*] The spec is clear that the server is also permitted to reply with
|
|
|
7711c0 |
a simple error, but that's a separate fix.
|
|
|
7711c0 |
|
|
|
7711c0 |
I was able to provoke this scenario with a hack to the server, then
|
|
|
7711c0 |
seeing whether ENOMEM makes it back to the caller:
|
|
|
7711c0 |
|
|
|
7711c0 |
| diff --git a/nbd/server.c b/nbd/server.c
|
|
|
7711c0 |
| index fd013a2817a..29c7995de02 100644
|
|
|
7711c0 |
| --- a/nbd/server.c
|
|
|
7711c0 |
| +++ b/nbd/server.c
|
|
|
7711c0 |
| @@ -2269,6 +2269,8 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
|
|
|
7711c0 |
| "discard failed", errp);
|
|
|
7711c0 |
|
|
|
|
7711c0 |
| case NBD_CMD_BLOCK_STATUS:
|
|
|
7711c0 |
| + return nbd_send_generic_reply(client, request->handle, -ENOMEM,
|
|
|
7711c0 |
| + "no status for you today", errp);
|
|
|
7711c0 |
| if (!request->len) {
|
|
|
7711c0 |
| return nbd_send_generic_reply(client, request->handle, -EINVAL,
|
|
|
7711c0 |
| "need non-zero length", errp);
|
|
|
7711c0 |
| --
|
|
|
7711c0 |
|
|
|
7711c0 |
Signed-off-by: Eric Blake <eblake@redhat.com>
|
|
|
7711c0 |
Message-Id: <20190325190104.30213-2-eblake@redhat.com>
|
|
|
7711c0 |
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
|
|
|
7711c0 |
(cherry picked from commit b29f3a3d2a5fab40dbb4a65fa2f91821ebffae51)
|
|
|
7711c0 |
Signed-off-by: John Snow <jsnow@redhat.com>
|
|
|
7711c0 |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
7711c0 |
---
|
|
|
7711c0 |
block/nbd-client.c | 9 +++------
|
|
|
7711c0 |
1 file changed, 3 insertions(+), 6 deletions(-)
|
|
|
7711c0 |
|
|
|
7711c0 |
diff --git a/block/nbd-client.c b/block/nbd-client.c
|
|
|
7711c0 |
index f3c31d1..532f90c 100644
|
|
|
7711c0 |
--- a/block/nbd-client.c
|
|
|
7711c0 |
+++ b/block/nbd-client.c
|
|
|
7711c0 |
@@ -756,12 +756,9 @@ static int nbd_co_receive_blockstatus_reply(NBDClientSession *s,
|
|
|
7711c0 |
payload = NULL;
|
|
|
7711c0 |
}
|
|
|
7711c0 |
|
|
|
7711c0 |
- if (!extent->length && !iter.err) {
|
|
|
7711c0 |
- error_setg(&iter.err,
|
|
|
7711c0 |
- "Server did not reply with any status extents");
|
|
|
7711c0 |
- if (!iter.ret) {
|
|
|
7711c0 |
- iter.ret = -EIO;
|
|
|
7711c0 |
- }
|
|
|
7711c0 |
+ if (!extent->length && !iter.request_ret) {
|
|
|
7711c0 |
+ error_setg(&local_err, "Server did not reply with any status extents");
|
|
|
7711c0 |
+ nbd_iter_channel_error(&iter, -EIO, &local_err);
|
|
|
7711c0 |
}
|
|
|
7711c0 |
|
|
|
7711c0 |
error_propagate(errp, iter.err);
|
|
|
7711c0 |
--
|
|
|
7711c0 |
1.8.3.1
|
|
|
7711c0 |
|