[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PATCH v3 30/44] nbd: Treat flags vs. command type as separ
From: |
Eric Blake |
Subject: |
[Qemu-block] [PATCH v3 30/44] nbd: Treat flags vs. command type as separate fields |
Date: |
Fri, 22 Apr 2016 17:40:38 -0600 |
Current upstream NBD documents that requests have a 16-bit flags,
followed by a 16-bit type integer; although older versions mentioned
only a 32-bit field with masking to find flags. Since the protocol
is in network order (big-endian over the wire), the ABI is unchanged;
but dealing with the flags as a separate field rather than masking
will make it easier to add support for upcoming NBD extensions that
increase the number of both flags and commands.
Improve some comments in nbd.h based on the current upstream
NBD protocol (https://github.com/yoe/nbd/blob/master/doc/proto.md),
and touch some nearby code to keep checkpatch.pl happy.
Signed-off-by: Eric Blake <address@hidden>
---
v3: rebase to other changes earlier in series
---
include/block/nbd.h | 18 ++++++++++++------
nbd/nbd-internal.h | 4 ++--
block/nbd-client.c | 9 +++------
nbd/client.c | 17 ++++++++++-------
nbd/server.c | 51 ++++++++++++++++++++++++++-------------------------
5 files changed, 53 insertions(+), 46 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 2c753cc..f4ae86c 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -1,4 +1,5 @@
/*
+ * Copyright (C) 2016 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <address@hidden>
*
* Network Block Device
@@ -27,7 +28,8 @@
struct nbd_request {
uint32_t magic;
- uint32_t type;
+ uint16_t flags;
+ uint16_t type;
uint64_t handle;
uint64_t from;
uint32_t len;
@@ -39,6 +41,8 @@ struct nbd_reply {
uint64_t handle;
} QEMU_PACKED;
+/* Transmission (export) flags: sent from server to client during handshake,
+ but describe what will happen during transmission */
#define NBD_FLAG_HAS_FLAGS (1 << 0) /* Flags are there */
#define NBD_FLAG_READ_ONLY (1 << 1) /* Device is read-only */
#define NBD_FLAG_SEND_FLUSH (1 << 2) /* Send FLUSH */
@@ -46,10 +50,12 @@ struct nbd_reply {
#define NBD_FLAG_ROTATIONAL (1 << 4) /* Use elevator algorithm -
rotational media */
#define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */
-/* New-style global flags. */
+/* New-style handshake (global) flags, sent from server to client, and
+ control what will happen during handshake phase. */
#define NBD_FLAG_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */
-/* New-style client flags. */
+/* New-style client flags, sent from client to server to control what happens
+ during handshake phase. */
#define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */
/* Reply types. */
@@ -60,10 +66,10 @@ struct nbd_reply {
#define NBD_REP_ERR_INVALID ((UINT32_C(1) << 31) | 3) /* Invalid length. */
#define NBD_REP_ERR_TLS_REQD ((UINT32_C(1) << 31) | 5) /* TLS required */
+/* Request flags, sent from client to server during transmission phase */
+#define NBD_CMD_FLAG_FUA (1 << 0)
-#define NBD_CMD_MASK_COMMAND 0x0000ffff
-#define NBD_CMD_FLAG_FUA (1 << 16)
-
+/* Supported request types */
enum {
NBD_CMD_READ = 0,
NBD_CMD_WRITE = 1,
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 035ead4..d0108a1 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -52,10 +52,10 @@
/* This is all part of the "official" NBD API.
*
* The most up-to-date documentation is available at:
- * https://github.com/yoe/nbd/blob/master/doc/proto.txt
+ * https://github.com/yoe/nbd/blob/master/doc/proto.md
*/
-#define NBD_REQUEST_SIZE (4 + 4 + 8 + 8 + 4)
+#define NBD_REQUEST_SIZE (4 + 2 + 2 + 8 + 8 + 4)
#define NBD_REPLY_SIZE (4 + 4 + 8)
#define NBD_REQUEST_MAGIC 0x25609513
#define NBD_REPLY_MAGIC 0x67446698
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 878e879..285025d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -1,6 +1,7 @@
/*
* QEMU Block driver for NBD
*
+ * Copyright (C) 2016 Red Hat, Inc.
* Copyright (C) 2008 Bull S.A.S.
* Author: Laurent Vivier <address@hidden>
*
@@ -252,7 +253,7 @@ static int nbd_co_writev_1(BlockDriverState *bs, int64_t
sector_num,
if ((*flags & BDRV_REQ_FUA) && (client->nbdflags & NBD_FLAG_SEND_FUA)) {
*flags &= ~BDRV_REQ_FUA;
- request.type |= NBD_CMD_FLAG_FUA;
+ request.flags |= NBD_CMD_FLAG_FUA;
}
request.from = sector_num * 512;
@@ -376,11 +377,7 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
void nbd_client_close(BlockDriverState *bs)
{
NbdClientSession *client = nbd_get_client_session(bs);
- struct nbd_request request = {
- .type = NBD_CMD_DISC,
- .from = 0,
- .len = 0
- };
+ struct nbd_request request = { .type = NBD_CMD_DISC };
if (client->ioc == NULL) {
return;
diff --git a/nbd/client.c b/nbd/client.c
index b700100..a9e173a 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1,4 +1,5 @@
/*
+ * Copyright (C) 2016 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <address@hidden>
*
* Network Block Device Client Side
@@ -713,14 +714,16 @@ ssize_t nbd_send_request(QIOChannel *ioc, struct
nbd_request *request)
TRACE("Sending request to server: "
"{ .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64
- ", .type=%" PRIu16 " }",
- request->from, request->len, request->handle, request->type);
+ ", .flags = %" PRIx16 ", .type = %" PRIu16 " }",
+ request->from, request->len, request->handle,
+ request->flags, request->type);
- cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC);
- cpu_to_be32w((uint32_t*)(buf + 4), request->type);
- cpu_to_be64w((uint64_t*)(buf + 8), request->handle);
- cpu_to_be64w((uint64_t*)(buf + 16), request->from);
- cpu_to_be32w((uint32_t*)(buf + 24), request->len);
+ cpu_to_be32w((uint32_t *)buf, NBD_REQUEST_MAGIC);
+ cpu_to_be16w((uint16_t *)(buf + 4), request->flags);
+ cpu_to_be16w((uint16_t *)(buf + 6), request->type);
+ cpu_to_be64w((uint64_t *)(buf + 8), request->handle);
+ cpu_to_be64w((uint64_t *)(buf + 16), request->from);
+ cpu_to_be32w((uint32_t *)(buf + 24), request->len);
ret = write_sync(ioc, buf, sizeof(buf));
if (ret < 0) {
diff --git a/nbd/server.c b/nbd/server.c
index a20bf8a..1d30b6d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1,4 +1,5 @@
/*
+ * Copyright (C) 2016 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <address@hidden>
*
* Network Block Device Server Side
@@ -646,21 +647,23 @@ static ssize_t nbd_receive_request(QIOChannel *ioc,
struct nbd_request *request)
/* Request
[ 0 .. 3] magic (NBD_REQUEST_MAGIC)
- [ 4 .. 7] type (0 == READ, 1 == WRITE)
+ [ 4 .. 5] flags (NBD_CMD_FLAG_FUA, ...)
+ [ 6 .. 7] type (NBD_CMD_READ, ...)
[ 8 .. 15] handle
[16 .. 23] from
[24 .. 27] len
*/
- magic = be32_to_cpup((uint32_t*)buf);
- request->type = be32_to_cpup((uint32_t*)(buf + 4));
- request->handle = be64_to_cpup((uint64_t*)(buf + 8));
- request->from = be64_to_cpup((uint64_t*)(buf + 16));
- request->len = be32_to_cpup((uint32_t*)(buf + 24));
+ magic = be32_to_cpup((uint32_t *)buf);
+ request->flags = be16_to_cpup((uint16_t *)(buf + 4));
+ request->type = be16_to_cpup((uint16_t *)(buf + 6));
+ request->handle = be64_to_cpup((uint64_t *)(buf + 8));
+ request->from = be64_to_cpup((uint64_t *)(buf + 16));
+ request->len = be32_to_cpup((uint32_t *)(buf + 24));
- TRACE("Got request: { magic = 0x%" PRIx32 ", .type = %" PRIx32
- ", from = %" PRIu64 " , len = %" PRIu32 " }",
- magic, request->type, request->from, request->len);
+ TRACE("Got request: { magic = 0x%" PRIx32 ", .flags = %" PRIx16
+ ", .type = %" PRIx16 ", from = %" PRIu64 " , len = %" PRIu32 " }",
+ magic, request->flags, request->type, request->from, request->len);
if (magic != NBD_REQUEST_MAGIC) {
LOG("invalid magic (got 0x%" PRIx32 ")", magic);
@@ -993,7 +996,6 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
struct nbd_request *request)
{
NBDClient *client = req->client;
- uint32_t command;
ssize_t rc;
g_assert(qemu_in_coroutine());
@@ -1010,8 +1012,7 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
TRACE("Decoding type");
- command = request->type & NBD_CMD_MASK_COMMAND;
- if (command == NBD_CMD_DISC) {
+ if (request->type == NBD_CMD_DISC) {
/* Special case: we're going to disconnect without a reply,
* whether or not flags, from, or len are bogus */
TRACE("Request type is DISCONNECT");
@@ -1028,7 +1029,7 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
goto out;
}
- if (command == NBD_CMD_READ || command == NBD_CMD_WRITE) {
+ if (request->type == NBD_CMD_READ || request->type == NBD_CMD_WRITE) {
if (request->len > NBD_MAX_BUFFER_SIZE) {
LOG("len (%" PRIu32" ) is larger than max len (%u)",
request->len, NBD_MAX_BUFFER_SIZE);
@@ -1042,7 +1043,7 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
goto out;
}
}
- if (command == NBD_CMD_WRITE) {
+ if (request->type == NBD_CMD_WRITE) {
TRACE("Reading %" PRIu32 " byte(s)", request->len);
if (read_sync(client->ioc, req->data, request->len) != request->len) {
@@ -1061,10 +1062,10 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
rc = -EINVAL;
goto out;
}
- if (request->type & ~NBD_CMD_MASK_COMMAND & ~NBD_CMD_FLAG_FUA) {
- LOG("unsupported flags (got 0x%x)",
- request->type & ~NBD_CMD_MASK_COMMAND);
- return -EINVAL;
+ if (request->flags & ~NBD_CMD_FLAG_FUA) {
+ LOG("unsupported flags (got 0x%x)", request->flags);
+ rc = -EINVAL;
+ goto out;
}
rc = 0;
@@ -1084,7 +1085,6 @@ static void nbd_trip(void *opaque)
struct nbd_request request;
struct nbd_reply reply;
ssize_t ret;
- uint32_t command;
int flags;
TRACE("Reading request.");
@@ -1108,7 +1108,6 @@ static void nbd_trip(void *opaque)
reply.error = -ret;
goto error_reply;
}
- command = request.type & NBD_CMD_MASK_COMMAND;
if (client->closing) {
/*
@@ -1118,11 +1117,12 @@ static void nbd_trip(void *opaque)
goto done;
}
- switch (command) {
+ switch (request.type) {
case NBD_CMD_READ:
TRACE("Request type is READ");
- if (request.type & NBD_CMD_FLAG_FUA) {
+ /* XXX: NBD Protocol only documents use of FUA with WRITE */
+ if (request.flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->blk);
if (ret < 0) {
LOG("flush failed");
@@ -1155,7 +1155,7 @@ static void nbd_trip(void *opaque)
TRACE("Writing to device");
flags = 0;
- if (request.type & NBD_CMD_FLAG_FUA) {
+ if (request.flags & NBD_CMD_FLAG_FUA) {
flags |= BDRV_REQ_FUA;
}
ret = blk_pwrite(exp->blk, request.from + exp->dev_offset,
@@ -1208,8 +1208,9 @@ static void nbd_trip(void *opaque)
* NBD_CMD_READ, since we choose not to send bogus filler
* data; likewise after NBD_CMD_WRITE if we did not read the
* payload. */
- if (nbd_co_send_reply(req, &reply, 0) < 0 || command == NBD_CMD_READ ||
- (command == NBD_CMD_WRITE && !req->complete)) {
+ if (nbd_co_send_reply(req, &reply, 0) < 0 ||
+ request.type == NBD_CMD_READ ||
+ (request.type == NBD_CMD_WRITE && !req->complete)) {
goto out;
}
break;
--
2.5.5
- [Qemu-block] [PATCH v3 22/44] block: Kill blk_write(), blk_read(), (continued)
- [Qemu-block] [PATCH v3 22/44] block: Kill blk_write(), blk_read(), Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 21/44] block: Switch blk_write_zeroes() to byte interface, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 28/44] nbd: Detect servers that send unexpected error values, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 27/44] nbd: Use BDRV_REQ_FUA for better FUA where supported, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 26/44] qemu-io: Add 'write -z -u' to test MAY_UNMAP flag, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 31/44] nbd: Share common reply-sending code in server, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 33/44] nbd: Let client skip portions of server reply, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 35/44] nbd: Support shorter handshake, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 30/44] nbd: Treat flags vs. command type as separate fields,
Eric Blake <=
- [Qemu-block] [PATCH v3 29/44] nbd: Avoid magic number for NBD max name size, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 38/44] block: Add blk_get_opt_transfer_length(), Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 41/44] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 43/44] nbd: Implement NBD_OPT_BLOCK_SIZE on server, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 44/44] nbd: Implement NBD_OPT_BLOCK_SIZE on client, Eric Blake, 2016/04/22