From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, vsementsov@virtuozzo.com,
pbonzini@redhat.com, kwolf@redhat.com
Subject: [Qemu-devel] [PATCH v2 7/7] nbd/server: Fix structured read of length 0
Date: Wed, 8 Nov 2017 15:57:03 -0600 [thread overview]
Message-ID: <20171108215703.9295-8-eblake@redhat.com> (raw)
In-Reply-To: <20171108215703.9295-1-eblake@redhat.com>
The NBD spec was recently clarified to state that a read of length 0
should not be attempted by a compliant client; but that a server must
still handle it correctly in an unspecified manner (that is, either
a successful no-op or an error reply, but not a crash) [1]. However,
it also implies that NBD_REPLY_TYPE_OFFSET_DATA must have a non-zero
payload length, but our existing code was replying with a chunk
that a picky client could reject as invalid because it was missing
a payload.
We are already doing successful no-ops for 0-length writes and for
non-structured reads; so for consistency, we want structured reply
reads to also be a no-op. The easiest way to do this is to return
a NBD_REPLY_TYPE_NONE chunk; this is best done via a new helper
function (especially since future patches for other structured
replies may benefit from using the same helper).
[1] https://github.com/NetworkBlockDevice/nbd/commit/ee926037
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 21 ++++++++++++++++++++-
nbd/trace-events | 1 +
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/nbd/server.c b/nbd/server.c
index 6ebb7d9c2e..df771fd42f 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1273,6 +1273,21 @@ static inline void set_be_chunk(NBDStructuredReplyChunk *chunk, uint16_t flags,
stl_be_p(&chunk->length, length);
}
+static int coroutine_fn nbd_co_send_structured_done(NBDClient *client,
+ uint64_t handle,
+ Error **errp)
+{
+ NBDStructuredReplyChunk chunk;
+ struct iovec iov[] = {
+ {.iov_base = &chunk, .iov_len = sizeof(chunk)},
+ };
+
+ trace_nbd_co_send_structured_done(handle);
+ set_be_chunk(&chunk, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_NONE, handle, 0);
+
+ return nbd_co_send_iov(client, iov, 1, errp);
+}
+
static int coroutine_fn nbd_co_send_structured_read(NBDClient *client,
uint64_t handle,
uint64_t offset,
@@ -1286,6 +1301,7 @@ static int coroutine_fn nbd_co_send_structured_read(NBDClient *client,
{.iov_base = data, .iov_len = size}
};
+ assert(size);
trace_nbd_co_send_structured_read(handle, offset, data, size);
set_be_chunk(&chunk.h, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_OFFSET_DATA,
handle, sizeof(chunk) - sizeof(chunk.h) + size);
@@ -1544,10 +1560,13 @@ reply:
if (ret < 0) {
ret = nbd_co_send_structured_error(req->client, request.handle,
-ret, msg, &local_err);
- } else {
+ } else if (reply_data_len) {
ret = nbd_co_send_structured_read(req->client, request.handle,
request.from, req->data,
reply_data_len, &local_err);
+ } else {
+ ret = nbd_co_send_structured_done(req->client, request.handle,
+ &local_err);
}
} else {
ret = nbd_co_send_simple_reply(req->client, request.handle,
diff --git a/nbd/trace-events b/nbd/trace-events
index bbc75f6414..92568edce5 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -55,6 +55,7 @@ nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from
nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p\n"
nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p\n"
nbd_co_send_simple_reply(uint64_t handle, uint32_t error, const char *errname, int len) "Send simple reply: handle = %" PRIu64 ", error = %" PRIu32 " (%s), len = %d"
+nbd_co_send_structured_done(uint64_t handle) "Send structured reply done: handle = %" PRIu64
nbd_co_send_structured_read(uint64_t handle, uint64_t offset, void *data, size_t size) "Send structured read data reply: handle = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
nbd_co_send_structured_error(uint64_t handle, int err, const char *errname, const char *msg) "Send structured error reply: handle = %" PRIu64 ", error = %d (%s), msg = '%s'"
nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *name) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16 " (%s)"
--
2.13.6
next prev parent reply other threads:[~2017-11-08 21:57 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-08 21:56 [Qemu-devel] [PATCH v2 0/7] various NBD fixes for 2.11 Eric Blake
2017-11-08 21:56 ` [Qemu-devel] [PATCH v2 1/7] nbd-client: Fix error message typos Eric Blake
2017-11-09 8:58 ` Vladimir Sementsov-Ogievskiy
2017-11-09 14:43 ` Eric Blake
2017-11-08 21:56 ` [Qemu-devel] [PATCH v2 2/7] nbd-client: Refuse read-only client with BDRV_O_RDWR Eric Blake
2017-11-08 22:07 ` Eric Blake
2017-11-08 22:23 ` [Qemu-devel] [PATCH v2 2.5/7] fixup! " Eric Blake
2017-11-09 9:05 ` Vladimir Sementsov-Ogievskiy
2017-11-09 9:04 ` [Qemu-devel] [PATCH v2 2/7] " Vladimir Sementsov-Ogievskiy
2017-11-08 21:56 ` [Qemu-devel] [PATCH v2 3/7] nbd/client: Nicer trace of structured reply Eric Blake
2017-11-09 9:10 ` Vladimir Sementsov-Ogievskiy
2017-11-08 21:57 ` [Qemu-devel] [PATCH v2 4/7] nbd: Fix struct name for structured reads Eric Blake
2017-11-09 9:13 ` Vladimir Sementsov-Ogievskiy
2017-11-08 21:57 ` [Qemu-devel] [PATCH v2 5/7] nbd-client: Short-circuit 0-length operations Eric Blake
2017-11-09 9:20 ` Vladimir Sementsov-Ogievskiy
2017-11-09 14:44 ` Eric Blake
2017-11-08 21:57 ` [Qemu-devel] [PATCH v2 6/7] nbd-client: Stricter enforcing of structured reply spec Eric Blake
2017-11-09 9:37 ` Vladimir Sementsov-Ogievskiy
2017-11-09 14:45 ` Eric Blake
2017-11-08 21:57 ` Eric Blake [this message]
2017-11-09 9:43 ` [Qemu-devel] [PATCH v2 7/7] nbd/server: Fix structured read of length 0 Vladimir Sementsov-Ogievskiy
2017-11-08 22:24 ` [Qemu-devel] [PATCH v2 0/7] various NBD fixes for 2.11 no-reply
2017-11-08 23:05 ` Eric Blake
2017-11-09 5:34 ` Fam Zheng
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=20171108215703.9295-8-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=kwolf@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).