* [Qemu-devel] [PATCH v7 01/16] nbd: Add qemu-nbd -D for human-readable description
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 02/16] nbd: Treat flags vs. command type as separate fields Eric Blake
` (16 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
The NBD protocol allows servers to advertise a human-readable
description alongside an export name during NBD_OPT_LIST. Add
an option to pass through the user's string to the NBD client.
Doing this also makes it easier to test commit 200650d4, which
is the client counterpart of receiving the description.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase to latest
v5: rebase to latest
v4: rebase to latest
---
include/block/nbd.h | 1 +
nbd/nbd-internal.h | 5 +++--
nbd/server.c | 34 ++++++++++++++++++++++++++--------
qemu-nbd.c | 12 +++++++++++-
qemu-nbd.texi | 5 ++++-
5 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 80610ff..fd58390 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -115,6 +115,7 @@ BlockBackend *nbd_export_get_blockdev(NBDExport *exp);
NBDExport *nbd_export_find(const char *name);
void nbd_export_set_name(NBDExport *exp, const char *name);
+void nbd_export_set_description(NBDExport *exp, const char *description);
void nbd_export_close_all(void);
void nbd_client_new(NBDExport *exp,
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 93a6ca8..7e78064 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -104,9 +104,10 @@ 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, void *buffer, size_t size)
+static inline ssize_t write_sync(QIOChannel *ioc, const void *buffer,
+ size_t size)
{
- struct iovec iov = { .iov_base = buffer, .iov_len = size };
+ struct iovec iov = { .iov_base = (void *) buffer, .iov_len = size };
return nbd_wr_syncv(ioc, &iov, 1, size, false);
}
diff --git a/nbd/server.c b/nbd/server.c
index 472f584..319827b 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -61,6 +61,7 @@ struct NBDExport {
BlockBackend *blk;
char *name;
+ char *description;
off_t dev_offset;
off_t size;
uint16_t nbdflags;
@@ -129,7 +130,8 @@ static ssize_t nbd_negotiate_read(QIOChannel *ioc, void *buffer, size_t size)
}
-static ssize_t nbd_negotiate_write(QIOChannel *ioc, void *buffer, size_t size)
+static ssize_t nbd_negotiate_write(QIOChannel *ioc, const void *buffer,
+ size_t size)
{
ssize_t ret;
guint watch;
@@ -225,11 +227,15 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
{
- uint64_t magic, name_len;
+ uint64_t magic;
+ size_t name_len, desc_len;
uint32_t opt, type, len;
+ const char *name = exp->name ? exp->name : "";
+ const char *desc = exp->description ? exp->description : "";
- TRACE("Advertising export name '%s'", exp->name ? exp->name : "");
- name_len = strlen(exp->name);
+ TRACE("Advertising export name '%s' description '%s'", name, desc);
+ name_len = strlen(name);
+ desc_len = strlen(desc);
magic = cpu_to_be64(NBD_REP_MAGIC);
if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
LOG("write failed (magic)");
@@ -245,18 +251,22 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
LOG("write failed (reply type)");
return -EINVAL;
}
- len = cpu_to_be32(name_len + sizeof(len));
+ len = cpu_to_be32(name_len + desc_len + sizeof(len));
if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
LOG("write failed (length)");
return -EINVAL;
}
len = cpu_to_be32(name_len);
if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
- LOG("write failed (length)");
+ LOG("write failed (name length)");
return -EINVAL;
}
- if (nbd_negotiate_write(ioc, exp->name, name_len) != name_len) {
- LOG("write failed (buffer)");
+ if (nbd_negotiate_write(ioc, name, name_len) != name_len) {
+ LOG("write failed (name buffer)");
+ return -EINVAL;
+ }
+ if (nbd_negotiate_write(ioc, desc, desc_len) != desc_len) {
+ LOG("write failed (description buffer)");
return -EINVAL;
}
return 0;
@@ -893,6 +903,12 @@ void nbd_export_set_name(NBDExport *exp, const char *name)
nbd_export_put(exp);
}
+void nbd_export_set_description(NBDExport *exp, const char *description)
+{
+ g_free(exp->description);
+ exp->description = g_strdup(description);
+}
+
void nbd_export_close(NBDExport *exp)
{
NBDClient *client, *next;
@@ -902,6 +918,7 @@ void nbd_export_close(NBDExport *exp)
client_close(client);
}
nbd_export_set_name(exp, NULL);
+ nbd_export_set_description(exp, NULL);
nbd_export_put(exp);
}
@@ -920,6 +937,7 @@ void nbd_export_put(NBDExport *exp)
if (--exp->refcount == 0) {
assert(exp->name == NULL);
+ assert(exp->description == NULL);
if (exp->close) {
exp->close(exp);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index cca4a98..5e0dc60 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -82,6 +82,7 @@ static void usage(const char *name)
" -t, --persistent don't exit on the last connection\n"
" -v, --verbose display extra debugging information\n"
" -x, --export-name=NAME expose export by name\n"
+" -D, --description=TEXT with -x, also export a human-readable description\n"
"\n"
"Exposing part of the image:\n"
" -o, --offset=OFFSET offset into the image\n"
@@ -474,7 +475,7 @@ int main(int argc, char **argv)
off_t fd_size;
QemuOpts *sn_opts = NULL;
const char *sn_id_or_name = NULL;
- const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:";
+ const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:";
struct option lopt[] = {
{ "help", no_argument, NULL, 'h' },
{ "version", no_argument, NULL, 'V' },
@@ -500,6 +501,7 @@ int main(int argc, char **argv)
{ "verbose", no_argument, NULL, 'v' },
{ "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT },
{ "export-name", required_argument, NULL, 'x' },
+ { "description", required_argument, NULL, 'D' },
{ "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
{ "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
{ "trace", required_argument, NULL, 'T' },
@@ -520,6 +522,7 @@ int main(int argc, char **argv)
BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
QDict *options = NULL;
const char *export_name = NULL;
+ const char *export_description = NULL;
const char *tlscredsid = NULL;
bool imageOpts = false;
bool writethrough = true;
@@ -683,6 +686,9 @@ int main(int argc, char **argv)
case 'x':
export_name = optarg;
break;
+ case 'D':
+ export_description = optarg;
+ break;
case 'v':
verbose = 1;
break;
@@ -927,7 +933,11 @@ int main(int argc, char **argv)
}
if (export_name) {
nbd_export_set_name(exp, export_name);
+ nbd_export_set_description(exp, export_description);
newproto = true;
+ } else if (export_description) {
+ error_report("Export description requires an export name");
+ exit(EXIT_FAILURE);
}
server_ioc = qio_channel_socket_new();
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 91ebf04..9ea8826 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -79,9 +79,12 @@ Disconnect the device @var{dev}
Allow up to @var{num} clients to share the device (default @samp{1})
@item -t, --persistent
Don't exit on the last connection
-@item -x NAME, --export-name=NAME
+@item -x, --export-name=@var{name}
Set the NBD volume export name. This switches the server to use
the new style NBD protocol negotiation
+@item -D, --description=@var{description}
+Set the NBD volume export description, as a human-readable
+string. Requires the use of @option{-x}
@item --tls-creds=ID
Enable mandatory TLS encryption for the server by setting the ID
of the TLS credentials object previously created with the --object
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 02/16] nbd: Treat flags vs. command type as separate fields
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 01/16] nbd: Add qemu-nbd -D for human-readable description Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 03/16] nbd: Rename NBDRequest to NBDRequestData Eric Blake
` (15 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
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 <eblake@redhat.com>
---
v6: no change
v5: no change
v4: rebase to earlier changes
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 | 9 ++++++---
nbd/server.c | 39 +++++++++++++++++++--------------------
5 files changed, 42 insertions(+), 37 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index fd58390..5fe2670 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 <anthony@codemonkey.ws>
*
* Network Block Device
@@ -32,7 +33,8 @@ struct nbd_request {
uint64_t handle;
uint64_t from;
uint32_t len;
- uint32_t type;
+ uint16_t flags;
+ uint16_t type;
};
struct nbd_reply {
@@ -40,6 +42,8 @@ struct nbd_reply {
uint32_t error;
};
+/* 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 */
@@ -47,10 +51,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. */
@@ -61,10 +67,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 7e78064..99e5157 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -53,10 +53,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 2cf3237..7e9c3ec 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 <Laurent.Vivier@bull.net>
*
@@ -258,7 +259,7 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
if (flags & BDRV_REQ_FUA) {
assert(client->nbdflags & NBD_FLAG_SEND_FUA);
- request.type |= NBD_CMD_FLAG_FUA;
+ request.flags |= NBD_CMD_FLAG_FUA;
}
assert(bytes <= NBD_MAX_BUFFER_SIZE);
@@ -343,11 +344,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 a92f1e2..7c172ed 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1,4 +1,5 @@
/*
+ * Copyright (C) 2016 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
* Network Block Device Client Side
@@ -713,11 +714,13 @@ ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request)
TRACE("Sending request to server: "
"{ .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64
- ", .type=%" PRIu32 " }",
- request->from, request->len, request->handle, request->type);
+ ", .flags = %" PRIx16 ", .type = %" PRIu16 " }",
+ request->from, request->len, request->handle,
+ request->flags, request->type);
stl_be_p(buf, NBD_REQUEST_MAGIC);
- stl_be_p(buf + 4, request->type);
+ stw_be_p(buf + 4, request->flags);
+ stw_be_p(buf + 6, request->type);
stq_be_p(buf + 8, request->handle);
stq_be_p(buf + 16, request->from);
stl_be_p(buf + 24, request->len);
diff --git a/nbd/server.c b/nbd/server.c
index 319827b..2e84d51 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1,4 +1,5 @@
/*
+ * Copyright (C) 2016 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
* Network Block Device Server Side
@@ -651,21 +652,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 = ldl_be_p(buf);
- request->type = ldl_be_p(buf + 4);
+ request->flags = lduw_be_p(buf + 4);
+ request->type = lduw_be_p(buf + 6);
request->handle = ldq_be_p(buf + 8);
request->from = ldq_be_p(buf + 16);
request->len = ldl_be_p(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);
@@ -1012,7 +1015,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());
@@ -1029,13 +1031,12 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
TRACE("Decoding type");
- command = request->type & NBD_CMD_MASK_COMMAND;
- if (command != NBD_CMD_WRITE) {
+ if (request->type != NBD_CMD_WRITE) {
/* No payload, we are ready to read the next request. */
req->complete = true;
}
- 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");
@@ -1052,7 +1053,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);
@@ -1066,7 +1067,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) {
@@ -1082,12 +1083,11 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
LOG("operation past EOF; From: %" PRIu64 ", Len: %" PRIu32
", Size: %" PRIu64, request->from, request->len,
(uint64_t)client->exp->size);
- rc = command == NBD_CMD_WRITE ? -ENOSPC : -EINVAL;
+ rc = request->type == NBD_CMD_WRITE ? -ENOSPC : -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);
+ if (request->flags & ~NBD_CMD_FLAG_FUA) {
+ LOG("unsupported flags (got 0x%x)", request->flags);
rc = -EINVAL;
goto out;
}
@@ -1109,7 +1109,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.");
@@ -1133,7 +1132,6 @@ static void nbd_trip(void *opaque)
reply.error = -ret;
goto error_reply;
}
- command = request.type & NBD_CMD_MASK_COMMAND;
if (client->closing) {
/*
@@ -1143,11 +1141,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");
@@ -1180,7 +1179,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,
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 03/16] nbd: Rename NBDRequest to NBDRequestData
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 01/16] nbd: Add qemu-nbd -D for human-readable description Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 02/16] nbd: Treat flags vs. command type as separate fields Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 04/16] nbd: Rename NbdClientSession to NBDClientSession Eric Blake
` (14 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz
We have both 'struct NBDRequest' and 'struct nbd_request'; making
it confusing to see which does what. Furthermore, we want to
rename nbd_request to align with our normal CamelCase naming
conventions. So, rename the struct which is used to associate
the data received during request callbacks, while leaving the
shorter name for the description of the request sent over the
wire in the NBD protocol.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch
---
nbd/server.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 2e84d51..78c0419 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -47,10 +47,10 @@ static int system_errno_to_nbd_errno(int err)
/* Definitions for opaque data types */
-typedef struct NBDRequest NBDRequest;
+typedef struct NBDRequestData NBDRequestData;
-struct NBDRequest {
- QSIMPLEQ_ENTRY(NBDRequest) entry;
+struct NBDRequestData {
+ QSIMPLEQ_ENTRY(NBDRequestData) entry;
NBDClient *client;
uint8_t *data;
bool complete;
@@ -759,21 +759,21 @@ static void client_close(NBDClient *client)
}
}
-static NBDRequest *nbd_request_get(NBDClient *client)
+static NBDRequestData *nbd_request_get(NBDClient *client)
{
- NBDRequest *req;
+ NBDRequestData *req;
assert(client->nb_requests <= MAX_NBD_REQUESTS - 1);
client->nb_requests++;
nbd_update_can_read(client);
- req = g_new0(NBDRequest, 1);
+ req = g_new0(NBDRequestData, 1);
nbd_client_get(client);
req->client = client;
return req;
}
-static void nbd_request_put(NBDRequest *req)
+static void nbd_request_put(NBDRequestData *req)
{
NBDClient *client = req->client;
@@ -975,7 +975,7 @@ void nbd_export_close_all(void)
}
}
-static ssize_t nbd_co_send_reply(NBDRequest *req, struct nbd_reply *reply,
+static ssize_t nbd_co_send_reply(NBDRequestData *req, struct nbd_reply *reply,
int len)
{
NBDClient *client = req->client;
@@ -1011,7 +1011,7 @@ static ssize_t nbd_co_send_reply(NBDRequest *req, struct nbd_reply *reply,
* and any other negative value to report an error to the client
* (although the caller may still need to disconnect after reporting
* the error). */
-static ssize_t nbd_co_receive_request(NBDRequest *req,
+static ssize_t nbd_co_receive_request(NBDRequestData *req,
struct nbd_request *request)
{
NBDClient *client = req->client;
@@ -1105,7 +1105,7 @@ static void nbd_trip(void *opaque)
{
NBDClient *client = opaque;
NBDExport *exp = client->exp;
- NBDRequest *req;
+ NBDRequestData *req;
struct nbd_request request;
struct nbd_reply reply;
ssize_t ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 04/16] nbd: Rename NbdClientSession to NBDClientSession
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (2 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 03/16] nbd: Rename NBDRequest to NBDRequestData Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 05/16] nbd: Rename struct nbd_request and nbd_reply Eric Blake
` (13 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
It's better to use consistent capitalization of the namespace
used for NBD functions; we have more instances of NBD* than
Nbd*.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch
---
block/nbd-client.h | 6 +++---
block/nbd-client.c | 26 +++++++++++++-------------
block/nbd.c | 4 ++--
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 044aca4..a84a478 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -17,7 +17,7 @@
#define MAX_NBD_REQUESTS 16
-typedef struct NbdClientSession {
+typedef struct NBDClientSession {
QIOChannelSocket *sioc; /* The master data channel */
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
uint16_t nbdflags;
@@ -32,9 +32,9 @@ typedef struct NbdClientSession {
struct nbd_reply reply;
bool is_unix;
-} NbdClientSession;
+} NBDClientSession;
-NbdClientSession *nbd_get_client_session(BlockDriverState *bs);
+NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
int nbd_client_init(BlockDriverState *bs,
QIOChannelSocket *sock,
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 7e9c3ec..c94608a 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -33,7 +33,7 @@
#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
#define INDEX_TO_HANDLE(bs, index) ((index) ^ ((uint64_t)(intptr_t)bs))
-static void nbd_recv_coroutines_enter_all(NbdClientSession *s)
+static void nbd_recv_coroutines_enter_all(NBDClientSession *s)
{
int i;
@@ -46,7 +46,7 @@ static void nbd_recv_coroutines_enter_all(NbdClientSession *s)
static void nbd_teardown_connection(BlockDriverState *bs)
{
- NbdClientSession *client = nbd_get_client_session(bs);
+ NBDClientSession *client = nbd_get_client_session(bs);
if (!client->ioc) { /* Already closed */
return;
@@ -68,7 +68,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
static void nbd_reply_ready(void *opaque)
{
BlockDriverState *bs = opaque;
- NbdClientSession *s = nbd_get_client_session(bs);
+ NBDClientSession *s = nbd_get_client_session(bs);
uint64_t i;
int ret;
@@ -119,7 +119,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
struct nbd_request *request,
QEMUIOVector *qiov)
{
- NbdClientSession *s = nbd_get_client_session(bs);
+ NBDClientSession *s = nbd_get_client_session(bs);
AioContext *aio_context;
int rc, ret, i;
@@ -167,7 +167,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
return rc;
}
-static void nbd_co_receive_reply(NbdClientSession *s,
+static void nbd_co_receive_reply(NBDClientSession *s,
struct nbd_request *request,
struct nbd_reply *reply,
QEMUIOVector *qiov)
@@ -195,7 +195,7 @@ static void nbd_co_receive_reply(NbdClientSession *s,
}
}
-static void nbd_coroutine_start(NbdClientSession *s,
+static void nbd_coroutine_start(NBDClientSession *s,
struct nbd_request *request)
{
/* Poor man semaphore. The free_sema is locked when no other request
@@ -209,7 +209,7 @@ static void nbd_coroutine_start(NbdClientSession *s,
/* s->recv_coroutine[i] is set as soon as we get the send_lock. */
}
-static void nbd_coroutine_end(NbdClientSession *s,
+static void nbd_coroutine_end(NBDClientSession *s,
struct nbd_request *request)
{
int i = HANDLE_TO_INDEX(s, request->handle);
@@ -222,7 +222,7 @@ static void nbd_coroutine_end(NbdClientSession *s,
int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
- NbdClientSession *client = nbd_get_client_session(bs);
+ NBDClientSession *client = nbd_get_client_session(bs);
struct nbd_request request = {
.type = NBD_CMD_READ,
.from = offset,
@@ -248,7 +248,7 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
- NbdClientSession *client = nbd_get_client_session(bs);
+ NBDClientSession *client = nbd_get_client_session(bs);
struct nbd_request request = {
.type = NBD_CMD_WRITE,
.from = offset,
@@ -277,7 +277,7 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
int nbd_client_co_flush(BlockDriverState *bs)
{
- NbdClientSession *client = nbd_get_client_session(bs);
+ NBDClientSession *client = nbd_get_client_session(bs);
struct nbd_request request = { .type = NBD_CMD_FLUSH };
struct nbd_reply reply;
ssize_t ret;
@@ -302,7 +302,7 @@ int nbd_client_co_flush(BlockDriverState *bs)
int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
{
- NbdClientSession *client = nbd_get_client_session(bs);
+ NBDClientSession *client = nbd_get_client_session(bs);
struct nbd_request request = {
.type = NBD_CMD_TRIM,
.from = offset,
@@ -343,7 +343,7 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
void nbd_client_close(BlockDriverState *bs)
{
- NbdClientSession *client = nbd_get_client_session(bs);
+ NBDClientSession *client = nbd_get_client_session(bs);
struct nbd_request request = { .type = NBD_CMD_DISC };
if (client->ioc == NULL) {
@@ -362,7 +362,7 @@ int nbd_client_init(BlockDriverState *bs,
const char *hostname,
Error **errp)
{
- NbdClientSession *client = nbd_get_client_session(bs);
+ NBDClientSession *client = nbd_get_client_session(bs);
int ret;
/* NBD handshake */
diff --git a/block/nbd.c b/block/nbd.c
index 6bc06d6..e227490 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -41,7 +41,7 @@
#define EN_OPTSTR ":exportname="
typedef struct BDRVNBDState {
- NbdClientSession client;
+ NBDClientSession client;
/* For nbd_refresh_filename() */
char *path, *host, *port, *export, *tlscredsid;
@@ -235,7 +235,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
return saddr;
}
-NbdClientSession *nbd_get_client_session(BlockDriverState *bs)
+NBDClientSession *nbd_get_client_session(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
return &s->client;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 05/16] nbd: Rename struct nbd_request and nbd_reply
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (3 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 04/16] nbd: Rename NbdClientSession to NBDClientSession Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 06/16] nbd: Share common reply-sending code in server Eric Blake
` (12 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
Our coding convention prefers CamelCase names, and we already
have other existing structs with NBDFoo naming. Let's be
consistent, before later patches add even more structs.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch
---
block/nbd-client.h | 2 +-
include/block/nbd.h | 10 ++++++----
block/nbd-client.c | 28 ++++++++++++++--------------
nbd/client.c | 4 ++--
nbd/server.c | 12 ++++++------
5 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index a84a478..78e8e57 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -29,7 +29,7 @@ typedef struct NBDClientSession {
int in_flight;
Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
- struct nbd_reply reply;
+ NBDReply reply;
bool is_unix;
} NBDClientSession;
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 5fe2670..a33581b 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -29,18 +29,20 @@
/* Note: these are _NOT_ the same as the network representation of an NBD
* request and reply!
*/
-struct nbd_request {
+struct NBDRequest {
uint64_t handle;
uint64_t from;
uint32_t len;
uint16_t flags;
uint16_t type;
};
+typedef struct NBDRequest NBDRequest;
-struct nbd_reply {
+struct NBDReply {
uint64_t handle;
uint32_t error;
};
+typedef struct NBDReply NBDReply;
/* Transmission (export) flags: sent from server to client during handshake,
but describe what will happen during transmission */
@@ -101,8 +103,8 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
QIOChannel **outioc,
off_t *size, Error **errp);
int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size);
-ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request);
-ssize_t nbd_receive_reply(QIOChannel *ioc, struct nbd_reply *reply);
+ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request);
+ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply);
int nbd_client(int fd);
int nbd_disconnect(int fd);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index c94608a..8e89add 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -116,7 +116,7 @@ static void nbd_restart_write(void *opaque)
}
static int nbd_co_send_request(BlockDriverState *bs,
- struct nbd_request *request,
+ NBDRequest *request,
QEMUIOVector *qiov)
{
NBDClientSession *s = nbd_get_client_session(bs);
@@ -168,8 +168,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
}
static void nbd_co_receive_reply(NBDClientSession *s,
- struct nbd_request *request,
- struct nbd_reply *reply,
+ NBDRequest *request,
+ NBDReply *reply,
QEMUIOVector *qiov)
{
int ret;
@@ -196,7 +196,7 @@ static void nbd_co_receive_reply(NBDClientSession *s,
}
static void nbd_coroutine_start(NBDClientSession *s,
- struct nbd_request *request)
+ NBDRequest *request)
{
/* Poor man semaphore. The free_sema is locked when no other request
* can be accepted, and unlocked after receiving one reply. */
@@ -210,7 +210,7 @@ static void nbd_coroutine_start(NBDClientSession *s,
}
static void nbd_coroutine_end(NBDClientSession *s,
- struct nbd_request *request)
+ NBDRequest *request)
{
int i = HANDLE_TO_INDEX(s, request->handle);
s->recv_coroutine[i] = NULL;
@@ -223,12 +223,12 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
NBDClientSession *client = nbd_get_client_session(bs);
- struct nbd_request request = {
+ NBDRequest request = {
.type = NBD_CMD_READ,
.from = offset,
.len = bytes,
};
- struct nbd_reply reply;
+ NBDReply reply;
ssize_t ret;
assert(bytes <= NBD_MAX_BUFFER_SIZE);
@@ -249,12 +249,12 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
NBDClientSession *client = nbd_get_client_session(bs);
- struct nbd_request request = {
+ NBDRequest request = {
.type = NBD_CMD_WRITE,
.from = offset,
.len = bytes,
};
- struct nbd_reply reply;
+ NBDReply reply;
ssize_t ret;
if (flags & BDRV_REQ_FUA) {
@@ -278,8 +278,8 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
int nbd_client_co_flush(BlockDriverState *bs)
{
NBDClientSession *client = nbd_get_client_session(bs);
- struct nbd_request request = { .type = NBD_CMD_FLUSH };
- struct nbd_reply reply;
+ NBDRequest request = { .type = NBD_CMD_FLUSH };
+ NBDReply reply;
ssize_t ret;
if (!(client->nbdflags & NBD_FLAG_SEND_FLUSH)) {
@@ -303,12 +303,12 @@ int nbd_client_co_flush(BlockDriverState *bs)
int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
{
NBDClientSession *client = nbd_get_client_session(bs);
- struct nbd_request request = {
+ NBDRequest request = {
.type = NBD_CMD_TRIM,
.from = offset,
.len = count,
};
- struct nbd_reply reply;
+ NBDReply reply;
ssize_t ret;
if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) {
@@ -344,7 +344,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 };
+ NBDRequest request = { .type = NBD_CMD_DISC };
if (client->ioc == NULL) {
return;
diff --git a/nbd/client.c b/nbd/client.c
index 7c172ed..86e29dc 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -707,7 +707,7 @@ int nbd_disconnect(int fd)
}
#endif
-ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request)
+ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request)
{
uint8_t buf[NBD_REQUEST_SIZE];
ssize_t ret;
@@ -737,7 +737,7 @@ ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request)
return 0;
}
-ssize_t nbd_receive_reply(QIOChannel *ioc, struct nbd_reply *reply)
+ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply)
{
uint8_t buf[NBD_REPLY_SIZE];
uint32_t magic;
diff --git a/nbd/server.c b/nbd/server.c
index 78c0419..8e0ad78 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -634,7 +634,7 @@ fail:
return rc;
}
-static ssize_t nbd_receive_request(QIOChannel *ioc, struct nbd_request *request)
+static ssize_t nbd_receive_request(QIOChannel *ioc, NBDRequest *request)
{
uint8_t buf[NBD_REQUEST_SIZE];
uint32_t magic;
@@ -677,7 +677,7 @@ static ssize_t nbd_receive_request(QIOChannel *ioc, struct nbd_request *request)
return 0;
}
-static ssize_t nbd_send_reply(QIOChannel *ioc, struct nbd_reply *reply)
+static ssize_t nbd_send_reply(QIOChannel *ioc, NBDReply *reply)
{
uint8_t buf[NBD_REPLY_SIZE];
ssize_t ret;
@@ -975,7 +975,7 @@ void nbd_export_close_all(void)
}
}
-static ssize_t nbd_co_send_reply(NBDRequestData *req, struct nbd_reply *reply,
+static ssize_t nbd_co_send_reply(NBDRequestData *req, NBDReply *reply,
int len)
{
NBDClient *client = req->client;
@@ -1012,7 +1012,7 @@ static ssize_t nbd_co_send_reply(NBDRequestData *req, struct nbd_reply *reply,
* (although the caller may still need to disconnect after reporting
* the error). */
static ssize_t nbd_co_receive_request(NBDRequestData *req,
- struct nbd_request *request)
+ NBDRequest *request)
{
NBDClient *client = req->client;
ssize_t rc;
@@ -1106,8 +1106,8 @@ static void nbd_trip(void *opaque)
NBDClient *client = opaque;
NBDExport *exp = client->exp;
NBDRequestData *req;
- struct nbd_request request;
- struct nbd_reply reply;
+ NBDRequest request;
+ NBDReply reply;
ssize_t ret;
int flags;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 06/16] nbd: Share common reply-sending code in server
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (4 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 05/16] nbd: Rename struct nbd_request and nbd_reply Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 07/16] nbd: Send message along with server NBD_REP_ERR errors Eric Blake
` (11 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz
Rather than open-coding NBD_REP_SERVER, reuse the code we
already have by adding a length parameter. Additionally,
the refactoring will make adding NBD_OPT_GO in a later patch
easier.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: improve (and add) function comments
v5: no change
v4: no change
v3: rebase to changes earlier in series
---
nbd/server.c | 52 +++++++++++++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 8e0ad78..14a5bd5 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -196,12 +196,15 @@ static ssize_t nbd_negotiate_drop_sync(QIOChannel *ioc, size_t size)
*/
-static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
+/* Send a reply header, including length, but no payload.
+ * Return -errno on error, 0 on success. */
+static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
+ uint32_t opt, uint32_t len)
{
uint64_t magic;
- uint32_t len;
- TRACE("Reply opt=%" PRIx32 " type=%" PRIx32, type, opt);
+ TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
+ type, opt, len);
magic = cpu_to_be64(NBD_REP_MAGIC);
if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
@@ -218,7 +221,7 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
LOG("write failed (rep type)");
return -EINVAL;
}
- len = cpu_to_be32(0);
+ len = cpu_to_be32(len);
if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
LOG("write failed (rep data length)");
return -EINVAL;
@@ -226,37 +229,32 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
return 0;
}
+/* Send a reply header with default 0 length.
+ * Return -errno on error, 0 on success. */
+static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
+{
+ return nbd_negotiate_send_rep_len(ioc, type, opt, 0);
+}
+
+/* Send a single NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
+ * Return -errno on error, 0 on success. */
static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
{
- uint64_t magic;
size_t name_len, desc_len;
- uint32_t opt, type, len;
+ uint32_t len;
const char *name = exp->name ? exp->name : "";
const char *desc = exp->description ? exp->description : "";
+ int rc;
TRACE("Advertising export name '%s' description '%s'", name, desc);
name_len = strlen(name);
desc_len = strlen(desc);
- magic = cpu_to_be64(NBD_REP_MAGIC);
- if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
- LOG("write failed (magic)");
- return -EINVAL;
- }
- opt = cpu_to_be32(NBD_OPT_LIST);
- if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
- LOG("write failed (opt)");
- return -EINVAL;
- }
- type = cpu_to_be32(NBD_REP_SERVER);
- if (nbd_negotiate_write(ioc, &type, sizeof(type)) != sizeof(type)) {
- LOG("write failed (reply type)");
- return -EINVAL;
- }
- len = cpu_to_be32(name_len + desc_len + sizeof(len));
- if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
- LOG("write failed (length)");
- return -EINVAL;
+ len = name_len + desc_len + sizeof(len);
+ rc = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, NBD_OPT_LIST, len);
+ if (rc < 0) {
+ return rc;
}
+
len = cpu_to_be32(name_len);
if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
LOG("write failed (name length)");
@@ -273,6 +271,8 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
return 0;
}
+/* Process the NBD_OPT_LIST command, with a potential series of replies.
+ * Return -errno on error, 0 on success. */
static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
{
NBDExport *exp;
@@ -381,6 +381,8 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
}
+/* Process all NBD_OPT_* client option commands.
+ * Return -errno on error, 0 on success. */
static int nbd_negotiate_options(NBDClient *client)
{
uint32_t flags;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 07/16] nbd: Send message along with server NBD_REP_ERR errors
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (5 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 06/16] nbd: Share common reply-sending code in server Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 08/16] nbd: Share common option-sending code in client Eric Blake
` (10 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz
The NBD Protocol allows us to send human-readable messages
along with any NBD_REP_ERR error during option negotiation;
make use of this fact for clients that know what to do with
our message.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: tweak comments, fix indentation
v5: don't leak 'msg'
v4: new patch
---
nbd/server.c | 78 +++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 59 insertions(+), 19 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 14a5bd5..3d39292 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -236,6 +236,38 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
return nbd_negotiate_send_rep_len(ioc, type, opt, 0);
}
+/* Send an error reply.
+ * Return -errno on error, 0 on success. */
+static int GCC_FMT_ATTR(4, 5)
+nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,
+ uint32_t opt, const char *fmt, ...)
+{
+ va_list va;
+ char *msg;
+ int ret;
+ size_t len;
+
+ va_start(va, fmt);
+ msg = g_strdup_vprintf(fmt, va);
+ va_end(va);
+ len = strlen(msg);
+ assert(len < 4096);
+ TRACE("sending error message \"%s\"", msg);
+ ret = nbd_negotiate_send_rep_len(ioc, type, opt, len);
+ if (ret < 0) {
+ goto out;
+ }
+ if (nbd_negotiate_write(ioc, msg, len) != len) {
+ LOG("write failed (error message)");
+ ret = -EIO;
+ } else {
+ ret = 0;
+ }
+out:
+ g_free(msg);
+ return ret;
+}
+
/* Send a single NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
* Return -errno on error, 0 on success. */
static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
@@ -281,8 +313,9 @@ static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
return -EIO;
}
- return nbd_negotiate_send_rep(client->ioc,
- NBD_REP_ERR_INVALID, NBD_OPT_LIST);
+ return nbd_negotiate_send_rep_err(client->ioc,
+ NBD_REP_ERR_INVALID, NBD_OPT_LIST,
+ "OPT_LIST should not have length");
}
/* For each export, send a NBD_REP_SERVER reply. */
@@ -329,7 +362,8 @@ fail:
return rc;
}
-
+/* Handle NBD_OPT_STARTTLS. Return NULL to drop connection, or else the
+ * new channel for all further (now-encrypted) communication. */
static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
uint32_t length)
{
@@ -343,7 +377,8 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
if (nbd_negotiate_drop_sync(ioc, length) != length) {
return NULL;
}
- nbd_negotiate_send_rep(ioc, NBD_REP_ERR_INVALID, NBD_OPT_STARTTLS);
+ nbd_negotiate_send_rep_err(ioc, NBD_REP_ERR_INVALID, NBD_OPT_STARTTLS,
+ "OPT_STARTTLS should not have length");
return NULL;
}
@@ -473,13 +508,15 @@ static int nbd_negotiate_options(NBDClient *client)
return -EINVAL;
default:
- TRACE("Option 0x%" PRIx32 " not permitted before TLS",
- clientflags);
if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
return -EIO;
}
- ret = nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_TLS_REQD,
- clientflags);
+ ret = nbd_negotiate_send_rep_err(client->ioc,
+ NBD_REP_ERR_TLS_REQD,
+ clientflags,
+ "Option 0x%" PRIx32
+ "not permitted before TLS",
+ clientflags);
if (ret < 0) {
return ret;
}
@@ -505,27 +542,30 @@ static int nbd_negotiate_options(NBDClient *client)
return -EIO;
}
if (client->tlscreds) {
- TRACE("TLS already enabled");
- ret = nbd_negotiate_send_rep(client->ioc,
- NBD_REP_ERR_INVALID,
- clientflags);
+ ret = nbd_negotiate_send_rep_err(client->ioc,
+ NBD_REP_ERR_INVALID,
+ clientflags,
+ "TLS already enabled");
} else {
- TRACE("TLS not configured");
- ret = nbd_negotiate_send_rep(client->ioc,
- NBD_REP_ERR_POLICY,
- clientflags);
+ ret = nbd_negotiate_send_rep_err(client->ioc,
+ NBD_REP_ERR_POLICY,
+ clientflags,
+ "TLS not configured");
}
if (ret < 0) {
return ret;
}
break;
default:
- TRACE("Unsupported option 0x%" PRIx32, clientflags);
if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
return -EIO;
}
- ret = nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP,
- clientflags);
+ ret = nbd_negotiate_send_rep_err(client->ioc,
+ NBD_REP_ERR_UNSUP,
+ clientflags,
+ "Unsupported option 0x%"
+ PRIx32,
+ clientflags);
if (ret < 0) {
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 08/16] nbd: Share common option-sending code in client
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (6 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 07/16] nbd: Send message along with server NBD_REP_ERR errors Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 09/16] nbd: Let server know when client gives up negotiation Eric Blake
` (9 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
Rather than open-coding each option request, it's easier to
have common helper functions do the work. That in turn requires
having convenient packed types for handling option requests
and replies.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: comment and formatting tweaks
v5: no change
v4: rebase
v3: rebase, tweak a debug message
---
include/block/nbd.h | 25 +++++-
nbd/nbd-internal.h | 2 +-
nbd/client.c | 255 ++++++++++++++++++++++------------------------------
3 files changed, 131 insertions(+), 151 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index a33581b..b69bf1d 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -26,15 +26,34 @@
#include "io/channel-socket.h"
#include "crypto/tlscreds.h"
-/* Note: these are _NOT_ the same as the network representation of an NBD
+/* Handshake phase structs - this struct is passed on the wire */
+
+struct nbd_option {
+ uint64_t magic; /* NBD_OPTS_MAGIC */
+ uint32_t option; /* NBD_OPT_* */
+ uint32_t length;
+} QEMU_PACKED;
+typedef struct nbd_option nbd_option;
+
+struct nbd_opt_reply {
+ uint64_t magic; /* NBD_REP_MAGIC */
+ uint32_t option; /* NBD_OPT_* */
+ uint32_t type; /* NBD_REP_* */
+ uint32_t length;
+} QEMU_PACKED;
+typedef struct nbd_opt_reply nbd_opt_reply;
+
+/* Transmission phase structs
+ *
+ * Note: these are _NOT_ the same as the network representation of an NBD
* request and reply!
*/
struct NBDRequest {
uint64_t handle;
uint64_t from;
uint32_t len;
- uint16_t flags;
- uint16_t type;
+ uint16_t flags; /* NBD_CMD_FLAG_* */
+ uint16_t type; /* NBD_CMD_* */
};
typedef struct NBDRequest NBDRequest;
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 99e5157..dd57e18 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -62,7 +62,7 @@
#define NBD_REPLY_MAGIC 0x67446698
#define NBD_OPTS_MAGIC 0x49484156454F5054LL
#define NBD_CLIENT_MAGIC 0x0000420281861253LL
-#define NBD_REP_MAGIC 0x3e889045565a9LL
+#define NBD_REP_MAGIC 0x0003e889045565a9LL
#define NBD_SET_SOCK _IO(0xab, 0)
#define NBD_SET_BLKSIZE _IO(0xab, 1)
diff --git a/nbd/client.c b/nbd/client.c
index 86e29dc..f7a2d6e 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -75,64 +75,128 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
*/
+/* Send an option request.
+ *
+ * The request is for option @opt, with @data containing @len bytes of
+ * additional payload for the request (@len may be -1 to treat @data as
+ * a C string; and @data may be NULL if @len is 0).
+ * Return 0 if successful, -1 with errp set if it is impossible to
+ * continue. */
+static int nbd_send_option_request(QIOChannel *ioc, uint32_t opt,
+ uint32_t len, const char *data,
+ Error **errp)
+{
+ nbd_option req;
+ QEMU_BUILD_BUG_ON(sizeof(req) != 16);
-/* If type represents success, return 1 without further action.
- * If type represents an error reply, consume the rest of the packet on ioc.
- * Then return 0 for unsupported (so the client can fall back to
- * other approaches), or -1 with errp set for other errors.
+ if (len == -1) {
+ req.length = len = strlen(data);
+ }
+ TRACE("Sending option request %" PRIu32", len %" PRIu32, opt, len);
+
+ stq_be_p(&req.magic, NBD_OPTS_MAGIC);
+ stl_be_p(&req.option, opt);
+ stl_be_p(&req.length, len);
+
+ if (write_sync(ioc, &req, sizeof(req)) != sizeof(req)) {
+ error_setg(errp, "Failed to send option request header");
+ return -1;
+ }
+
+ if (len && write_sync(ioc, (char *) data, len) != len) {
+ error_setg(errp, "Failed to send option request data");
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Receive the header of an option reply, which should match the given
+ * opt. Read through the length field, but NOT the length bytes of
+ * payload. Return 0 if successful, -1 with errp set if it is
+ * impossible to continue. */
+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)) {
+ error_setg(errp, "failed to read option reply");
+ return -1;
+ }
+ be64_to_cpus(&reply->magic);
+ be32_to_cpus(&reply->option);
+ be32_to_cpus(&reply->type);
+ be32_to_cpus(&reply->length);
+
+ TRACE("Received option reply %" PRIx32", type %" PRIx32", len %" PRIu32,
+ reply->option, reply->type, reply->length);
+
+ if (reply->magic != NBD_REP_MAGIC) {
+ error_setg(errp, "Unexpected option reply magic");
+ return -1;
+ }
+ if (reply->option != opt) {
+ error_setg(errp, "Unexpected option type %x expected %x",
+ reply->option, opt);
+ return -1;
+ }
+ return 0;
+}
+
+/* If reply represents success, return 1 without further action.
+ * If reply represents an error, consume the optional payload of
+ * the packet on ioc. Then return 0 for unsupported (so the client
+ * can fall back to other approaches), or -1 with errp set for other
+ * errors.
*/
-static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type,
+static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
Error **errp)
{
- uint32_t len;
char *msg = NULL;
int result = -1;
- if (!(type & (1 << 31))) {
+ if (!(reply->type & (1 << 31))) {
return 1;
}
- if (read_sync(ioc, &len, sizeof(len)) != sizeof(len)) {
- error_setg(errp, "failed to read option length");
- return -1;
- }
- len = be32_to_cpu(len);
- if (len) {
- if (len > NBD_MAX_BUFFER_SIZE) {
+ if (reply->length) {
+ if (reply->length > NBD_MAX_BUFFER_SIZE) {
error_setg(errp, "server's error message is too long");
goto cleanup;
}
- msg = g_malloc(len + 1);
- if (read_sync(ioc, msg, len) != len) {
+ msg = g_malloc(reply->length + 1);
+ if (read_sync(ioc, msg, reply->length) != reply->length) {
error_setg(errp, "failed to read option error message");
goto cleanup;
}
- msg[len] = '\0';
+ msg[reply->length] = '\0';
}
- switch (type) {
+ switch (reply->type) {
case NBD_REP_ERR_UNSUP:
TRACE("server doesn't understand request %" PRIx32
- ", attempting fallback", opt);
+ ", attempting fallback", reply->option);
result = 0;
goto cleanup;
case NBD_REP_ERR_POLICY:
- error_setg(errp, "Denied by server for option %" PRIx32, opt);
+ error_setg(errp, "Denied by server for option %" PRIx32,
+ reply->option);
break;
case NBD_REP_ERR_INVALID:
- error_setg(errp, "Invalid data length for option %" PRIx32, opt);
+ error_setg(errp, "Invalid data length for option %" PRIx32,
+ reply->option);
break;
case NBD_REP_ERR_TLS_REQD:
error_setg(errp, "TLS negotiation required before option %" PRIx32,
- opt);
+ reply->option);
break;
default:
error_setg(errp, "Unknown error code when asking for option %" PRIx32,
- opt);
+ reply->option);
break;
}
@@ -147,58 +211,29 @@ static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type,
static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
{
- uint64_t magic;
- uint32_t opt;
- uint32_t type;
+ nbd_opt_reply reply;
uint32_t len;
uint32_t namelen;
int error;
*name = NULL;
- if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
- error_setg(errp, "failed to read list option magic");
+ if (nbd_receive_option_reply(ioc, NBD_OPT_LIST, &reply, errp) < 0) {
return -1;
}
- magic = be64_to_cpu(magic);
- if (magic != NBD_REP_MAGIC) {
- error_setg(errp, "Unexpected option list magic");
- return -1;
- }
- if (read_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
- error_setg(errp, "failed to read list option");
- return -1;
- }
- opt = be32_to_cpu(opt);
- if (opt != NBD_OPT_LIST) {
- error_setg(errp, "Unexpected option type %" PRIx32 " expected %x",
- opt, NBD_OPT_LIST);
- return -1;
- }
-
- if (read_sync(ioc, &type, sizeof(type)) != sizeof(type)) {
- error_setg(errp, "failed to read list option type");
- return -1;
- }
- type = be32_to_cpu(type);
- error = nbd_handle_reply_err(ioc, opt, type, errp);
+ error = nbd_handle_reply_err(ioc, &reply, errp);
if (error <= 0) {
return error;
}
+ len = reply.length;
- if (read_sync(ioc, &len, sizeof(len)) != sizeof(len)) {
- error_setg(errp, "failed to read option length");
- return -1;
- }
- len = be32_to_cpu(len);
-
- if (type == NBD_REP_ACK) {
+ if (reply.type == NBD_REP_ACK) {
if (len != 0) {
error_setg(errp, "length too long for option end");
return -1;
}
- } else if (type == NBD_REP_SERVER) {
+ } else if (reply.type == NBD_REP_SERVER) {
if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) {
- error_setg(errp, "incorrect option length");
+ error_setg(errp, "incorrect option length %" PRIu32, len);
return -1;
}
if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
@@ -240,7 +275,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
}
} else {
error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
- type, NBD_REP_SERVER);
+ reply.type, NBD_REP_SERVER);
return -1;
}
return 1;
@@ -251,24 +286,10 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
const char *wantname,
Error **errp)
{
- uint64_t magic = cpu_to_be64(NBD_OPTS_MAGIC);
- uint32_t opt = cpu_to_be32(NBD_OPT_LIST);
- uint32_t length = 0;
bool foundExport = false;
TRACE("Querying export list");
- if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
- error_setg(errp, "Failed to send list option magic");
- return -1;
- }
-
- if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
- error_setg(errp, "Failed to send list option number");
- return -1;
- }
-
- if (write_sync(ioc, &length, sizeof(length)) != sizeof(length)) {
- error_setg(errp, "Failed to send list option length");
+ if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) {
return -1;
}
@@ -314,72 +335,29 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
const char *hostname, Error **errp)
{
- uint64_t magic = cpu_to_be64(NBD_OPTS_MAGIC);
- uint32_t opt = cpu_to_be32(NBD_OPT_STARTTLS);
- uint32_t length = 0;
- uint32_t type;
+ nbd_opt_reply reply;
QIOChannelTLS *tioc;
struct NBDTLSHandshakeData data = { 0 };
TRACE("Requesting TLS from server");
- if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
- error_setg(errp, "Failed to send option magic");
- return NULL;
- }
-
- if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
- error_setg(errp, "Failed to send option number");
- return NULL;
- }
-
- if (write_sync(ioc, &length, sizeof(length)) != sizeof(length)) {
- error_setg(errp, "Failed to send option length");
- return NULL;
- }
-
- TRACE("Getting TLS reply from server1");
- if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
- error_setg(errp, "failed to read option magic");
- return NULL;
- }
- magic = be64_to_cpu(magic);
- if (magic != NBD_REP_MAGIC) {
- error_setg(errp, "Unexpected option magic");
- return NULL;
- }
- TRACE("Getting TLS reply from server2");
- if (read_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
- error_setg(errp, "failed to read option");
- return NULL;
- }
- opt = be32_to_cpu(opt);
- if (opt != NBD_OPT_STARTTLS) {
- error_setg(errp, "Unexpected option type %" PRIx32 " expected %x",
- opt, NBD_OPT_STARTTLS);
+ if (nbd_send_option_request(ioc, NBD_OPT_STARTTLS, 0, NULL, errp) < 0) {
return NULL;
}
TRACE("Getting TLS reply from server");
- if (read_sync(ioc, &type, sizeof(type)) != sizeof(type)) {
- error_setg(errp, "failed to read option type");
+ if (nbd_receive_option_reply(ioc, NBD_OPT_STARTTLS, &reply, errp) < 0) {
return NULL;
}
- type = be32_to_cpu(type);
- if (type != NBD_REP_ACK) {
+
+ if (reply.type != NBD_REP_ACK) {
error_setg(errp, "Server rejected request to start TLS %" PRIx32,
- type);
+ reply.type);
return NULL;
}
- TRACE("Getting TLS reply from server");
- if (read_sync(ioc, &length, sizeof(length)) != sizeof(length)) {
- error_setg(errp, "failed to read option length");
- return NULL;
- }
- length = be32_to_cpu(length);
- if (length != 0) {
+ if (reply.length != 0) {
error_setg(errp, "Start TLS response was not zero %" PRIu32,
- length);
+ reply.length);
return NULL;
}
@@ -466,8 +444,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
if (magic == NBD_OPTS_MAGIC) {
uint32_t clientflags = 0;
- uint32_t opt;
- uint32_t namesize;
uint16_t globalflags;
bool fixedNewStyle = false;
@@ -517,28 +493,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
}
- /* write the export name */
- magic = cpu_to_be64(magic);
- if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
- error_setg(errp, "Failed to send export name magic");
- goto fail;
- }
- opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
- if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
- error_setg(errp, "Failed to send export name option number");
- goto fail;
- }
- namesize = cpu_to_be32(strlen(name));
- if (write_sync(ioc, &namesize, sizeof(namesize)) !=
- sizeof(namesize)) {
- error_setg(errp, "Failed to send export name length");
- goto fail;
- }
- if (write_sync(ioc, (char *)name, strlen(name)) != strlen(name)) {
- error_setg(errp, "Failed to send export name");
+ /* write the export name request */
+ if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, name,
+ errp) < 0) {
goto fail;
}
+ /* Read the response */
if (read_sync(ioc, &s, sizeof(s)) != sizeof(s)) {
error_setg(errp, "Failed to read export length");
goto fail;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 09/16] nbd: Let server know when client gives up negotiation
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (7 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 08/16] nbd: Share common option-sending code in client Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 10/16] nbd: Let client skip portions of server reply Eric Blake
` (8 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz
The NBD spec says that a client should send NBD_OPT_ABORT
rather than just dropping the connection, if the client doesn't
like something the server sent during option negotiation. This
is a best-effort attempt only, and can only be done in places
where we know the server is still in sync with what we've sent,
whether or not we've read everything the server has sent.
Technically, the server then has to reply with NBD_REP_ACK, but
it's not worth complicating the client to wait around for that
reply.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase
v5: no change
v4: new patch
---
nbd/client.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/nbd/client.c b/nbd/client.c
index f7a2d6e..a3e1e7a 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -111,6 +111,19 @@ static int nbd_send_option_request(QIOChannel *ioc, uint32_t opt,
return 0;
}
+/* Send NBD_OPT_ABORT as a courtesy to let the server know that we are
+ * not going to attempt further negotiation. */
+static void nbd_send_opt_abort(QIOChannel *ioc)
+{
+ /* Technically, a compliant server is supposed to reply to us; but
+ * older servers disconnected instead. At any rate, we're allowed
+ * to disconnect without waiting for the server reply, so we don't
+ * even care if the request makes it to the server, let alone
+ * waiting around for whether the server replies. */
+ nbd_send_option_request(ioc, NBD_OPT_ABORT, 0, NULL, NULL);
+}
+
+
/* Receive the header of an option reply, which should match the given
* opt. Read through the length field, but NOT the length bytes of
* payload. Return 0 if successful, -1 with errp set if it is
@@ -121,6 +134,7 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
QEMU_BUILD_BUG_ON(sizeof(*reply) != 20);
if (read_sync(ioc, reply, sizeof(*reply)) != sizeof(*reply)) {
error_setg(errp, "failed to read option reply");
+ nbd_send_opt_abort(ioc);
return -1;
}
be64_to_cpus(&reply->magic);
@@ -133,11 +147,13 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
if (reply->magic != NBD_REP_MAGIC) {
error_setg(errp, "Unexpected option reply magic");
+ nbd_send_opt_abort(ioc);
return -1;
}
if (reply->option != opt) {
error_setg(errp, "Unexpected option type %x expected %x",
reply->option, opt);
+ nbd_send_opt_abort(ioc);
return -1;
}
return 0;
@@ -206,6 +222,9 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
cleanup:
g_free(msg);
+ if (result < 0) {
+ nbd_send_opt_abort(ioc);
+ }
return result;
}
@@ -229,25 +248,30 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
if (reply.type == NBD_REP_ACK) {
if (len != 0) {
error_setg(errp, "length too long for option end");
+ nbd_send_opt_abort(ioc);
return -1;
}
} else if (reply.type == NBD_REP_SERVER) {
if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) {
error_setg(errp, "incorrect option length %" PRIu32, len);
+ nbd_send_opt_abort(ioc);
return -1;
}
if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
error_setg(errp, "failed to read option name length");
+ nbd_send_opt_abort(ioc);
return -1;
}
namelen = be32_to_cpu(namelen);
len -= sizeof(namelen);
if (len < namelen) {
error_setg(errp, "incorrect option name length");
+ nbd_send_opt_abort(ioc);
return -1;
}
if (namelen > NBD_MAX_NAME_SIZE) {
error_setg(errp, "export name length too long %" PRIu32, namelen);
+ nbd_send_opt_abort(ioc);
return -1;
}
@@ -256,6 +280,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
error_setg(errp, "failed to read export name");
g_free(*name);
*name = NULL;
+ nbd_send_opt_abort(ioc);
return -1;
}
(*name)[namelen] = '\0';
@@ -267,6 +292,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
g_free(*name);
g_free(buf);
*name = NULL;
+ nbd_send_opt_abort(ioc);
return -1;
}
buf[len] = '\0';
@@ -276,6 +302,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
} else {
error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
reply.type, NBD_REP_SERVER);
+ nbd_send_opt_abort(ioc);
return -1;
}
return 1;
@@ -325,6 +352,7 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
if (!foundExport) {
error_setg(errp, "No export with name '%s' available", wantname);
+ nbd_send_opt_abort(ioc);
return -1;
}
@@ -352,12 +380,14 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
if (reply.type != NBD_REP_ACK) {
error_setg(errp, "Server rejected request to start TLS %" PRIx32,
reply.type);
+ nbd_send_opt_abort(ioc);
return NULL;
}
if (reply.length != 0) {
error_setg(errp, "Start TLS response was not zero %" PRIu32,
reply.length);
+ nbd_send_opt_abort(ioc);
return NULL;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 10/16] nbd: Let client skip portions of server reply
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (8 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 09/16] nbd: Let server know when client gives up negotiation Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 11/16] nbd: Less allocation during NBD_OPT_LIST Eric Blake
` (7 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz
The server has a nice helper function nbd_negotiate_drop_sync()
which lets it easily ignore fluff from the client (such as the
payload to an unknown option request). We can't quite make it
common, since it depends on nbd_negotiate_read() which handles
coroutine magic, but we can copy the idea into the client where
we have places where we want to ignore data (such as the
description tacked on the end of NBD_REP_SERVER).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase
v5: no change
v4: rebase
v3: rebase
---
nbd/client.c | 47 +++++++++++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index a3e1e7a..df7eb9c 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -75,6 +75,32 @@ 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)
+{
+ ssize_t ret, dropped = size;
+ char small[1024];
+ char *buffer;
+
+ buffer = sizeof(small) < size ? small : g_malloc(MIN(65536, size));
+ while (size > 0) {
+ ret = read_sync(ioc, buffer, MIN(65536, size));
+ if (ret < 0) {
+ goto cleanup;
+ }
+ assert(ret <= size);
+ size -= ret;
+ }
+ ret = dropped;
+
+ 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
@@ -285,19 +311,12 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
}
(*name)[namelen] = '\0';
len -= namelen;
- if (len) {
- char *buf = g_malloc(len + 1);
- if (read_sync(ioc, buf, len) != len) {
- error_setg(errp, "failed to read export description");
- g_free(*name);
- g_free(buf);
- *name = NULL;
- nbd_send_opt_abort(ioc);
- return -1;
- }
- buf[len] = '\0';
- TRACE("Ignoring export description: %s", buf);
- g_free(buf);
+ if (drop_sync(ioc, len) != len) {
+ error_setg(errp, "failed to read export description");
+ g_free(*name);
+ *name = NULL;
+ nbd_send_opt_abort(ioc);
+ return -1;
}
} else {
error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
@@ -576,7 +595,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
- if (read_sync(ioc, &buf, 124) != 124) {
+ if (drop_sync(ioc, 124) != 124) {
error_setg(errp, "Failed to read reserved block");
goto fail;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 11/16] nbd: Less allocation during NBD_OPT_LIST
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (9 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 10/16] nbd: Let client skip portions of server reply Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 12/16] nbd: Support shorter handshake Eric Blake
` (6 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz
Since we know that the maximum name we are willing to accept
is small enough to stack-allocate, rework the iteration over
NBD_OPT_LIST responses to reuse a stack buffer rather than
allocating every time. Furthermore, we don't even have to
allocate if we know the server's length doesn't match what
we are searching for.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase
v5: alter signature of nbd_receive_list for simpler logic
v4: rebase
v3: tweak commit message
---
nbd/client.c | 145 +++++++++++++++++++++++++++++------------------------------
1 file changed, 70 insertions(+), 75 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index df7eb9c..f6468db 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -254,19 +254,28 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
return result;
}
-static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
+/* Process another portion of the NBD_OPT_LIST reply. Set *@match if
+ * the current reply matches @want or if the server does not support
+ * NBD_OPT_LIST, otherwise leave @match alone. Return 0 if iteration
+ * is complete, positive if more replies are expected, or negative
+ * with @errp set if an unrecoverable error occurred. */
+static int nbd_receive_list(QIOChannel *ioc, const char *want, bool *match,
+ Error **errp)
{
nbd_opt_reply reply;
uint32_t len;
uint32_t namelen;
+ char name[NBD_MAX_NAME_SIZE + 1];
int error;
- *name = NULL;
if (nbd_receive_option_reply(ioc, NBD_OPT_LIST, &reply, errp) < 0) {
return -1;
}
error = nbd_handle_reply_err(ioc, &reply, errp);
if (error <= 0) {
+ /* The server did not support NBD_OPT_LIST, so set *match on
+ * the assumption that any name will be accepted. */
+ *match = true;
return error;
}
len = reply.length;
@@ -277,105 +286,91 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
nbd_send_opt_abort(ioc);
return -1;
}
- } else if (reply.type == NBD_REP_SERVER) {
- if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) {
- error_setg(errp, "incorrect option length %" PRIu32, len);
- nbd_send_opt_abort(ioc);
- return -1;
- }
- if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
- error_setg(errp, "failed to read option name length");
- nbd_send_opt_abort(ioc);
- return -1;
- }
- namelen = be32_to_cpu(namelen);
- len -= sizeof(namelen);
- if (len < namelen) {
- error_setg(errp, "incorrect option name length");
- nbd_send_opt_abort(ioc);
- return -1;
- }
- if (namelen > NBD_MAX_NAME_SIZE) {
- error_setg(errp, "export name length too long %" PRIu32, namelen);
- nbd_send_opt_abort(ioc);
- return -1;
- }
-
- *name = g_new0(char, namelen + 1);
- if (read_sync(ioc, *name, namelen) != namelen) {
- error_setg(errp, "failed to read export name");
- g_free(*name);
- *name = NULL;
- nbd_send_opt_abort(ioc);
- return -1;
- }
- (*name)[namelen] = '\0';
- len -= namelen;
- if (drop_sync(ioc, len) != len) {
- error_setg(errp, "failed to read export description");
- g_free(*name);
- *name = NULL;
- nbd_send_opt_abort(ioc);
- return -1;
- }
- } else {
+ return 0;
+ } else if (reply.type != NBD_REP_SERVER) {
error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
reply.type, NBD_REP_SERVER);
nbd_send_opt_abort(ioc);
return -1;
}
+
+ if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) {
+ error_setg(errp, "incorrect option length %" PRIu32, len);
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
+ error_setg(errp, "failed to read option name length");
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ namelen = be32_to_cpu(namelen);
+ len -= sizeof(namelen);
+ if (len < namelen) {
+ error_setg(errp, "incorrect option name length");
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ if (namelen != strlen(want)) {
+ if (drop_sync(ioc, len) != len) {
+ error_setg(errp, "failed to skip export name with wrong length");
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ return 1;
+ }
+
+ assert(namelen < sizeof(name));
+ if (read_sync(ioc, name, namelen) != namelen) {
+ 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) {
+ error_setg(errp, "failed to read export description");
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ if (!strcmp(name, want)) {
+ *match = true;
+ }
return 1;
}
+/* Return -1 on failure, 0 if wantname is an available export. */
static int nbd_receive_query_exports(QIOChannel *ioc,
const char *wantname,
Error **errp)
{
bool foundExport = false;
- TRACE("Querying export list");
+ TRACE("Querying export list for '%s'", wantname);
if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) {
return -1;
}
TRACE("Reading available export names");
while (1) {
- char *name = NULL;
- int ret = nbd_receive_list(ioc, &name, errp);
+ int ret = nbd_receive_list(ioc, wantname, &foundExport, errp);
if (ret < 0) {
- g_free(name);
- name = NULL;
+ /* Server gave unexpected reply */
return -1;
+ } else if (ret == 0) {
+ /* Done iterating. */
+ if (!foundExport) {
+ error_setg(errp, "No export with name '%s' available",
+ wantname);
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ TRACE("Found desired export name '%s'", wantname);
+ return 0;
}
- if (ret == 0) {
- /* Server doesn't support export listing, so
- * we will just assume an export with our
- * wanted name exists */
- foundExport = true;
- break;
- }
- if (name == NULL) {
- TRACE("End of export name list");
- break;
- }
- if (g_str_equal(name, wantname)) {
- foundExport = true;
- TRACE("Found desired export name '%s'", name);
- } else {
- TRACE("Ignored export name '%s'", name);
- }
- g_free(name);
- }
-
- if (!foundExport) {
- error_setg(errp, "No export with name '%s' available", wantname);
- nbd_send_opt_abort(ioc);
- return -1;
}
-
- return 0;
}
static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 12/16] nbd: Support shorter handshake
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (10 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 11/16] nbd: Less allocation during NBD_OPT_LIST Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 13/16] nbd: Refactor conversion to errno to silence checkpatch Eric Blake
` (5 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
The NBD Protocol allows the server and client to mutually agree
on a shorter handshake (omit the 124 bytes of reserved 0), via
the server advertising NBD_FLAG_NO_ZEROES and the client
acknowledging with NBD_FLAG_C_NO_ZEROES (only possible in
newstyle, whether or not it is fixed newstyle). It doesn't
shave much off the wire, but we might as well implement it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alex Bligh <alex@alex.org.uk>
---
v6: rebase
v5: no change
v4: rebase
v3: rebase
---
include/block/nbd.h | 6 ++++--
nbd/client.c | 8 +++++++-
nbd/server.c | 15 +++++++++++----
3 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index b69bf1d..d326308 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -74,11 +74,13 @@ typedef struct NBDReply NBDReply;
/* 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. */
+#define NBD_FLAG_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */
+#define NBD_FLAG_NO_ZEROES (1 << 1) /* End handshake without zeroes. */
/* 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. */
+#define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */
+#define NBD_FLAG_C_NO_ZEROES (1 << 1) /* End handshake without zeroes. */
/* Reply types. */
#define NBD_REP_ACK (1) /* Data sending finished. */
diff --git a/nbd/client.c b/nbd/client.c
index f6468db..f5e4c74 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -439,6 +439,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
char buf[256];
uint64_t magic, s;
int rc;
+ bool zeroes = true;
TRACE("Receiving negotiation tlscreds=%p hostname=%s.",
tlscreds, hostname ? hostname : "<null>");
@@ -503,6 +504,11 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
TRACE("Server supports fixed new style");
clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE;
}
+ if (globalflags & NBD_FLAG_NO_ZEROES) {
+ zeroes = false;
+ TRACE("Server supports no zeroes");
+ clientflags |= NBD_FLAG_C_NO_ZEROES;
+ }
/* client requested flags */
clientflags = cpu_to_be32(clientflags);
if (write_sync(ioc, &clientflags, sizeof(clientflags)) !=
@@ -590,7 +596,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
- if (drop_sync(ioc, 124) != 124) {
+ if (zeroes && drop_sync(ioc, 124) != 124) {
error_setg(errp, "Failed to read reserved block");
goto fail;
}
diff --git a/nbd/server.c b/nbd/server.c
index 3d39292..20f1086 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -81,6 +81,7 @@ struct NBDClient {
int refcount;
void (*close)(NBDClient *client);
+ bool no_zeroes;
NBDExport *exp;
QCryptoTLSCreds *tlscreds;
char *tlsaclname;
@@ -449,6 +450,11 @@ static int nbd_negotiate_options(NBDClient *client)
fixedNewstyle = true;
flags &= ~NBD_FLAG_C_FIXED_NEWSTYLE;
}
+ if (flags & NBD_FLAG_C_NO_ZEROES) {
+ TRACE("Client supports no zeroes at handshake end");
+ client->no_zeroes = true;
+ flags &= ~NBD_FLAG_C_NO_ZEROES;
+ }
if (flags != 0) {
TRACE("Unknown client flags 0x%" PRIx32 " received", flags);
return -EIO;
@@ -601,6 +607,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
bool oldStyle;
+ size_t len;
/* Old style negotiation header without options
[ 0 .. 7] passwd ("NBDMAGIC")
@@ -617,7 +624,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
....options sent....
[18 .. 25] size
[26 .. 27] export flags
- [28 .. 151] reserved (0)
+ [28 .. 151] reserved (0, omit if no_zeroes)
*/
qio_channel_set_blocking(client->ioc, false, NULL);
@@ -636,7 +643,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
} else {
stq_be_p(buf + 8, NBD_OPTS_MAGIC);
- stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE);
+ stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
}
if (oldStyle) {
@@ -663,8 +670,8 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 18, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
- if (nbd_negotiate_write(client->ioc, buf + 18, sizeof(buf) - 18) !=
- sizeof(buf) - 18) {
+ len = client->no_zeroes ? 10 : sizeof(buf) - 18;
+ if (nbd_negotiate_write(client->ioc, buf + 18, len) != len) {
LOG("write failed");
goto fail;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 13/16] nbd: Refactor conversion to errno to silence checkpatch
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (11 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 12/16] nbd: Support shorter handshake Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 14/16] nbd: Improve server handling of shutdown requests Eric Blake
` (4 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz
Checkpatch complains that 'return EINVAL' is usually wrong
(since we tend to favor 'return -EINVAL'). But it is a
false positive for nbd_errno_to_system_errno(). Since NBD
may add future defined wire values, refactor the code to
keep checkpatch happy.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v7: new patch
---
nbd/client.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index f5e4c74..c6da4c4 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -23,23 +23,31 @@
static int nbd_errno_to_system_errno(int err)
{
+ int ret;
switch (err) {
case NBD_SUCCESS:
- return 0;
+ ret = 0;
+ break;
case NBD_EPERM:
- return EPERM;
+ ret = EPERM;
+ break;
case NBD_EIO:
- return EIO;
+ ret = EIO;
+ break;
case NBD_ENOMEM:
- return ENOMEM;
+ ret = ENOMEM;
+ break;
case NBD_ENOSPC:
- return ENOSPC;
+ ret = ENOSPC;
+ break;
default:
TRACE("Squashing unexpected error %d to EINVAL", err);
/* fallthrough */
case NBD_EINVAL:
- return EINVAL;
+ ret = EINVAL;
+ break;
}
+ return ret;
}
/* Definitions for opaque data types */
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 14/16] nbd: Improve server handling of shutdown requests
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (12 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 13/16] nbd: Refactor conversion to errno to silence checkpatch Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 15/16] nbd: Implement NBD_CMD_WRITE_ZEROES on server Eric Blake
` (3 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
NBD commit 6d34500b clarified how clients and servers are supposed
to behave before closing a connection. It added NBD_REP_ERR_SHUTDOWN
(for the server to announce it is about to go away during option
haggling, so the client should quit sending NBD_OPT_* other than
NBD_OPT_ABORT) and ESHUTDOWN (for the server to announce it is about
to go away during transmission, so the client should quit sending
NBD_CMD_* other than NBD_CMD_DISC). It also clarified that
NBD_OPT_ABORT gets a reply, while NBD_CMD_DISC does not.
This patch merely adds the missing reply to NBD_OPT_ABORT and teaches
the client to recognize server errors. Actually teaching the server
to send NBD_REP_ERR_SHUTDOWN or ESHUTDOWN would require knowing that
the server has been requested to shut down soon (maybe we could do
that by installing a SIGINT handler in qemu-nbd, which transitions
from RUNNING to a new state that waits for the client to react,
rather than just out-right quitting - but that's a bigger task for
another day).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v7: conditionalize ESHUTDOWN for mingw
v6: rebase
v5: no change
v4: new patch
---
include/block/nbd.h | 13 +++++++++----
nbd/nbd-internal.h | 1 +
nbd/client.c | 24 ++++++++++++++++++++++++
nbd/server.c | 12 ++++++++++++
4 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index d326308..eea7ef0 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -83,12 +83,17 @@ typedef struct NBDReply NBDReply;
#define NBD_FLAG_C_NO_ZEROES (1 << 1) /* End handshake without zeroes. */
/* Reply types. */
+#define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value))
+
#define NBD_REP_ACK (1) /* Data sending finished. */
#define NBD_REP_SERVER (2) /* Export description. */
-#define NBD_REP_ERR_UNSUP ((UINT32_C(1) << 31) | 1) /* Unknown option. */
-#define NBD_REP_ERR_POLICY ((UINT32_C(1) << 31) | 2) /* Server denied */
-#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 */
+
+#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option */
+#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */
+#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */
+#define NBD_REP_ERR_PLATFORM NBD_REP_ERR(4) /* Not compiled in */
+#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR(5) /* TLS required */
+#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting down */
/* Request flags, sent from client to server during transmission phase */
#define NBD_CMD_FLAG_FUA (1 << 0)
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index dd57e18..eee20ab 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -92,6 +92,7 @@
#define NBD_ENOMEM 12
#define NBD_EINVAL 22
#define NBD_ENOSPC 28
+#define NBD_ESHUTDOWN 108
static inline ssize_t read_sync(QIOChannel *ioc, void *buffer, size_t size)
{
diff --git a/nbd/client.c b/nbd/client.c
index c6da4c4..d109e26 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -21,6 +21,12 @@
#include "qapi/error.h"
#include "nbd-internal.h"
+/* mingw lacks ESHUTDOWN. For this file, we can just fake it to any
+ * value unlikely to collide with any real errno */
+#ifndef ESHUTDOWN
+#define ESHUTDOWN 123456
+#endif
+
static int nbd_errno_to_system_errno(int err)
{
int ret;
@@ -40,6 +46,9 @@ static int nbd_errno_to_system_errno(int err)
case NBD_ENOSPC:
ret = ENOSPC;
break;
+ case NBD_ESHUTDOWN:
+ ret = ESHUTDOWN;
+ break;
default:
TRACE("Squashing unexpected error %d to EINVAL", err);
/* fallthrough */
@@ -239,11 +248,21 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
reply->option);
break;
+ case NBD_REP_ERR_PLATFORM:
+ error_setg(errp, "Server lacks support for option %" PRIx32,
+ reply->option);
+ break;
+
case NBD_REP_ERR_TLS_REQD:
error_setg(errp, "TLS negotiation required before option %" PRIx32,
reply->option);
break;
+ case NBD_REP_ERR_SHUTDOWN:
+ error_setg(errp, "Server shutting down before option %" PRIx32,
+ reply->option);
+ break;
+
default:
error_setg(errp, "Unknown error code when asking for option %" PRIx32,
reply->option);
@@ -784,6 +803,11 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply)
reply->error = nbd_errno_to_system_errno(reply->error);
+ if (reply->error == ESHUTDOWN) {
+ /* This works even on mingw which lacks a native ESHUTDOWN */
+ LOG("server shutting down");
+ return -EINVAL;
+ }
TRACE("Got reply: { magic = 0x%" PRIx32 ", .error = % " PRId32
", handle = %" PRIu64" }",
magic, reply->error, reply->handle);
diff --git a/nbd/server.c b/nbd/server.c
index 20f1086..66dc484 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -39,6 +39,10 @@ static int system_errno_to_nbd_errno(int err)
case EFBIG:
case ENOSPC:
return NBD_ENOSPC;
+#ifdef ESHUTDOWN
+ case ESHUTDOWN:
+ return NBD_ESHUTDOWN;
+#endif
case EINVAL:
default:
return NBD_EINVAL;
@@ -526,6 +530,10 @@ static int nbd_negotiate_options(NBDClient *client)
if (ret < 0) {
return ret;
}
+ /* Let the client keep trying, unless they asked to quit */
+ if (clientflags == NBD_OPT_ABORT) {
+ return -EINVAL;
+ }
break;
}
} else if (fixedNewstyle) {
@@ -538,6 +546,10 @@ static int nbd_negotiate_options(NBDClient *client)
break;
case NBD_OPT_ABORT:
+ /* NBD spec says we must try to reply before
+ * disconnecting, but that we must also tolerate
+ * guests that don't wait for our reply. */
+ nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, clientflags);
return -EINVAL;
case NBD_OPT_EXPORT_NAME:
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 15/16] nbd: Implement NBD_CMD_WRITE_ZEROES on server
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (13 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 14/16] nbd: Improve server handling of shutdown requests Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 16/16] nbd: Implement NBD_CMD_WRITE_ZEROES on client Eric Blake
` (2 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
Upstream NBD protocol recently added the ability to efficiently
write zeroes without having to send the zeroes over the wire,
along with a flag to control whether the client wants to allow
a hole.
Note that when it comes to requiring full allocation, vs.
permitting optimizations, the NBD spec intentionally picked a
different sense for the flag; the rules in qemu are:
MAY_UNMAP == 0: must write zeroes
MAY_UNMAP == 1: may use holes if reads will see zeroes
while in NBD, the rules are:
FLAG_NO_HOLE == 1: must write zeroes
FLAG_NO_HOLE == 0: may use holes if reads will see zeroes
In all cases, the 'may use holes' scenario is optional (the
server need not use a hole, and must not use a hole if
subsequent reads would not see zeroes).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase, improve commit message
v5: no change
v4: rebase, fix value for constant
v3: abandon NBD_CMD_CLOSE extension, rebase to use blk_pwrite_zeroes
---
include/block/nbd.h | 8 ++++++--
nbd/server.c | 42 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index eea7ef0..3e373f0 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -71,6 +71,7 @@ typedef struct NBDReply NBDReply;
#define NBD_FLAG_SEND_FUA (1 << 3) /* Send FUA (Force Unit Access) */
#define NBD_FLAG_ROTATIONAL (1 << 4) /* Use elevator algorithm - rotational media */
#define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */
+#define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* Send WRITE_ZEROES */
/* New-style handshake (global) flags, sent from server to client, and
control what will happen during handshake phase. */
@@ -96,7 +97,8 @@ typedef struct NBDReply NBDReply;
#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting down */
/* Request flags, sent from client to server during transmission phase */
-#define NBD_CMD_FLAG_FUA (1 << 0)
+#define NBD_CMD_FLAG_FUA (1 << 0) /* 'force unit access' during write */
+#define NBD_CMD_FLAG_NO_HOLE (1 << 1) /* don't punch hole on zero run */
/* Supported request types */
enum {
@@ -104,7 +106,9 @@ enum {
NBD_CMD_WRITE = 1,
NBD_CMD_DISC = 2,
NBD_CMD_FLUSH = 3,
- NBD_CMD_TRIM = 4
+ NBD_CMD_TRIM = 4,
+ /* 5 reserved for failed experiment NBD_CMD_CACHE */
+ NBD_CMD_WRITE_ZEROES = 6,
};
#define NBD_DEFAULT_PORT 10809
diff --git a/nbd/server.c b/nbd/server.c
index 66dc484..ea234ae 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -617,7 +617,8 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
char buf[8 + 8 + 8 + 128];
int rc;
const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
- NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
+ NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA |
+ NBD_FLAG_SEND_WRITE_ZEROES);
bool oldStyle;
size_t len;
@@ -1147,11 +1148,17 @@ static ssize_t nbd_co_receive_request(NBDRequestData *req,
rc = request->type == NBD_CMD_WRITE ? -ENOSPC : -EINVAL;
goto out;
}
- if (request->flags & ~NBD_CMD_FLAG_FUA) {
+ if (request->flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE)) {
LOG("unsupported flags (got 0x%x)", request->flags);
rc = -EINVAL;
goto out;
}
+ if (request->type != NBD_CMD_WRITE_ZEROES &&
+ (request->flags & NBD_CMD_FLAG_NO_HOLE)) {
+ LOG("unexpected flags (got 0x%x)", request->flags);
+ rc = -EINVAL;
+ goto out;
+ }
rc = 0;
@@ -1256,6 +1263,37 @@ static void nbd_trip(void *opaque)
}
break;
+ case NBD_CMD_WRITE_ZEROES:
+ TRACE("Request type is WRITE_ZEROES");
+
+ if (exp->nbdflags & NBD_FLAG_READ_ONLY) {
+ TRACE("Server is read-only, return error");
+ reply.error = EROFS;
+ goto error_reply;
+ }
+
+ TRACE("Writing to device");
+
+ flags = 0;
+ if (request.flags & NBD_CMD_FLAG_FUA) {
+ flags |= BDRV_REQ_FUA;
+ }
+ if (!(request.flags & NBD_CMD_FLAG_NO_HOLE)) {
+ flags |= BDRV_REQ_MAY_UNMAP;
+ }
+ ret = blk_pwrite_zeroes(exp->blk, request.from + exp->dev_offset,
+ request.len, flags);
+ if (ret < 0) {
+ LOG("writing to file failed");
+ reply.error = -ret;
+ goto error_reply;
+ }
+
+ if (nbd_co_send_reply(req, &reply, 0) < 0) {
+ goto out;
+ }
+ break;
+
case NBD_CMD_DISC:
/* unreachable, thanks to special case in nbd_co_receive_request() */
abort();
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PATCH v7 16/16] nbd: Implement NBD_CMD_WRITE_ZEROES on client
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (14 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 15/16] nbd: Implement NBD_CMD_WRITE_ZEROES on server Eric Blake
@ 2016-10-14 18:33 ` Eric Blake
2016-10-25 16:13 ` [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
2016-10-27 12:50 ` Paolo Bonzini
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-14 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, pbonzini, mreitz, Kevin Wolf
Upstream NBD protocol recently added the ability to efficiently
write zeroes without having to send the zeroes over the wire,
along with a flag to control whether the client wants a hole.
The generic block code takes care of falling back to the obvious
write of lots of zeroes if we return -ENOTSUP because the server
does not have WRITE_ZEROES.
Ideally, since NBD_CMD_WRITE_ZEROES does not involve any data
over the wire, we want to support transactions that are much
larger than the normal 32M limit imposed on NBD_CMD_WRITE. But
the server may still have a limit smaller than UINT_MAX, so
until experimental NBD protocol additions for advertising various
command sizes is finalized (see [1], [2]), for now we just stick to
the same limits as normal writes.
[1] https://github.com/yoe/nbd/blob/extension-info/doc/proto.md
[2] https://sourceforge.net/p/nbd/mailman/message/35081223/
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase
v5: enhance commit message
v4: rebase to byte-based limits
v3: rebase, tell block layer about our support
---
block/nbd-client.h | 2 ++
block/nbd-client.c | 35 +++++++++++++++++++++++++++++++++++
block/nbd.c | 4 ++++
3 files changed, 41 insertions(+)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 78e8e57..e51df22 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -48,6 +48,8 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count);
int nbd_client_co_flush(BlockDriverState *bs);
int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags);
+int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+ int count, BdrvRequestFlags flags);
int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 8e89add..31db557 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -275,6 +275,41 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
return -reply.error;
}
+int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+ int count, BdrvRequestFlags flags)
+{
+ ssize_t ret;
+ NBDClientSession *client = nbd_get_client_session(bs);
+ NBDRequest request = {
+ .type = NBD_CMD_WRITE_ZEROES,
+ .from = offset,
+ .len = count,
+ };
+ NBDReply reply;
+
+ if (!(client->nbdflags & NBD_FLAG_SEND_WRITE_ZEROES)) {
+ return -ENOTSUP;
+ }
+
+ if (flags & BDRV_REQ_FUA) {
+ assert(client->nbdflags & NBD_FLAG_SEND_FUA);
+ request.flags |= NBD_CMD_FLAG_FUA;
+ }
+ if (!(flags & BDRV_REQ_MAY_UNMAP)) {
+ 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;
+ } else {
+ nbd_co_receive_reply(client, &request, &reply, NULL);
+ }
+ nbd_coroutine_end(client, &request);
+ return -reply.error;
+}
+
int nbd_client_co_flush(BlockDriverState *bs)
{
NBDClientSession *client = nbd_get_client_session(bs);
diff --git a/block/nbd.c b/block/nbd.c
index e227490..6c7bbc8 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -403,6 +403,7 @@ static int nbd_co_flush(BlockDriverState *bs)
static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
{
bs->bl.max_pdiscard = NBD_MAX_BUFFER_SIZE;
+ bs->bl.max_pwrite_zeroes = NBD_MAX_BUFFER_SIZE;
bs->bl.max_transfer = NBD_MAX_BUFFER_SIZE;
}
@@ -491,6 +492,7 @@ static BlockDriver bdrv_nbd = {
.bdrv_file_open = nbd_open,
.bdrv_co_preadv = nbd_client_co_preadv,
.bdrv_co_pwritev = nbd_client_co_pwritev,
+ .bdrv_co_pwrite_zeroes = nbd_client_co_pwrite_zeroes,
.bdrv_close = nbd_close,
.bdrv_co_flush_to_os = nbd_co_flush,
.bdrv_co_pdiscard = nbd_client_co_pdiscard,
@@ -509,6 +511,7 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_file_open = nbd_open,
.bdrv_co_preadv = nbd_client_co_preadv,
.bdrv_co_pwritev = nbd_client_co_pwritev,
+ .bdrv_co_pwrite_zeroes = nbd_client_co_pwrite_zeroes,
.bdrv_close = nbd_close,
.bdrv_co_flush_to_os = nbd_co_flush,
.bdrv_co_pdiscard = nbd_client_co_pdiscard,
@@ -527,6 +530,7 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_file_open = nbd_open,
.bdrv_co_preadv = nbd_client_co_preadv,
.bdrv_co_pwritev = nbd_client_co_pwritev,
+ .bdrv_co_pwrite_zeroes = nbd_client_co_pwrite_zeroes,
.bdrv_close = nbd_close,
.bdrv_co_flush_to_os = nbd_co_flush,
.bdrv_co_pdiscard = nbd_client_co_pdiscard,
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (15 preceding siblings ...)
2016-10-14 18:33 ` [Qemu-devel] [PATCH v7 16/16] nbd: Implement NBD_CMD_WRITE_ZEROES on client Eric Blake
@ 2016-10-25 16:13 ` Eric Blake
2016-10-27 12:50 ` Paolo Bonzini
17 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-10-25 16:13 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, qemu-block, mreitz
[-- Attachment #1: Type: text/plain, Size: 3284 bytes --]
ping - I'd really like this in 2.8, since it missed 2.7, but soft freeze
is again approaching
On 10/14/2016 01:33 PM, Eric Blake wrote:
> Also available as a tag at:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-zero-v7
>
> v5 was here, but missed 2.7 freeze:
> https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04053.html
>
> Since then, I've rebased the series, and the bulk of the changes
> were to use consistent NBDFoo CamelCase naming, as well as to
> improve the commit messages for questions raised on v5.
>
> v6 was here, with no human review yet:
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg03048.html
>
> Since then, I addressed the buildbot complaints.
>
> 001/16:[----] [--] 'nbd: Add qemu-nbd -D for human-readable description'
> 002/16:[----] [--] 'nbd: Treat flags vs. command type as separate fields'
> 003/16:[----] [--] 'nbd: Rename NBDRequest to NBDRequestData'
> 004/16:[----] [--] 'nbd: Rename NbdClientSession to NBDClientSession'
> 005/16:[----] [--] 'nbd: Rename struct nbd_request and nbd_reply'
> 006/16:[----] [--] 'nbd: Share common reply-sending code in server'
> 007/16:[----] [--] 'nbd: Send message along with server NBD_REP_ERR errors'
> 008/16:[----] [--] 'nbd: Share common option-sending code in client'
> 009/16:[----] [--] 'nbd: Let server know when client gives up negotiation'
> 010/16:[----] [--] 'nbd: Let client skip portions of server reply'
> 011/16:[----] [--] 'nbd: Less allocation during NBD_OPT_LIST'
> 012/16:[----] [--] 'nbd: Support shorter handshake'
> 013/16:[down] 'nbd: Refactor conversion to errno to silence checkpatch'
> 014/16:[0012] [FC] 'nbd: Improve server handling of shutdown requests'
> 015/16:[----] [--] 'nbd: Implement NBD_CMD_WRITE_ZEROES on server'
> 016/16:[----] [--] 'nbd: Implement NBD_CMD_WRITE_ZEROES on client'
>
> Eric Blake (16):
> nbd: Add qemu-nbd -D for human-readable description
> nbd: Treat flags vs. command type as separate fields
> nbd: Rename NBDRequest to NBDRequestData
> nbd: Rename NbdClientSession to NBDClientSession
> nbd: Rename struct nbd_request and nbd_reply
> nbd: Share common reply-sending code in server
> nbd: Send message along with server NBD_REP_ERR errors
> nbd: Share common option-sending code in client
> nbd: Let server know when client gives up negotiation
> nbd: Let client skip portions of server reply
> nbd: Less allocation during NBD_OPT_LIST
> nbd: Support shorter handshake
> nbd: Refactor conversion to errno to silence checkpatch
> nbd: Improve server handling of shutdown requests
> nbd: Implement NBD_CMD_WRITE_ZEROES on server
> nbd: Implement NBD_CMD_WRITE_ZEROES on client
>
> block/nbd-client.h | 10 +-
> include/block/nbd.h | 73 ++++++--
> nbd/nbd-internal.h | 12 +-
> block/nbd-client.c | 96 ++++++----
> block/nbd.c | 8 +-
> nbd/client.c | 510 ++++++++++++++++++++++++++++------------------------
> nbd/server.c | 296 ++++++++++++++++++++----------
> qemu-nbd.c | 12 +-
> qemu-nbd.texi | 5 +-
> 9 files changed, 638 insertions(+), 384 deletions(-)
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes
2016-10-14 18:33 [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
` (16 preceding siblings ...)
2016-10-25 16:13 ` [Qemu-devel] [PATCH v7 00/16] nbd: efficient write zeroes Eric Blake
@ 2016-10-27 12:50 ` Paolo Bonzini
17 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2016-10-27 12:50 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: qemu-block, mreitz
On 14/10/2016 20:33, Eric Blake wrote:
> Also available as a tag at:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-zero-v7
>
> v5 was here, but missed 2.7 freeze:
> https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04053.html
>
> Since then, I've rebased the series, and the bulk of the changes
> were to use consistent NBDFoo CamelCase naming, as well as to
> improve the commit messages for questions raised on v5.
>
> v6 was here, with no human review yet:
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg03048.html
>
> Since then, I addressed the buildbot complaints.
The right place for the mingw-specific ESHUTDOWN is
include/qemu/osdep.h. Queued with that change.
Paolo
> 001/16:[----] [--] 'nbd: Add qemu-nbd -D for human-readable description'
> 002/16:[----] [--] 'nbd: Treat flags vs. command type as separate fields'
> 003/16:[----] [--] 'nbd: Rename NBDRequest to NBDRequestData'
> 004/16:[----] [--] 'nbd: Rename NbdClientSession to NBDClientSession'
> 005/16:[----] [--] 'nbd: Rename struct nbd_request and nbd_reply'
> 006/16:[----] [--] 'nbd: Share common reply-sending code in server'
> 007/16:[----] [--] 'nbd: Send message along with server NBD_REP_ERR errors'
> 008/16:[----] [--] 'nbd: Share common option-sending code in client'
> 009/16:[----] [--] 'nbd: Let server know when client gives up negotiation'
> 010/16:[----] [--] 'nbd: Let client skip portions of server reply'
> 011/16:[----] [--] 'nbd: Less allocation during NBD_OPT_LIST'
> 012/16:[----] [--] 'nbd: Support shorter handshake'
> 013/16:[down] 'nbd: Refactor conversion to errno to silence checkpatch'
> 014/16:[0012] [FC] 'nbd: Improve server handling of shutdown requests'
> 015/16:[----] [--] 'nbd: Implement NBD_CMD_WRITE_ZEROES on server'
> 016/16:[----] [--] 'nbd: Implement NBD_CMD_WRITE_ZEROES on client'
>
> Eric Blake (16):
> nbd: Add qemu-nbd -D for human-readable description
> nbd: Treat flags vs. command type as separate fields
> nbd: Rename NBDRequest to NBDRequestData
> nbd: Rename NbdClientSession to NBDClientSession
> nbd: Rename struct nbd_request and nbd_reply
> nbd: Share common reply-sending code in server
> nbd: Send message along with server NBD_REP_ERR errors
> nbd: Share common option-sending code in client
> nbd: Let server know when client gives up negotiation
> nbd: Let client skip portions of server reply
> nbd: Less allocation during NBD_OPT_LIST
> nbd: Support shorter handshake
> nbd: Refactor conversion to errno to silence checkpatch
> nbd: Improve server handling of shutdown requests
> nbd: Implement NBD_CMD_WRITE_ZEROES on server
> nbd: Implement NBD_CMD_WRITE_ZEROES on client
>
> block/nbd-client.h | 10 +-
> include/block/nbd.h | 73 ++++++--
> nbd/nbd-internal.h | 12 +-
> block/nbd-client.c | 96 ++++++----
> block/nbd.c | 8 +-
> nbd/client.c | 510 ++++++++++++++++++++++++++++------------------------
> nbd/server.c | 296 ++++++++++++++++++++----------
> qemu-nbd.c | 12 +-
> qemu-nbd.texi | 5 +-
> 9 files changed, 638 insertions(+), 384 deletions(-)
>
^ permalink raw reply [flat|nested] 19+ messages in thread