fix qemu 2.9 drive mirroring to nbd target

cherry pick from qemu-kvm-ev-2.9.0-16.el7_4.11.1
https://cbs.centos.org/koji/buildinfo?buildID=21003

Tue Jun 13 2017 Miroslav Rezanina <mrezanin@redhat.com> - rhev-2.9.0-10.el7

- kvm-nbd-make-it-thread-safe-fix-qcow2-over-nbd.patch [bz#1454582]

Tue Aug 15 2017 Miroslav Rezanina <mrezanin@redhat.com> - rhev-2.9.0-16.el7_4.4
- kvm-nbd-strict-nbd_wr_syncv.patch [bz#1467509]
- kvm-nbd-read_sync-and-friends-return-0-on-success.patch [bz#1467509]
- kvm-nbd-make-nbd_drop-public.patch [bz#1467509]
- kvm-nbd-server-get-rid-of-nbd_negotiate_read-and-friends.patch [bz#1467509]

Mon Oct 09 2017 Miroslav Rezanina <mrezanin@redhat.com> - rhev-2.9.0-16.el7_4.9
- kvm-nbd-client-Fix-regression-when-server-sends-garbage.patch [bz#1495474]
- kvm-fix-build-failure-in-nbd_read_reply_entry.patch [bz#1495474]
- kvm-nbd-client-avoid-spurious-qio_channel_yield-re-entry.patch [bz#1495474]
- kvm-nbd-client-avoid-read_reply_co-entry-if-send-failed.patch [bz#1495474]
- kvm-qemu-iotests-improve-nbd-fault-injector.py-startup-p.patch [bz#1495474]
- kvm-qemu-iotests-test-NBD-over-UNIX-domain-sockets-in-08.patch [bz#1495474]
- kvm-block-nbd-client-nbd_co_send_request-fix-return-code.patch [bz#1495474]
- Resolves: bz#1495474
This commit is contained in:
Alexandre Derumier 2018-02-07 23:40:35 +01:00 committed by Wolfgang Bumiller
parent db2a3b4757
commit b45e13fe5c
13 changed files with 2389 additions and 0 deletions

View File

@ -0,0 +1,136 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Sun, 11 Jun 2017 03:30:07 +0200
Subject: [PATCH] nbd: make it thread-safe, fix qcow2 over nbd
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170611033007.399-1-eblake@redhat.com>
Patchwork-id: 75581
O-Subject: [RHEV-7.4 qemu-kvm-rhev PATCH] nbd: make it thread-safe, fix qcow2 over nbd
Bugzilla: 1454582
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
From: Paolo Bonzini <pbonzini@redhat.com>
NBD is not thread safe, because it accesses s->in_flight without
a CoMutex. Fixing this will be required for multiqueue.
CoQueue doesn't have spurious wakeups but, when another coroutine can
run between qemu_co_queue_next's wakeup and qemu_co_queue_wait's
re-locking of the mutex, the wait condition can become false and
a loop is necessary.
In fact, it turns out that the loop is necessary even without this
multi-threaded scenario. A particular sequence of coroutine wakeups
is happening ~80% of the time when starting a guest with qcow2 image
served over NBD (i.e. qemu-nbd --format=raw, and QEMU's -drive option
has -format=qcow2). This patch fixes that issue too.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 6bdcc018a6ed760b9dfe43539124e420aed83092)
Signed-off-by: Eric Blake <eblake@redhat.com>
Upstream-status: v6 pull request https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg01841.html
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
block/nbd-client.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 1e2952f..43e0292 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -114,6 +114,10 @@ static int nbd_co_send_request(BlockDriverState *bs,
int rc, ret, i;
qemu_co_mutex_lock(&s->send_mutex);
+ while (s->in_flight == MAX_NBD_REQUESTS) {
+ qemu_co_queue_wait(&s->free_sema, &s->send_mutex);
+ }
+ s->in_flight++;
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
if (s->recv_coroutine[i] == NULL) {
@@ -176,20 +180,6 @@ static void nbd_co_receive_reply(NBDClientSession *s,
}
}
-static void nbd_coroutine_start(NBDClientSession *s,
- NBDRequest *request)
-{
- /* Poor man semaphore. The free_sema is locked when no other request
- * can be accepted, and unlocked after receiving one reply. */
- if (s->in_flight == MAX_NBD_REQUESTS) {
- qemu_co_queue_wait(&s->free_sema, NULL);
- assert(s->in_flight < MAX_NBD_REQUESTS);
- }
- s->in_flight++;
-
- /* s->recv_coroutine[i] is set as soon as we get the send_lock. */
-}
-
static void nbd_coroutine_end(BlockDriverState *bs,
NBDRequest *request)
{
@@ -197,13 +187,16 @@ static void nbd_coroutine_end(BlockDriverState *bs,
int i = HANDLE_TO_INDEX(s, request->handle);
s->recv_coroutine[i] = NULL;
- s->in_flight--;
- qemu_co_queue_next(&s->free_sema);
/* Kick the read_reply_co to get the next reply. */
if (s->read_reply_co) {
aio_co_wake(s->read_reply_co);
}
+
+ qemu_co_mutex_lock(&s->send_mutex);
+ s->in_flight--;
+ qemu_co_queue_next(&s->free_sema);
+ qemu_co_mutex_unlock(&s->send_mutex);
}
int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
@@ -221,7 +214,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
assert(bytes <= NBD_MAX_BUFFER_SIZE);
assert(!flags);
- nbd_coroutine_start(client, &request);
ret = nbd_co_send_request(bs, &request, NULL);
if (ret < 0) {
reply.error = -ret;
@@ -251,7 +243,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
assert(bytes <= NBD_MAX_BUFFER_SIZE);
- nbd_coroutine_start(client, &request);
ret = nbd_co_send_request(bs, &request, qiov);
if (ret < 0) {
reply.error = -ret;
@@ -286,7 +277,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
request.flags |= NBD_CMD_FLAG_NO_HOLE;
}
- nbd_coroutine_start(client, &request);
ret = nbd_co_send_request(bs, &request, NULL);
if (ret < 0) {
reply.error = -ret;
@@ -311,7 +301,6 @@ int nbd_client_co_flush(BlockDriverState *bs)
request.from = 0;
request.len = 0;
- nbd_coroutine_start(client, &request);
ret = nbd_co_send_request(bs, &request, NULL);
if (ret < 0) {
reply.error = -ret;
@@ -337,7 +326,6 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
return 0;
}
- nbd_coroutine_start(client, &request);
ret = nbd_co_send_request(bs, &request, NULL);
if (ret < 0) {
reply.error = -ret;
--
1.8.3.1

View File

@ -0,0 +1,66 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 19 Jul 2017 18:01:59 +0200
Subject: [PATCH] nbd: strict nbd_wr_syncv
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170719180202.23329-2-eblake@redhat.com>
Patchwork-id: 75817
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 1/4] nbd: strict nbd_wr_syncv
Bugzilla: 1467509
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
nbd_wr_syncv is called either from coroutine or from client negotiation
code, when socket is in blocking mode. So, -EAGAIN is impossible.
Furthermore, EAGAIN is confusing, as, what to read/write again? With
EAGAIN as a return code we don't know how much data is already
read or written by the function, so in case of EAGAIN the whole
communication is broken.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170516094533.6160-2-vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit f250a42ddaee042ad2eb02022a3ebd18fcf987de)
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
nbd/common.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/nbd/common.c b/nbd/common.c
index dccbb8e..4db45b3 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -20,6 +20,10 @@
#include "qapi/error.h"
#include "nbd-internal.h"
+/* nbd_wr_syncv
+ * The function may be called from coroutine or from non-coroutine context.
+ * When called from non-coroutine context @ioc must be in blocking mode.
+ */
ssize_t nbd_wr_syncv(QIOChannel *ioc,
struct iovec *iov,
size_t niov,
@@ -42,11 +46,8 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
len = qio_channel_writev(ioc, local_iov, nlocal_iov, &local_err);
}
if (len == QIO_CHANNEL_ERR_BLOCK) {
- if (qemu_in_coroutine()) {
- qio_channel_yield(ioc, do_read ? G_IO_IN : G_IO_OUT);
- } else {
- return -EAGAIN;
- }
+ assert(qemu_in_coroutine());
+ qio_channel_yield(ioc, do_read ? G_IO_IN : G_IO_OUT);
continue;
}
if (len < 0) {
--
1.8.3.1

View File

@ -0,0 +1,620 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 19 Jul 2017 18:02:00 +0200
Subject: [PATCH] nbd: read_sync and friends: return 0 on success
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170719180202.23329-3-eblake@redhat.com>
Patchwork-id: 75818
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 2/4] nbd: read_sync and friends: return 0 on success
Bugzilla: 1467509
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
functions read_sync, drop_sync, write_sync, and also
nbd_negotiate_write, nbd_negotiate_read, nbd_negotiate_drop_sync
returns number of processed bytes. But what this number can be,
except requested number of bytes?
Actually, underlying nbd_wr_syncv function returns a value >= 0 and
!= requested_bytes only on eof on read operation. So, firstly, it is
impossible on write (let's add an assert) and on read it actually
means, that communication is broken (except nbd_receive_reply, see
below).
Most of callers operate like this:
if (func(..., size) != size) {
/* error path */
}
, i.e.:
1. They are not interested in partial success
2. Extra duplications in code (especially bad are duplications of
magic numbers)
3. User doesn't see actual error message, as return code is lost.
(this patch doesn't fix this point, but it makes fixing easier)
Several callers handles ret >= 0 and != requested-size separately, by
just returning EINVAL in this case. This patch makes read_sync and
friends return EINVAL in this case, so final behavior is the same.
And only one caller - nbd_receive_reply() does something not so
obvious. It returns EINVAL for ret > 0 and != requested-size, like
previous group, but for ret == 0 it returns 0. The only caller of
nbd_receive_reply() - nbd_read_reply_entry() handles ret == 0 in the
same way as ret < 0, so for now it doesn't matter. However, in
following commits error path handling will be improved and we'll need
to distinguish success from fail in this case too. So, this patch adds
separate helper for this case - read_sync_eof.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170516094533.6160-3-vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit f5d406fe86bb28da85824b6581e58980cc1a07f3)
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
nbd/client.c | 63 ++++++++++++++++------------------------
nbd/nbd-internal.h | 34 +++++++++++++++++++---
nbd/server.c | 84 +++++++++++++++++++++---------------------------------
3 files changed, 88 insertions(+), 93 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index a58fb02..6b74a62 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -86,9 +86,9 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
*/
-/* Discard length bytes from channel. Return -errno on failure, or
- * the amount of bytes consumed. */
-static ssize_t drop_sync(QIOChannel *ioc, size_t size)
+/* Discard length bytes from channel. Return -errno on failure and 0 on
+ * success*/
+static int drop_sync(QIOChannel *ioc, size_t size)
{
ssize_t ret = 0;
char small[1024];
@@ -96,14 +96,13 @@ static ssize_t drop_sync(QIOChannel *ioc, size_t size)
buffer = sizeof(small) >= size ? small : g_malloc(MIN(65536, size));
while (size > 0) {
- ssize_t count = read_sync(ioc, buffer, MIN(65536, size));
+ ssize_t count = MIN(65536, size);
+ ret = read_sync(ioc, buffer, MIN(65536, size));
- if (count <= 0) {
+ if (ret < 0) {
goto cleanup;
}
- assert(count <= size);
size -= count;
- ret += count;
}
cleanup:
@@ -136,12 +135,12 @@ static int nbd_send_option_request(QIOChannel *ioc, uint32_t opt,
stl_be_p(&req.option, opt);
stl_be_p(&req.length, len);
- if (write_sync(ioc, &req, sizeof(req)) != sizeof(req)) {
+ if (write_sync(ioc, &req, sizeof(req)) < 0) {
error_setg(errp, "Failed to send option request header");
return -1;
}
- if (len && write_sync(ioc, (char *) data, len) != len) {
+ if (len && write_sync(ioc, (char *) data, len) < 0) {
error_setg(errp, "Failed to send option request data");
return -1;
}
@@ -170,7 +169,7 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
nbd_opt_reply *reply, Error **errp)
{
QEMU_BUILD_BUG_ON(sizeof(*reply) != 20);
- if (read_sync(ioc, reply, sizeof(*reply)) != sizeof(*reply)) {
+ if (read_sync(ioc, reply, sizeof(*reply)) < 0) {
error_setg(errp, "failed to read option reply");
nbd_send_opt_abort(ioc);
return -1;
@@ -219,7 +218,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
goto cleanup;
}
msg = g_malloc(reply->length + 1);
- if (read_sync(ioc, msg, reply->length) != reply->length) {
+ if (read_sync(ioc, msg, reply->length) < 0) {
error_setg(errp, "failed to read option error message");
goto cleanup;
}
@@ -321,7 +320,7 @@ static int nbd_receive_list(QIOChannel *ioc, const char *want, bool *match,
nbd_send_opt_abort(ioc);
return -1;
}
- if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
+ if (read_sync(ioc, &namelen, sizeof(namelen)) < 0) {
error_setg(errp, "failed to read option name length");
nbd_send_opt_abort(ioc);
return -1;
@@ -334,7 +333,7 @@ static int nbd_receive_list(QIOChannel *ioc, const char *want, bool *match,
return -1;
}
if (namelen != strlen(want)) {
- if (drop_sync(ioc, len) != len) {
+ if (drop_sync(ioc, len) < 0) {
error_setg(errp, "failed to skip export name with wrong length");
nbd_send_opt_abort(ioc);
return -1;
@@ -343,14 +342,14 @@ static int nbd_receive_list(QIOChannel *ioc, const char *want, bool *match,
}
assert(namelen < sizeof(name));
- if (read_sync(ioc, name, namelen) != namelen) {
+ if (read_sync(ioc, name, namelen) < 0) {
error_setg(errp, "failed to read export name");
nbd_send_opt_abort(ioc);
return -1;
}
name[namelen] = '\0';
len -= namelen;
- if (drop_sync(ioc, len) != len) {
+ if (drop_sync(ioc, len) < 0) {
error_setg(errp, "failed to read export description");
nbd_send_opt_abort(ioc);
return -1;
@@ -477,7 +476,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
- if (read_sync(ioc, buf, 8) != 8) {
+ if (read_sync(ioc, buf, 8) < 0) {
error_setg(errp, "Failed to read data");
goto fail;
}
@@ -503,7 +502,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
- if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
+ if (read_sync(ioc, &magic, sizeof(magic)) < 0) {
error_setg(errp, "Failed to read magic");
goto fail;
}
@@ -515,8 +514,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
uint16_t globalflags;
bool fixedNewStyle = false;
- if (read_sync(ioc, &globalflags, sizeof(globalflags)) !=
- sizeof(globalflags)) {
+ if (read_sync(ioc, &globalflags, sizeof(globalflags)) < 0) {
error_setg(errp, "Failed to read server flags");
goto fail;
}
@@ -534,8 +532,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
/* client requested flags */
clientflags = cpu_to_be32(clientflags);
- if (write_sync(ioc, &clientflags, sizeof(clientflags)) !=
- sizeof(clientflags)) {
+ if (write_sync(ioc, &clientflags, sizeof(clientflags)) < 0) {
error_setg(errp, "Failed to send clientflags field");
goto fail;
}
@@ -573,13 +570,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
/* Read the response */
- if (read_sync(ioc, &s, sizeof(s)) != sizeof(s)) {
+ if (read_sync(ioc, &s, sizeof(s)) < 0) {
error_setg(errp, "Failed to read export length");
goto fail;
}
*size = be64_to_cpu(s);
- if (read_sync(ioc, flags, sizeof(*flags)) != sizeof(*flags)) {
+ if (read_sync(ioc, flags, sizeof(*flags)) < 0) {
error_setg(errp, "Failed to read export flags");
goto fail;
}
@@ -596,14 +593,14 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
- if (read_sync(ioc, &s, sizeof(s)) != sizeof(s)) {
+ if (read_sync(ioc, &s, sizeof(s)) < 0) {
error_setg(errp, "Failed to read export length");
goto fail;
}
*size = be64_to_cpu(s);
TRACE("Size is %" PRIu64, *size);
- if (read_sync(ioc, &oldflags, sizeof(oldflags)) != sizeof(oldflags)) {
+ if (read_sync(ioc, &oldflags, sizeof(oldflags)) < 0) {
error_setg(errp, "Failed to read export flags");
goto fail;
}
@@ -619,7 +616,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
- if (zeroes && drop_sync(ioc, 124) != 124) {
+ if (zeroes && drop_sync(ioc, 124) < 0) {
error_setg(errp, "Failed to read reserved block");
goto fail;
}
@@ -744,7 +741,6 @@ int nbd_disconnect(int fd)
ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request)
{
uint8_t buf[NBD_REQUEST_SIZE];
- ssize_t ret;
TRACE("Sending request to server: "
"{ .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64
@@ -759,16 +755,7 @@ ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request)
stq_be_p(buf + 16, request->from);
stl_be_p(buf + 24, request->len);
- ret = write_sync(ioc, buf, sizeof(buf));
- if (ret < 0) {
- return ret;
- }
-
- if (ret != sizeof(buf)) {
- LOG("writing to socket failed");
- return -EINVAL;
- }
- return 0;
+ return write_sync(ioc, buf, sizeof(buf));
}
ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply)
@@ -777,7 +764,7 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply)
uint32_t magic;
ssize_t ret;
- ret = read_sync(ioc, buf, sizeof(buf));
+ ret = read_sync_eof(ioc, buf, sizeof(buf));
if (ret <= 0) {
return ret;
}
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index f43d990..e6bbc7c 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -94,7 +94,13 @@
#define NBD_ENOSPC 28
#define NBD_ESHUTDOWN 108
-static inline ssize_t read_sync(QIOChannel *ioc, void *buffer, size_t size)
+/* read_sync_eof
+ * Tries to read @size bytes from @ioc. Returns number of bytes actually read.
+ * May return a value >= 0 and < size only on EOF, i.e. when iteratively called
+ * qio_channel_readv() returns 0. So, there are no needs to call read_sync_eof
+ * iteratively.
+ */
+static inline ssize_t read_sync_eof(QIOChannel *ioc, void *buffer, size_t size)
{
struct iovec iov = { .iov_base = buffer, .iov_len = size };
/* Sockets are kept in blocking mode in the negotiation phase. After
@@ -105,12 +111,32 @@ static inline ssize_t read_sync(QIOChannel *ioc, void *buffer, size_t size)
return nbd_wr_syncv(ioc, &iov, 1, size, true);
}
-static inline ssize_t write_sync(QIOChannel *ioc, const void *buffer,
- size_t size)
+/* read_sync
+ * Reads @size bytes from @ioc. Returns 0 on success.
+ */
+static inline int read_sync(QIOChannel *ioc, void *buffer, size_t size)
+{
+ ssize_t ret = read_sync_eof(ioc, buffer, size);
+
+ if (ret >= 0 && ret != size) {
+ ret = -EINVAL;
+ }
+
+ return ret < 0 ? ret : 0;
+}
+
+/* write_sync
+ * Writes @size bytes to @ioc. Returns 0 on success.
+ */
+static inline int write_sync(QIOChannel *ioc, const void *buffer, size_t size)
{
struct iovec iov = { .iov_base = (void *) buffer, .iov_len = size };
- return nbd_wr_syncv(ioc, &iov, 1, size, false);
+ ssize_t ret = nbd_wr_syncv(ioc, &iov, 1, size, false);
+
+ assert(ret < 0 || ret == size);
+
+ return ret < 0 ? ret : 0;
}
struct NBDTLSHandshakeData {
diff --git a/nbd/server.c b/nbd/server.c
index a98bb21..b44cbe6 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -112,7 +112,7 @@ static gboolean nbd_negotiate_continue(QIOChannel *ioc,
return TRUE;
}
-static ssize_t nbd_negotiate_read(QIOChannel *ioc, void *buffer, size_t size)
+static int nbd_negotiate_read(QIOChannel *ioc, void *buffer, size_t size)
{
ssize_t ret;
guint watch;
@@ -130,8 +130,7 @@ static ssize_t nbd_negotiate_read(QIOChannel *ioc, void *buffer, size_t size)
}
-static ssize_t nbd_negotiate_write(QIOChannel *ioc, const void *buffer,
- size_t size)
+static int nbd_negotiate_write(QIOChannel *ioc, const void *buffer, size_t size)
{
ssize_t ret;
guint watch;
@@ -148,24 +147,24 @@ static ssize_t nbd_negotiate_write(QIOChannel *ioc, const void *buffer,
return ret;
}
-static ssize_t nbd_negotiate_drop_sync(QIOChannel *ioc, size_t size)
+static int nbd_negotiate_drop_sync(QIOChannel *ioc, size_t size)
{
- ssize_t ret, dropped = size;
+ ssize_t ret;
uint8_t *buffer = g_malloc(MIN(65536, size));
while (size > 0) {
- ret = nbd_negotiate_read(ioc, buffer, MIN(65536, size));
+ size_t count = MIN(65536, size);
+ ret = nbd_negotiate_read(ioc, buffer, count);
if (ret < 0) {
g_free(buffer);
return ret;
}
- assert(ret <= size);
- size -= ret;
+ size -= count;
}
g_free(buffer);
- return dropped;
+ return 0;
}
/* Basic flow for negotiation
@@ -206,22 +205,22 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
type, opt, len);
magic = cpu_to_be64(NBD_REP_MAGIC);
- if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
+ if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) < 0) {
LOG("write failed (rep magic)");
return -EINVAL;
}
opt = cpu_to_be32(opt);
- if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
+ if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) < 0) {
LOG("write failed (rep opt)");
return -EINVAL;
}
type = cpu_to_be32(type);
- if (nbd_negotiate_write(ioc, &type, sizeof(type)) != sizeof(type)) {
+ if (nbd_negotiate_write(ioc, &type, sizeof(type)) < 0) {
LOG("write failed (rep type)");
return -EINVAL;
}
len = cpu_to_be32(len);
- if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
+ if (nbd_negotiate_write(ioc, &len, sizeof(len)) < 0) {
LOG("write failed (rep data length)");
return -EINVAL;
}
@@ -256,7 +255,7 @@ nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,
if (ret < 0) {
goto out;
}
- if (nbd_negotiate_write(ioc, msg, len) != len) {
+ if (nbd_negotiate_write(ioc, msg, len) < 0) {
LOG("write failed (error message)");
ret = -EIO;
} else {
@@ -287,15 +286,15 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
}
len = cpu_to_be32(name_len);
- if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
+ if (nbd_negotiate_write(ioc, &len, sizeof(len)) < 0) {
LOG("write failed (name length)");
return -EINVAL;
}
- if (nbd_negotiate_write(ioc, name, name_len) != name_len) {
+ if (nbd_negotiate_write(ioc, name, name_len) < 0) {
LOG("write failed (name buffer)");
return -EINVAL;
}
- if (nbd_negotiate_write(ioc, desc, desc_len) != desc_len) {
+ if (nbd_negotiate_write(ioc, desc, desc_len) < 0) {
LOG("write failed (description buffer)");
return -EINVAL;
}
@@ -309,7 +308,7 @@ static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
NBDExport *exp;
if (length) {
- if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
+ if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
return -EIO;
}
return nbd_negotiate_send_rep_err(client->ioc,
@@ -340,7 +339,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
LOG("Bad length received");
goto fail;
}
- if (nbd_negotiate_read(client->ioc, name, length) != length) {
+ if (nbd_negotiate_read(client->ioc, name, length) < 0) {
LOG("read failed");
goto fail;
}
@@ -373,7 +372,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
TRACE("Setting up TLS");
ioc = client->ioc;
if (length) {
- if (nbd_negotiate_drop_sync(ioc, length) != length) {
+ if (nbd_negotiate_drop_sync(ioc, length) < 0) {
return NULL;
}
nbd_negotiate_send_rep_err(ioc, NBD_REP_ERR_INVALID, NBD_OPT_STARTTLS,
@@ -437,8 +436,7 @@ static int nbd_negotiate_options(NBDClient *client)
... Rest of request
*/
- if (nbd_negotiate_read(client->ioc, &flags, sizeof(flags)) !=
- sizeof(flags)) {
+ if (nbd_negotiate_read(client->ioc, &flags, sizeof(flags)) < 0) {
LOG("read failed");
return -EIO;
}
@@ -464,8 +462,7 @@ static int nbd_negotiate_options(NBDClient *client)
uint32_t clientflags, length;
uint64_t magic;
- if (nbd_negotiate_read(client->ioc, &magic, sizeof(magic)) !=
- sizeof(magic)) {
+ if (nbd_negotiate_read(client->ioc, &magic, sizeof(magic)) < 0) {
LOG("read failed");
return -EINVAL;
}
@@ -476,14 +473,14 @@ static int nbd_negotiate_options(NBDClient *client)
}
if (nbd_negotiate_read(client->ioc, &clientflags,
- sizeof(clientflags)) != sizeof(clientflags)) {
+ sizeof(clientflags)) < 0)
+ {
LOG("read failed");
return -EINVAL;
}
clientflags = be32_to_cpu(clientflags);
- if (nbd_negotiate_read(client->ioc, &length, sizeof(length)) !=
- sizeof(length)) {
+ if (nbd_negotiate_read(client->ioc, &length, sizeof(length)) < 0) {
LOG("read failed");
return -EINVAL;
}
@@ -513,7 +510,7 @@ static int nbd_negotiate_options(NBDClient *client)
return -EINVAL;
default:
- if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
+ if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
return -EIO;
}
ret = nbd_negotiate_send_rep_err(client->ioc,
@@ -551,7 +548,7 @@ static int nbd_negotiate_options(NBDClient *client)
return nbd_negotiate_handle_export_name(client, length);
case NBD_OPT_STARTTLS:
- if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
+ if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
return -EIO;
}
if (client->tlscreds) {
@@ -570,7 +567,7 @@ static int nbd_negotiate_options(NBDClient *client)
}
break;
default:
- if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
+ if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
return -EIO;
}
ret = nbd_negotiate_send_rep_err(client->ioc,
@@ -659,12 +656,12 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
TRACE("TLS cannot be enabled with oldstyle protocol");
goto fail;
}
- if (nbd_negotiate_write(client->ioc, buf, sizeof(buf)) != sizeof(buf)) {
+ if (nbd_negotiate_write(client->ioc, buf, sizeof(buf)) < 0) {
LOG("write failed");
goto fail;
}
} else {
- if (nbd_negotiate_write(client->ioc, buf, 18) != 18) {
+ if (nbd_negotiate_write(client->ioc, buf, 18) < 0) {
LOG("write failed");
goto fail;
}
@@ -679,7 +676,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
stq_be_p(buf + 18, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
len = client->no_zeroes ? 10 : sizeof(buf) - 18;
- if (nbd_negotiate_write(client->ioc, buf + 18, len) != len) {
+ if (nbd_negotiate_write(client->ioc, buf + 18, len) < 0) {
LOG("write failed");
goto fail;
}
@@ -702,11 +699,6 @@ static ssize_t nbd_receive_request(QIOChannel *ioc, NBDRequest *request)
return ret;
}
- if (ret != sizeof(buf)) {
- LOG("read failed");
- return -EINVAL;
- }
-
/* Request
[ 0 .. 3] magic (NBD_REQUEST_MAGIC)
[ 4 .. 5] flags (NBD_CMD_FLAG_FUA, ...)
@@ -737,7 +729,6 @@ static ssize_t nbd_receive_request(QIOChannel *ioc, NBDRequest *request)
static ssize_t nbd_send_reply(QIOChannel *ioc, NBDReply *reply)
{
uint8_t buf[NBD_REPLY_SIZE];
- ssize_t ret;
reply->error = system_errno_to_nbd_errno(reply->error);
@@ -754,16 +745,7 @@ static ssize_t nbd_send_reply(QIOChannel *ioc, NBDReply *reply)
stl_be_p(buf + 4, reply->error);
stq_be_p(buf + 8, reply->handle);
- ret = write_sync(ioc, buf, sizeof(buf));
- if (ret < 0) {
- return ret;
- }
-
- if (ret != sizeof(buf)) {
- LOG("writing to socket failed");
- return -EINVAL;
- }
- return 0;
+ return write_sync(ioc, buf, sizeof(buf));
}
#define MAX_NBD_REQUESTS 16
@@ -1067,7 +1049,7 @@ static ssize_t nbd_co_send_reply(NBDRequestData *req, NBDReply *reply,
rc = nbd_send_reply(client->ioc, reply);
if (rc >= 0) {
ret = write_sync(client->ioc, req->data, len);
- if (ret != len) {
+ if (ret < 0) {
rc = -EIO;
}
}
@@ -1141,7 +1123,7 @@ static ssize_t nbd_co_receive_request(NBDRequestData *req,
if (request->type == NBD_CMD_WRITE) {
TRACE("Reading %" PRIu32 " byte(s)", request->len);
- if (read_sync(client->ioc, req->data, request->len) != request->len) {
+ if (read_sync(client->ioc, req->data, request->len) < 0) {
LOG("reading from socket failed");
rc = -EIO;
goto out;
--
1.8.3.1

View File

@ -0,0 +1,151 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 19 Jul 2017 18:02:01 +0200
Subject: [PATCH] nbd: make nbd_drop public
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170719180202.23329-4-eblake@redhat.com>
Patchwork-id: 75814
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 3/4] nbd: make nbd_drop public
Bugzilla: 1467509
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Following commit will reuse it for nbd server too.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170602150150.258222-3-vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 44298024d30ad36439707b89715a76333f58791b)
Conflicts:
nbd/client.c, nbd/nbd_internal.h, nbd/common.c - missing errp
addition (e44ed99) and bulk rename (d1fdf25)
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
nbd/client.c | 32 +++-----------------------------
nbd/common.c | 26 ++++++++++++++++++++++++++
nbd/nbd-internal.h | 2 ++
3 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 6b74a62..1652f28 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -86,32 +86,6 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
*/
-/* Discard length bytes from channel. Return -errno on failure and 0 on
- * success*/
-static int drop_sync(QIOChannel *ioc, size_t size)
-{
- ssize_t ret = 0;
- char small[1024];
- char *buffer;
-
- buffer = sizeof(small) >= size ? small : g_malloc(MIN(65536, size));
- while (size > 0) {
- ssize_t count = MIN(65536, size);
- ret = read_sync(ioc, buffer, MIN(65536, size));
-
- if (ret < 0) {
- goto cleanup;
- }
- size -= count;
- }
-
- cleanup:
- if (buffer != small) {
- g_free(buffer);
- }
- return ret;
-}
-
/* Send an option request.
*
* The request is for option @opt, with @data containing @len bytes of
@@ -333,7 +307,7 @@ static int nbd_receive_list(QIOChannel *ioc, const char *want, bool *match,
return -1;
}
if (namelen != strlen(want)) {
- if (drop_sync(ioc, len) < 0) {
+ if (nbd_drop(ioc, len) < 0) {
error_setg(errp, "failed to skip export name with wrong length");
nbd_send_opt_abort(ioc);
return -1;
@@ -349,7 +323,7 @@ static int nbd_receive_list(QIOChannel *ioc, const char *want, bool *match,
}
name[namelen] = '\0';
len -= namelen;
- if (drop_sync(ioc, len) < 0) {
+ if (nbd_drop(ioc, len) < 0) {
error_setg(errp, "failed to read export description");
nbd_send_opt_abort(ioc);
return -1;
@@ -616,7 +590,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
- if (zeroes && drop_sync(ioc, 124) < 0) {
+ if (zeroes && nbd_drop(ioc, 124) < 0) {
error_setg(errp, "Failed to read reserved block");
goto fail;
}
diff --git a/nbd/common.c b/nbd/common.c
index 4db45b3..9a54010 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -71,6 +71,32 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
return done;
}
+/* Discard length bytes from channel. Return -errno on failure and 0 on
+ * success */
+int nbd_drop(QIOChannel *ioc, size_t size)
+{
+ ssize_t ret = 0;
+ char small[1024];
+ char *buffer;
+
+ buffer = sizeof(small) >= size ? small : g_malloc(MIN(65536, size));
+ while (size > 0) {
+ ssize_t count = MIN(65536, size);
+ ret = read_sync(ioc, buffer, MIN(65536, size));
+
+ if (ret < 0) {
+ goto cleanup;
+ }
+ size -= count;
+ }
+
+ cleanup:
+ if (buffer != small) {
+ g_free(buffer);
+ }
+ return ret;
+}
+
void nbd_tls_handshake(QIOTask *task,
void *opaque)
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index e6bbc7c..c02378c 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -149,4 +149,6 @@ struct NBDTLSHandshakeData {
void nbd_tls_handshake(QIOTask *task,
void *opaque);
+int nbd_drop(QIOChannel *ioc, size_t size);
+
#endif
--
1.8.3.1

View File

@ -0,0 +1,292 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 19 Jul 2017 18:02:02 +0200
Subject: [PATCH] nbd/server: get rid of nbd_negotiate_read and friends
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170719180202.23329-5-eblake@redhat.com>
Patchwork-id: 75816
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 4/4] nbd/server: get rid of nbd_negotiate_read and friends
Bugzilla: 1467509
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Functions nbd_negotiate_{read,write,drop_sync} were introduced in
1a6245a5b, when nbd_rwv (was nbd_wr_sync) was working through
qemu_co_sendv_recvv (the path is nbd_wr_sync -> qemu_co_{recv/send} ->
qemu_co_send_recv -> qemu_co_sendv_recvv), which just yields, without
setting any handlers. But starting from ff82911cd nbd_rwv (was
nbd_wr_syncv) works through qio_channel_yield() which sets handlers, so
watchers are redundant in nbd_negotiate_{read,write,drop_sync}, then,
let's just use nbd_{read,write,drop} functions.
Functions nbd_{read,write,drop} has errp parameter, which is unused in
this patch. This will be fixed later.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170602150150.258222-4-vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 2b0bbc4f8809c972bad134bc1a2570dbb01dea0b)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
Conflicts:
nbd/server.c - missing errp addition (e44ed99) and bulk
rename (d1fdf25)
Fixes CVE-2017-7539
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 106 ++++++++++++-----------------------------------------------
1 file changed, 21 insertions(+), 85 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index b44cbe6..8e3b8e5 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -104,69 +104,6 @@ struct NBDClient {
static void nbd_client_receive_next_request(NBDClient *client);
-static gboolean nbd_negotiate_continue(QIOChannel *ioc,
- GIOCondition condition,
- void *opaque)
-{
- qemu_coroutine_enter(opaque);
- return TRUE;
-}
-
-static int nbd_negotiate_read(QIOChannel *ioc, void *buffer, size_t size)
-{
- ssize_t ret;
- guint watch;
-
- assert(qemu_in_coroutine());
- /* Negotiation are always in main loop. */
- watch = qio_channel_add_watch(ioc,
- G_IO_IN,
- nbd_negotiate_continue,
- qemu_coroutine_self(),
- NULL);
- ret = read_sync(ioc, buffer, size);
- g_source_remove(watch);
- return ret;
-
-}
-
-static int nbd_negotiate_write(QIOChannel *ioc, const void *buffer, size_t size)
-{
- ssize_t ret;
- guint watch;
-
- assert(qemu_in_coroutine());
- /* Negotiation are always in main loop. */
- watch = qio_channel_add_watch(ioc,
- G_IO_OUT,
- nbd_negotiate_continue,
- qemu_coroutine_self(),
- NULL);
- ret = write_sync(ioc, buffer, size);
- g_source_remove(watch);
- return ret;
-}
-
-static int nbd_negotiate_drop_sync(QIOChannel *ioc, size_t size)
-{
- ssize_t ret;
- uint8_t *buffer = g_malloc(MIN(65536, size));
-
- while (size > 0) {
- size_t count = MIN(65536, size);
- ret = nbd_negotiate_read(ioc, buffer, count);
- if (ret < 0) {
- g_free(buffer);
- return ret;
- }
-
- size -= count;
- }
-
- g_free(buffer);
- return 0;
-}
-
/* Basic flow for negotiation
Server Client
@@ -205,22 +142,22 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
type, opt, len);
magic = cpu_to_be64(NBD_REP_MAGIC);
- if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) < 0) {
+ if (write_sync(ioc, &magic, sizeof(magic)) < 0) {
LOG("write failed (rep magic)");
return -EINVAL;
}
opt = cpu_to_be32(opt);
- if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) < 0) {
+ if (write_sync(ioc, &opt, sizeof(opt)) < 0) {
LOG("write failed (rep opt)");
return -EINVAL;
}
type = cpu_to_be32(type);
- if (nbd_negotiate_write(ioc, &type, sizeof(type)) < 0) {
+ if (write_sync(ioc, &type, sizeof(type)) < 0) {
LOG("write failed (rep type)");
return -EINVAL;
}
len = cpu_to_be32(len);
- if (nbd_negotiate_write(ioc, &len, sizeof(len)) < 0) {
+ if (write_sync(ioc, &len, sizeof(len)) < 0) {
LOG("write failed (rep data length)");
return -EINVAL;
}
@@ -255,7 +192,7 @@ nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,
if (ret < 0) {
goto out;
}
- if (nbd_negotiate_write(ioc, msg, len) < 0) {
+ if (write_sync(ioc, msg, len) < 0) {
LOG("write failed (error message)");
ret = -EIO;
} else {
@@ -286,15 +223,15 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
}
len = cpu_to_be32(name_len);
- if (nbd_negotiate_write(ioc, &len, sizeof(len)) < 0) {
+ if (write_sync(ioc, &len, sizeof(len)) < 0) {
LOG("write failed (name length)");
return -EINVAL;
}
- if (nbd_negotiate_write(ioc, name, name_len) < 0) {
+ if (write_sync(ioc, name, name_len) < 0) {
LOG("write failed (name buffer)");
return -EINVAL;
}
- if (nbd_negotiate_write(ioc, desc, desc_len) < 0) {
+ if (write_sync(ioc, desc, desc_len) < 0) {
LOG("write failed (description buffer)");
return -EINVAL;
}
@@ -308,7 +245,7 @@ static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
NBDExport *exp;
if (length) {
- if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
+ if (nbd_drop(client->ioc, length) < 0) {
return -EIO;
}
return nbd_negotiate_send_rep_err(client->ioc,
@@ -339,7 +276,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
LOG("Bad length received");
goto fail;
}
- if (nbd_negotiate_read(client->ioc, name, length) < 0) {
+ if (read_sync(client->ioc, name, length) < 0) {
LOG("read failed");
goto fail;
}
@@ -372,7 +309,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
TRACE("Setting up TLS");
ioc = client->ioc;
if (length) {
- if (nbd_negotiate_drop_sync(ioc, length) < 0) {
+ if (nbd_drop(ioc, length) < 0) {
return NULL;
}
nbd_negotiate_send_rep_err(ioc, NBD_REP_ERR_INVALID, NBD_OPT_STARTTLS,
@@ -436,7 +373,7 @@ static int nbd_negotiate_options(NBDClient *client)
... Rest of request
*/
- if (nbd_negotiate_read(client->ioc, &flags, sizeof(flags)) < 0) {
+ if (read_sync(client->ioc, &flags, sizeof(flags)) < 0) {
LOG("read failed");
return -EIO;
}
@@ -462,7 +399,7 @@ static int nbd_negotiate_options(NBDClient *client)
uint32_t clientflags, length;
uint64_t magic;
- if (nbd_negotiate_read(client->ioc, &magic, sizeof(magic)) < 0) {
+ if (read_sync(client->ioc, &magic, sizeof(magic)) < 0) {
LOG("read failed");
return -EINVAL;
}
@@ -472,15 +409,14 @@ static int nbd_negotiate_options(NBDClient *client)
return -EINVAL;
}
- if (nbd_negotiate_read(client->ioc, &clientflags,
- sizeof(clientflags)) < 0)
+ if (read_sync(client->ioc, &clientflags, sizeof(clientflags)) < 0)
{
LOG("read failed");
return -EINVAL;
}
clientflags = be32_to_cpu(clientflags);
- if (nbd_negotiate_read(client->ioc, &length, sizeof(length)) < 0) {
+ if (read_sync(client->ioc, &length, sizeof(length)) < 0) {
LOG("read failed");
return -EINVAL;
}
@@ -510,7 +446,7 @@ static int nbd_negotiate_options(NBDClient *client)
return -EINVAL;
default:
- if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
+ if (nbd_drop(client->ioc, length) < 0) {
return -EIO;
}
ret = nbd_negotiate_send_rep_err(client->ioc,
@@ -548,7 +484,7 @@ static int nbd_negotiate_options(NBDClient *client)
return nbd_negotiate_handle_export_name(client, length);
case NBD_OPT_STARTTLS:
- if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
+ if (nbd_drop(client->ioc, length) < 0) {
return -EIO;
}
if (client->tlscreds) {
@@ -567,7 +503,7 @@ static int nbd_negotiate_options(NBDClient *client)
}
break;
default:
- if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {
+ if (nbd_drop(client->ioc, length) < 0) {
return -EIO;
}
ret = nbd_negotiate_send_rep_err(client->ioc,
@@ -656,12 +592,12 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
TRACE("TLS cannot be enabled with oldstyle protocol");
goto fail;
}
- if (nbd_negotiate_write(client->ioc, buf, sizeof(buf)) < 0) {
+ if (write_sync(client->ioc, buf, sizeof(buf)) < 0) {
LOG("write failed");
goto fail;
}
} else {
- if (nbd_negotiate_write(client->ioc, buf, 18) < 0) {
+ if (write_sync(client->ioc, buf, 18) < 0) {
LOG("write failed");
goto fail;
}
@@ -676,7 +612,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
stq_be_p(buf + 18, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
len = client->no_zeroes ? 10 : sizeof(buf) - 18;
- if (nbd_negotiate_write(client->ioc, buf + 18, len) < 0) {
+ if (write_sync(client->ioc, buf + 18, len) < 0) {
LOG("write failed");
goto fail;
}
--
1.8.3.1

View File

@ -0,0 +1,153 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 27 Sep 2017 17:57:19 +0200
Subject: [PATCH] nbd-client: Fix regression when server sends garbage
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170927175725.20023-2-eblake@redhat.com>
Patchwork-id: 76672
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 1/7] nbd-client: Fix regression when server sends garbage
Bugzilla: 1495474
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
When we switched NBD to use coroutines for qemu 2.9 (in particular,
commit a12a712a), we introduced a regression: if a server sends us
garbage (such as a corrupted magic number), we quit the read loop
but do not stop sending further queued commands, resulting in the
client hanging when it never reads the response to those additional
commands. In qemu 2.8, we properly detected that the server is no
longer reliable, and cancelled all existing pending commands with
EIO, then tore down the socket so that all further command attempts
get EPIPE.
Restore the proper behavior of quitting (almost) all communication
with a broken server: Once we know we are out of sync or otherwise
can't trust the server, we must assume that any further incoming
data is unreliable and therefore end all pending commands with EIO,
and quit trying to send any further commands. As an exception, we
still (try to) send NBD_CMD_DISC to let the server know we are going
away (in part, because it is easier to do that than to further
refactor nbd_teardown_connection, and in part because it is the
only command where we do not have to wait for a reply).
Based on a patch by Vladimir Sementsov-Ogievskiy.
A malicious server can be created with the following hack,
followed by setting NBD_SERVER_DEBUG to a non-zero value in the
environment when running qemu-nbd:
| --- a/nbd/server.c
| +++ b/nbd/server.c
| @@ -919,6 +919,17 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
| stl_be_p(buf + 4, reply->error);
| stq_be_p(buf + 8, reply->handle);
|
| + static int debug;
| + static int count;
| + if (!count++) {
| + const char *str = getenv("NBD_SERVER_DEBUG");
| + if (str) {
| + debug = atoi(str);
| + }
| + }
| + if (debug && !(count % debug)) {
| + buf[0] = 0;
| + }
| return nbd_write(ioc, buf, sizeof(buf), errp);
| }
Reported-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170814213426.24681-1-eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit 72b6ffc76653214b69a94a7b1643ff80df134486)
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
Conflicts:
block/nbd-client.c
---
block/nbd-client.c | 17 +++++++++++++----
block/nbd-client.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 282679b..701b4ce 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -71,7 +71,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
uint64_t i;
int ret;
- for (;;) {
+ while (!s->quit) {
assert(s->reply.handle == 0);
ret = nbd_receive_reply(s->ioc, &s->reply);
if (ret <= 0) {
@@ -102,6 +102,9 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
qemu_coroutine_yield();
}
+ if (ret < 0) {
+ s->quit = true;
+ }
nbd_recv_coroutines_enter_all(s);
s->read_reply_co = NULL;
}
@@ -130,6 +133,10 @@ static int nbd_co_send_request(BlockDriverState *bs,
assert(i < MAX_NBD_REQUESTS);
request->handle = INDEX_TO_HANDLE(s, i);
+ if (s->quit) {
+ qemu_co_mutex_unlock(&s->send_mutex);
+ return -EIO;
+ }
if (!s->ioc) {
qemu_co_mutex_unlock(&s->send_mutex);
return -EPIPE;
@@ -138,7 +145,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
if (qiov) {
qio_channel_set_cork(s->ioc, true);
rc = nbd_send_request(s->ioc, request);
- if (rc >= 0) {
+ if (rc >= 0 && !s->quit) {
ret = nbd_wr_syncv(s->ioc, qiov->iov, qiov->niov, request->len,
false);
if (ret != request->len) {
@@ -149,6 +156,9 @@ static int nbd_co_send_request(BlockDriverState *bs,
} else {
rc = nbd_send_request(s->ioc, request);
}
+ if (rc < 0) {
+ s->quit = true;
+ }
qemu_co_mutex_unlock(&s->send_mutex);
return rc;
}
@@ -163,8 +173,7 @@ static void nbd_co_receive_reply(NBDClientSession *s,
/* Wait until we're woken up by nbd_read_reply_entry. */
qemu_coroutine_yield();
*reply = s->reply;
- if (reply->handle != request->handle ||
- !s->ioc) {
+ if (reply->handle != request->handle || !s->ioc || s->quit) {
reply->error = EIO;
} else {
if (qiov && reply->error == 0) {
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 891ba44..9774a8e 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -30,6 +30,7 @@ typedef struct NBDClientSession {
Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
NBDReply reply;
+ bool quit;
} NBDClientSession;
NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
--
1.8.3.1

View File

@ -0,0 +1,55 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 27 Sep 2017 17:57:20 +0200
Subject: [PATCH] fix build failure in nbd_read_reply_entry()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170927175725.20023-3-eblake@redhat.com>
Patchwork-id: 76668
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 2/7] fix build failure in nbd_read_reply_entry()
Bugzilla: 1495474
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
From: Igor Mammedov <imammedo@redhat.com>
travis builds fail at HEAD at rc3 master with
block/nbd-client.c: In function nbd_read_reply_entry:
block/nbd-client.c:110:8: error: ret may be used uninitialized in this function [-Werror=uninitialized]
fix it by initializing 'ret' to 0
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
(cherry picked from commit d0a180131c6655487b47ea72e7da0a909a479a3c)
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
Conflicts:
block/nbd-client.c - context
---
block/nbd-client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 701b4ce..256dabe 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -69,7 +69,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
{
NBDClientSession *s = opaque;
uint64_t i;
- int ret;
+ int ret = 0;
while (!s->quit) {
assert(s->reply.handle == 0);
--
1.8.3.1

View File

@ -0,0 +1,184 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 27 Sep 2017 17:57:21 +0200
Subject: [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170927175725.20023-4-eblake@redhat.com>
Patchwork-id: 76671
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 3/7] nbd-client: avoid spurious qio_channel_yield() re-entry
Bugzilla: 1495474
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
The following scenario leads to an assertion failure in
qio_channel_yield():
1. Request coroutine calls qio_channel_yield() successfully when sending
would block on the socket. It is now yielded.
2. nbd_read_reply_entry() calls nbd_recv_coroutines_enter_all() because
nbd_receive_reply() failed.
3. Request coroutine is entered and returns from qio_channel_yield().
Note that the socket fd handler has not fired yet so
ioc->write_coroutine is still set.
4. Request coroutine attempts to send the request body with nbd_rwv()
but the socket would still block. qio_channel_yield() is called
again and assert(!ioc->write_coroutine) is hit.
The problem is that nbd_read_reply_entry() does not distinguish between
request coroutines that are waiting to receive a reply and those that
are not.
This patch adds a per-request bool receiving flag so
nbd_read_reply_entry() can avoid spurious aio_wake() calls.
Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20170822125113.5025-1-stefanha@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 40f4a21895b5a7eae4011593837069f63460d983)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
block/nbd-client.c | 35 ++++++++++++++++++++++-------------
block/nbd-client.h | 7 ++++++-
2 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 256dabe..f7bca3f 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -38,8 +38,10 @@ static void nbd_recv_coroutines_enter_all(NBDClientSession *s)
int i;
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
- if (s->recv_coroutine[i]) {
- aio_co_wake(s->recv_coroutine[i]);
+ NBDClientRequest *req = &s->requests[i];
+
+ if (req->coroutine && req->receiving) {
+ aio_co_wake(req->coroutine);
}
}
}
@@ -83,28 +85,28 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
* one coroutine is called until the reply finishes.
*/
i = HANDLE_TO_INDEX(s, s->reply.handle);
- if (i >= MAX_NBD_REQUESTS || !s->recv_coroutine[i]) {
+ if (i >= MAX_NBD_REQUESTS ||
+ !s->requests[i].coroutine ||
+ !s->requests[i].receiving) {
break;
}
- /* We're woken up by the recv_coroutine itself. Note that there
+ /* We're woken up again by the request itself. Note that there
* is no race between yielding and reentering read_reply_co. This
* is because:
*
- * - if recv_coroutine[i] runs on the same AioContext, it is only
+ * - if the request runs on the same AioContext, it is only
* entered after we yield
*
- * - if recv_coroutine[i] runs on a different AioContext, reentering
+ * - if the request runs on a different AioContext, reentering
* read_reply_co happens through a bottom half, which can only
* run after we yield.
*/
- aio_co_wake(s->recv_coroutine[i]);
+ aio_co_wake(s->requests[i].coroutine);
qemu_coroutine_yield();
}
- if (ret < 0) {
- s->quit = true;
- }
+ s->quit = true;
nbd_recv_coroutines_enter_all(s);
s->read_reply_co = NULL;
}
@@ -123,14 +125,17 @@ static int nbd_co_send_request(BlockDriverState *bs,
s->in_flight++;
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
- if (s->recv_coroutine[i] == NULL) {
- s->recv_coroutine[i] = qemu_coroutine_self();
+ if (s->requests[i].coroutine == NULL) {
break;
}
}
g_assert(qemu_in_coroutine());
assert(i < MAX_NBD_REQUESTS);
+
+ s->requests[i].coroutine = qemu_coroutine_self();
+ s->requests[i].receiving = false;
+
request->handle = INDEX_TO_HANDLE(s, i);
if (s->quit) {
@@ -168,10 +173,13 @@ static void nbd_co_receive_reply(NBDClientSession *s,
NBDReply *reply,
QEMUIOVector *qiov)
{
+ int i = HANDLE_TO_INDEX(s, request->handle);
int ret;
/* Wait until we're woken up by nbd_read_reply_entry. */
+ s->requests[i].receiving = true;
qemu_coroutine_yield();
+ s->requests[i].receiving = false;
*reply = s->reply;
if (reply->handle != request->handle || !s->ioc || s->quit) {
reply->error = EIO;
@@ -181,6 +189,7 @@ static void nbd_co_receive_reply(NBDClientSession *s,
true);
if (ret != request->len) {
reply->error = EIO;
+ s->quit = true;
}
}
@@ -195,7 +204,7 @@ static void nbd_coroutine_end(BlockDriverState *bs,
NBDClientSession *s = nbd_get_client_session(bs);
int i = HANDLE_TO_INDEX(s, request->handle);
- s->recv_coroutine[i] = NULL;
+ s->requests[i].coroutine = NULL;
/* Kick the read_reply_co to get the next reply. */
if (s->read_reply_co) {
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 9774a8e..f97792f 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -17,6 +17,11 @@
#define MAX_NBD_REQUESTS 16
+typedef struct {
+ Coroutine *coroutine;
+ bool receiving; /* waiting for read_reply_co? */
+} NBDClientRequest;
+
typedef struct NBDClientSession {
QIOChannelSocket *sioc; /* The master data channel */
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
@@ -28,7 +33,7 @@ typedef struct NBDClientSession {
Coroutine *read_reply_co;
int in_flight;
- Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
+ NBDClientRequest requests[MAX_NBD_REQUESTS];
NBDReply reply;
bool quit;
} NBDClientSession;
--
1.8.3.1

View File

@ -0,0 +1,160 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 27 Sep 2017 17:57:22 +0200
Subject: [PATCH] nbd-client: avoid read_reply_co entry if send failed
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170927175725.20023-5-eblake@redhat.com>
Patchwork-id: 76674
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 4/7] nbd-client: avoid read_reply_co entry if send failed
Bugzilla: 1495474
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
The following segfault is encountered if the NBD server closes the UNIX
domain socket immediately after negotiation:
Program terminated with signal SIGSEGV, Segmentation fault.
#0 aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441
441 QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
(gdb) bt
#0 0x000000d3c01a50f8 in aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441
#1 0x000000d3c012fa90 in nbd_coroutine_end (bs=bs@entry=0xd3c0fec650, request=<optimized out>) at block/nbd-client.c:207
#2 0x000000d3c012fb58 in nbd_client_co_preadv (bs=0xd3c0fec650, offset=0, bytes=<optimized out>, qiov=0x7ffc10a91b20, flags=0) at block/nbd-client.c:237
#3 0x000000d3c0128e63 in bdrv_driver_preadv (bs=bs@entry=0xd3c0fec650, offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, flags=0) at block/io.c:836
#4 0x000000d3c012c3e0 in bdrv_aligned_preadv (child=child@entry=0xd3c0ff51d0, req=req@entry=0x7f31885d6e90, offset=offset@entry=0, bytes=bytes@entry=512, align=align@entry=1, qiov=qiov@entry=0x7ffc10a91b20, f
+lags=0) at block/io.c:1086
#5 0x000000d3c012c6b8 in bdrv_co_preadv (child=0xd3c0ff51d0, offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, flags=flags@entry=0) at block/io.c:1182
#6 0x000000d3c011cc17 in blk_co_preadv (blk=0xd3c0ff4f80, offset=0, bytes=512, qiov=0x7ffc10a91b20, flags=0) at block/block-backend.c:1032
#7 0x000000d3c011ccec in blk_read_entry (opaque=0x7ffc10a91b40) at block/block-backend.c:1079
#8 0x000000d3c01bbb96 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at util/coroutine-ucontext.c:79
#9 0x00007f3196cb8600 in __start_context () at /lib64/libc.so.6
The problem is that nbd_client_init() uses
nbd_client_attach_aio_context() -> aio_co_schedule(new_context,
client->read_reply_co). Execution of read_reply_co is deferred to a BH
which doesn't run until later.
In the mean time blk_co_preadv() can be called and nbd_coroutine_end()
calls aio_wake() on read_reply_co. At this point in time
read_reply_co's ctx isn't set because it has never been entered yet.
This patch simplifies the nbd_co_send_request() ->
nbd_co_receive_reply() -> nbd_coroutine_end() lifecycle to just
nbd_co_send_request() -> nbd_co_receive_reply(). The request is "ended"
if an error occurs at any point. Callers no longer have to invoke
nbd_coroutine_end().
This cleanup also eliminates the segfault because we don't call
aio_co_schedule() to wake up s->read_reply_co if sending the request
failed. It is only necessary to wake up s->read_reply_co if a reply was
received.
Note this only happens with UNIX domain sockets on Linux. It doesn't
seem possible to reproduce this with TCP sockets.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20170829122745.14309-2-stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 3c2d5183f9fa4eac3d17d841e26da65a0181ae7b)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
block/nbd-client.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index f7bca3f..434acf6 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -139,12 +139,12 @@ static int nbd_co_send_request(BlockDriverState *bs,
request->handle = INDEX_TO_HANDLE(s, i);
if (s->quit) {
- qemu_co_mutex_unlock(&s->send_mutex);
- return -EIO;
+ rc = -EIO;
+ goto err;
}
if (!s->ioc) {
- qemu_co_mutex_unlock(&s->send_mutex);
- return -EPIPE;
+ rc = -EPIPE;
+ goto err;
}
if (qiov) {
@@ -161,8 +161,13 @@ static int nbd_co_send_request(BlockDriverState *bs,
} else {
rc = nbd_send_request(s->ioc, request);
}
+
+err:
if (rc < 0) {
s->quit = true;
+ s->requests[i].coroutine = NULL;
+ s->in_flight--;
+ qemu_co_queue_next(&s->free_sema);
}
qemu_co_mutex_unlock(&s->send_mutex);
return rc;
@@ -196,13 +201,6 @@ static void nbd_co_receive_reply(NBDClientSession *s,
/* Tell the read handler to read another header. */
s->reply.handle = 0;
}
-}
-
-static void nbd_coroutine_end(BlockDriverState *bs,
- NBDRequest *request)
-{
- NBDClientSession *s = nbd_get_client_session(bs);
- int i = HANDLE_TO_INDEX(s, request->handle);
s->requests[i].coroutine = NULL;
@@ -238,7 +236,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
} else {
nbd_co_receive_reply(client, &request, &reply, qiov);
}
- nbd_coroutine_end(bs, &request);
return -reply.error;
}
@@ -267,7 +264,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
} else {
nbd_co_receive_reply(client, &request, &reply, NULL);
}
- nbd_coroutine_end(bs, &request);
return -reply.error;
}
@@ -301,7 +297,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
} else {
nbd_co_receive_reply(client, &request, &reply, NULL);
}
- nbd_coroutine_end(bs, &request);
return -reply.error;
}
@@ -325,7 +320,6 @@ int nbd_client_co_flush(BlockDriverState *bs)
} else {
nbd_co_receive_reply(client, &request, &reply, NULL);
}
- nbd_coroutine_end(bs, &request);
return -reply.error;
}
@@ -350,7 +344,6 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
} else {
nbd_co_receive_reply(client, &request, &reply, NULL);
}
- nbd_coroutine_end(bs, &request);
return -reply.error;
}
--
1.8.3.1

View File

@ -0,0 +1,61 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 27 Sep 2017 17:57:23 +0200
Subject: [PATCH] qemu-iotests: improve nbd-fault-injector.py startup
protocol
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170927175725.20023-6-eblake@redhat.com>
Patchwork-id: 76675
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 5/7] qemu-iotests: improve nbd-fault-injector.py startup protocol
Bugzilla: 1495474
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Currently 083 waits for the nbd-fault-injector.py server to start up by
looping until netstat shows the TCP listen socket.
The startup protocol can be simplified by passing a 0 port number to
nbd-fault-injector.py. The kernel will allocate a port in bind(2) and
the final port number can be printed by nbd-fault-injector.py.
This should make it slightly nicer and less TCP-specific to wait for
server startup. This patch changes nbd-fault-injector.py, the next one
will rewrite server startup in 083.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20170829122745.14309-3-stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 6e592fc92234a58c7156c385840633c17dedd24f)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
tests/qemu-iotests/nbd-fault-injector.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
index 6c07191..1c10dcb 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -235,11 +235,15 @@ def open_socket(path):
sock = socket.socket()
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
sock.bind((host, int(port)))
+
+ # If given port was 0 the final port number is now available
+ path = '%s:%d' % sock.getsockname()
else:
sock = socket.socket(socket.AF_UNIX)
sock.bind(path)
sock.listen(0)
print 'Listening on %s' % path
+ sys.stdout.flush() # another process may be waiting, show message now
return sock
def usage(args):
--
1.8.3.1

View File

@ -0,0 +1,454 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 27 Sep 2017 17:57:24 +0200
Subject: [PATCH] qemu-iotests: test NBD over UNIX domain sockets in 083
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170927175725.20023-7-eblake@redhat.com>
Patchwork-id: 76670
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 6/7] qemu-iotests: test NBD over UNIX domain sockets in 083
Bugzilla: 1495474
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
083 only tests TCP. Some failures might be specific to UNIX domain
sockets.
A few adjustments are necessary:
1. Generating a port number and waiting for server startup is
TCP-specific. Use the new nbd-fault-injector.py startup protocol to
fetch the address. This is a little more elegant because we don't
need netstat anymore.
2. The NBD filter does not work for the UNIX domain sockets URIs we
generate and must be extended.
3. Run all tests twice: once for TCP and once for UNIX domain sockets.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20170829122745.14309-4-stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 02d2d860d25e439f0e88658c701668ab684568fb)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
Conflicts:
tests/qemu-iotests/083.out - error message improvements not backported
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/083 | 136 +++++++++++++++++++++++--------------
tests/qemu-iotests/083.out | 143 ++++++++++++++++++++++++++++++++++-----
tests/qemu-iotests/common.filter | 4 +-
3 files changed, 212 insertions(+), 71 deletions(-)
diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
index bff9360..0306f11 100755
--- a/tests/qemu-iotests/083
+++ b/tests/qemu-iotests/083
@@ -27,6 +27,14 @@ echo "QA output created by $seq"
here=`pwd`
status=1 # failure is the default!
+_cleanup()
+{
+ rm -f nbd.sock
+ rm -f nbd-fault-injector.out
+ rm -f nbd-fault-injector.conf
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
# get standard environment, filters and checks
. ./common.rc
. ./common.filter
@@ -35,81 +43,105 @@ _supported_fmt generic
_supported_proto nbd
_supported_os Linux
-# Pick a TCP port based on our pid. This way multiple instances of this test
-# can run in parallel without conflicting.
-choose_tcp_port() {
- echo $((($$ % 31744) + 1024)) # 1024 <= port < 32768
-}
-
-wait_for_tcp_port() {
- while ! (netstat --tcp --listening --numeric | \
- grep "$1.*0\\.0\\.0\\.0:\\*.*LISTEN") >/dev/null 2>&1; do
- sleep 0.1
+check_disconnect() {
+ local event export_name=foo extra_args nbd_addr nbd_url proto when
+
+ while true; do
+ case $1 in
+ --classic-negotiation)
+ shift
+ extra_args=--classic-negotiation
+ export_name=
+ ;;
+ --tcp)
+ shift
+ proto=tcp
+ ;;
+ --unix)
+ shift
+ proto=unix
+ ;;
+ *)
+ break
+ ;;
+ esac
done
-}
-check_disconnect() {
event=$1
when=$2
- negotiation=$3
echo "=== Check disconnect $when $event ==="
echo
- port=$(choose_tcp_port)
-
cat > "$TEST_DIR/nbd-fault-injector.conf" <<EOF
[inject-error]
event=$event
when=$when
EOF
- if [ "$negotiation" = "--classic-negotiation" ]; then
- extra_args=--classic-negotiation
- nbd_url="nbd:127.0.0.1:$port"
+ if [ "$proto" = "tcp" ]; then
+ nbd_addr="127.0.0.1:0"
else
- nbd_url="nbd:127.0.0.1:$port:exportname=foo"
+ nbd_addr="$TEST_DIR/nbd.sock"
+ fi
+
+ rm -f "$TEST_DIR/nbd.sock"
+
+ $PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" "$TEST_DIR/nbd-fault-injector.conf" >"$TEST_DIR/nbd-fault-injector.out" 2>&1 &
+
+ # Wait for server to be ready
+ while ! grep -q 'Listening on ' "$TEST_DIR/nbd-fault-injector.out"; do
+ sleep 0.1
+ done
+
+ # Extract the final address (port number has now been assigned in tcp case)
+ nbd_addr=$(sed 's/Listening on \(.*\)$/\1/' "$TEST_DIR/nbd-fault-injector.out")
+
+ if [ "$proto" = "tcp" ]; then
+ nbd_url="nbd+tcp://$nbd_addr/$export_name"
+ else
+ nbd_url="nbd+unix:///$export_name?socket=$nbd_addr"
fi
- $PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" >/dev/null 2>&1 &
- wait_for_tcp_port "127\\.0\\.0\\.1:$port"
$QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | _filter_nbd
echo
}
-for event in neg1 "export" neg2 request reply data; do
- for when in before after; do
- check_disconnect "$event" "$when"
- done
-
- # Also inject short replies from the NBD server
- case "$event" in
- neg1)
- for when in 8 16; do
- check_disconnect "$event" "$when"
- done
- ;;
- "export")
- for when in 4 12 16; do
- check_disconnect "$event" "$when"
+for proto in tcp unix; do
+ for event in neg1 "export" neg2 request reply data; do
+ for when in before after; do
+ check_disconnect "--$proto" "$event" "$when"
done
- ;;
- neg2)
- for when in 8 10; do
- check_disconnect "$event" "$when"
- done
- ;;
- reply)
- for when in 4 8; do
- check_disconnect "$event" "$when"
- done
- ;;
- esac
-done
-# Also check classic negotiation without export information
-for when in before 8 16 24 28 after; do
- check_disconnect "neg-classic" "$when" --classic-negotiation
+ # Also inject short replies from the NBD server
+ case "$event" in
+ neg1)
+ for when in 8 16; do
+ check_disconnect "--$proto" "$event" "$when"
+ done
+ ;;
+ "export")
+ for when in 4 12 16; do
+ check_disconnect "--$proto" "$event" "$when"
+ done
+ ;;
+ neg2)
+ for when in 8 10; do
+ check_disconnect "--$proto" "$event" "$when"
+ done
+ ;;
+ reply)
+ for when in 4 8; do
+ check_disconnect "--$proto" "$event" "$when"
+ done
+ ;;
+ esac
+ done
+
+ # Also check classic negotiation without export information
+ for when in before 8 16 24 28 after; do
+ check_disconnect "--$proto" --classic-negotiation "neg-classic" "$when"
+ done
done
# success, all done
diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
index 0c13888..7419722 100644
--- a/tests/qemu-iotests/083.out
+++ b/tests/qemu-iotests/083.out
@@ -1,43 +1,43 @@
QA output created by 083
=== Check disconnect before neg1 ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect after neg1 ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect 8 neg1 ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect 16 neg1 ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect before export ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect after export ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect 4 export ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect 12 export ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect 16 export ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect before neg2 ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect after neg2 ===
@@ -45,11 +45,11 @@ read failed: Input/output error
=== Check disconnect 8 neg2 ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect 10 neg2 ===
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
=== Check disconnect before request ===
@@ -86,23 +86,132 @@ read 512/512 bytes at offset 0
=== Check disconnect before neg-classic ===
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
=== Check disconnect 8 neg-classic ===
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
=== Check disconnect 16 neg-classic ===
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
=== Check disconnect 24 neg-classic ===
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
=== Check disconnect 28 neg-classic ===
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
+
+=== Check disconnect after neg-classic ===
+
+read failed: Input/output error
+
+=== Check disconnect before neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect after neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 8 neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 16 neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect before export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect after export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 4 export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 12 export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 16 export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect before neg2 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect after neg2 ===
+
+read failed: Input/output error
+
+=== Check disconnect 8 neg2 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 10 neg2 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect before request ===
+
+read failed: Input/output error
+
+=== Check disconnect after request ===
+
+read failed: Input/output error
+
+=== Check disconnect before reply ===
+
+read failed: Input/output error
+
+=== Check disconnect after reply ===
+
+read failed: Input/output error
+
+=== Check disconnect 4 reply ===
+
+read failed: Input/output error
+
+=== Check disconnect 8 reply ===
+
+read failed: Input/output error
+
+=== Check disconnect before data ===
+
+read failed: Input/output error
+
+=== Check disconnect after data ===
+
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Check disconnect before neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 8 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 16 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 24 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 28 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
=== Check disconnect after neg-classic ===
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index c9a2d5c..f06d4dc 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -154,9 +154,9 @@ _filter_nbd()
#
# Filter out the TCP port number since this changes between runs.
sed -e '/nbd\/.*\.c:/d' \
- -e 's#nbd:\(//\)\?127\.0\.0\.1:[0-9]*#nbd:\1127.0.0.1:PORT#g' \
+ -e 's#127\.0\.0\.1:[0-9]*#127.0.0.1:PORT#g' \
-e "s#?socket=$TEST_DIR#?socket=TEST_DIR#g" \
- -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#'
+ -e 's#\(foo\|PORT/\?\|.sock\): Failed to .*$#\1#'
}
# make sure this script returns success
--
1.8.3.1

View File

@ -0,0 +1,45 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 27 Sep 2017 17:57:25 +0200
Subject: [PATCH] block/nbd-client: nbd_co_send_request: fix return code
RH-Author: Eric Blake <eblake@redhat.com>
Message-id: <20170927175725.20023-8-eblake@redhat.com>
Patchwork-id: 76673
O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 7/7] block/nbd-client: nbd_co_send_request: fix return code
Bugzilla: 1495474
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
It's incorrect to return success rc >= 0 if we skip qio_channel_writev_all()
call due to s->quit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170920124507.18841-4-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit a693437037328a95d815ad5aec37ac2f8e130e58)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
block/nbd-client.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 434acf6..76789c1 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -156,6 +156,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
if (ret != request->len) {
rc = -EIO;
}
+ } else if (rc >= 0) {
+ rc = -EIO;
}
qio_channel_set_cork(s->ioc, false);
} else {
--
1.8.3.1

12
debian/patches/series vendored
View File

@ -62,3 +62,15 @@ extra/0032-i386-Add-spec-ctrl-CPUID-bit.patch
extra/0033-i386-Add-FEAT_8000_0008_EBX-CPUID-feature-word.patch extra/0033-i386-Add-FEAT_8000_0008_EBX-CPUID-feature-word.patch
extra/0034-i386-Add-new-IBRS-versions-of-Intel-CPU-models.patch extra/0034-i386-Add-new-IBRS-versions-of-Intel-CPU-models.patch
extra/0035-ratelimit-don-t-align-wait-time-with-slices.patch extra/0035-ratelimit-don-t-align-wait-time-with-slices.patch
extra/0036-kvm-nbd-make-it-thread-safe-fix-qcow2-over-nbd.patch
extra/0037-kvm-nbd-strict-nbd_wr_syncv.patch
extra/0038-kvm-nbd-read_sync-and-friends-return-0-on-success.patch
extra/0039-kvm-nbd-make-nbd_drop-public.patch
extra/0040-kvm-nbd-server-get-rid-of-nbd_negotiate_read-and-friends.patch
extra/0041-kvm-nbd-client-Fix-regression-when-server-sends-garbage.patch
extra/0042-kvm-fix-build-failure-in-nbd_read_reply_entry.patch
extra/0043-kvm-nbd-client-avoid-spurious-qio_channel_yield-re-entry.patch
extra/0044-kvm-nbd-client-avoid-read_reply_co-entry-if-send-failed.patch
extra/0045-kvm-qemu-iotests-improve-nbd-fault-injector.py-startup-p.patch
extra/0046-kvm-qemu-iotests-test-NBD-over-UNIX-domain-sockets-in-08.patch
extra/0047-kvm-block-nbd-client-nbd_co_send_request-fix-return-code.patch