On 3/20/19 5:11 PM, Richard W.M. Jones wrote:
This commit implements the core NBD protocol for the
"base:allocation"
Block Status replies.
---
server/internal.h | 7 ++
server/protocol.h | 15 +++
server/protocol-handshake-newstyle.c | 81 ++++++++++++++-
server/protocol.c | 141 ++++++++++++++++++++++++---
4 files changed, 229 insertions(+), 15 deletions(-)
+static int
+send_structured_reply_block_status (struct connection *conn,
+ uint64_t handle,
+ uint16_t cmd, uint16_t flags,
+ uint32_t count, uint64_t offset,
+ struct nbdkit_extents *extents)
+{
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock);
+ struct structured_reply reply;
+ size_t nr_extents = nbdkit_extents_size (extents);
+ uint32_t context_id;
+ size_t i;
+ int r;
+
+ assert (cmd == NBD_CMD_BLOCK_STATUS);
+
Worth asserting conn->meta_context_base_allocation?
+
+ /* Send each block descriptor. */
+ for (i = 0; i < nr_extents; ++i) {
Where does the list terminate after 1 extent if REQ_ONE was set? Also,
if REQ_ONE is set but the plugin provided coalesced status beyond the
request, this would need to clamp the answer to the requested length.
+ const struct nbdkit_extent e = nbdkit_get_extent (extents, i);
+ struct block_descriptor bd;
+
+ if (i == 0)
+ assert (e.offset == offset);
+
+ bd.length = htobe32 (e.length);
+ bd.status_flags = htobe32 (e.type & 3);
+
+ r = conn->send (conn, &bd, sizeof bd);
+ if (r == -1) {
+ nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd));
+ return connection_set_status (conn, -1);
+ }
+ }
Where does the list terminate once you send the final extent that
overlaps the end of the original request? (If you implement my idea of
tracking a maximum offset in nbdkit_extents in patch 1, this may be
picked up automatically)
@@ -498,15 +605,23 @@ protocol_recv_request_send_reply (struct
connection *conn)
}
/* Currently we prefer to send simple replies for everything except
- * where we have to (ie. NBD_CMD_READ when structured_replies have
- * been negotiated). However this prevents us from sending
- * human-readable error messages to the client, so we should
- * reconsider this in future.
+ * where we have to (ie. NBD_CMD_READ and NBD_CMD_BLOCK_STATUS when
+ * structured_replies have been negotiated). However this prevents
+ * us from sending human-readable error messages to the client, so
+ * we should reconsider this in future.
*/
- if (conn->structured_replies && cmd == NBD_CMD_READ) {
- if (!error)
- return send_structured_reply_read (conn, request.handle, cmd,
- buf, count, offset);
+ if (conn->structured_replies &&
+ (cmd == NBD_CMD_READ || cmd == NBD_CMD_BLOCK_STATUS)) {
+ if (!error) {
+ if (cmd == NBD_CMD_READ)
+ return send_structured_reply_read (conn, request.handle, cmd,
+ buf, count, offset);
+ else /* NBD_CMD_BLOCK_STATUS */
+ return send_structured_reply_block_status (conn, request.handle,
+ cmd, flags,
+ count, offset,
+ extents);
+ }
else
return send_structured_reply_error (conn, request.handle, cmd, error);
Ah, so you are already sending a structured error, even though the
protocol didn't require it (which is the qemu bug I just sent a patch
for today). Technically, the protocol requires a structured error for
NBD_CMD_READ, but permits a simple error for NBD_CMD_BLOCK_STATUS; but
portability wise, since qemu 2.12 through 3.1 choke on the simple error,
this works around those versions of qemu.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org