* [PULL 00/14] NBD patches for 2023-07-19
@ 2023-07-19 20:27 Eric Blake
2023-07-19 20:27 ` [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char* Eric Blake
` (14 more replies)
0 siblings, 15 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
The following changes since commit 2c27fdc7a626408ee2cf30d791aa0b63027c7404:
Update version for v8.1.0-rc0 release (2023-07-19 20:31:43 +0100)
are available in the Git repository at:
https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2023-07-19
for you to fetch changes up to bfe04d0a7d5e8a4f4c9014ee7622af2056685974:
nbd: Use enum for various negotiation modes (2023-07-19 15:26:13 -0500)
----------------------------------------------------------------
NBD patches through 2023-07-19
- Denis V. Lunev: fix hang with 'ssh ... "qemu-nbd -c"'
- Eric Blake: preliminary work towards NBD 64-bit extensions
----------------------------------------------------------------
Denis V. Lunev (6):
qemu-nbd: pass structure into nbd_client_thread instead of plain char*
qemu-nbd: fix regression with qemu-nbd --fork run over ssh
qemu-nbd: properly report error if qemu_daemon() is failed
qemu-nbd: properly report error on error in dup2() after qemu_daemon()
qemu-nbd: handle dup2() error when qemu-nbd finished setup process
qemu-nbd: make verbose bool and local variable in main()
Eric Blake (8):
nbd/client: Use smarter assert
nbd: Consistent typedef usage in header
nbd/server: Prepare for alternate-size headers
nbd/server: Refactor to pass full request around
nbd: s/handle/cookie/ to match NBD spec
nbd/client: Simplify cookie vs. index computation
nbd/client: Add safety check on chunk payload length
nbd: Use enum for various negotiation modes
include/block/nbd.h | 61 +++++++-------
block/nbd.c | 96 +++++++++++-----------
nbd/client.c | 79 ++++++++++--------
nbd/common.c | 17 ++++
nbd/server.c | 224 +++++++++++++++++++++++++++++-----------------------
qemu-nbd.c | 68 +++++++++++-----
nbd/trace-events | 22 +++---
7 files changed, 332 insertions(+), 235 deletions(-)
base-commit: 2c27fdc7a626408ee2cf30d791aa0b63027c7404
--
2.41.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char*
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh Eric Blake
` (13 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Denis V. Lunev, Vladimir Sementsov-Ogievskiy, qemu-stable,
open list:Network Block Dev...
From: "Denis V. Lunev" <den@openvz.org>
We are going to pass additional flag inside next patch.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: <qemu-stable@nongnu.org>
Message-ID: <20230717145544.194786-2-den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-nbd.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 4276163564b..77f98c736bb 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -272,9 +272,13 @@ static void *show_parts(void *arg)
return NULL;
}
+struct NbdClientOpts {
+ char *device;
+};
+
static void *nbd_client_thread(void *arg)
{
- char *device = arg;
+ struct NbdClientOpts *opts = arg;
NBDExportInfo info = { .request_sizes = false, .name = g_strdup("") };
QIOChannelSocket *sioc;
int fd = -1;
@@ -298,10 +302,10 @@ static void *nbd_client_thread(void *arg)
goto out;
}
- fd = open(device, O_RDWR);
+ fd = open(opts->device, O_RDWR);
if (fd < 0) {
/* Linux-only, we can use %m in printf. */
- error_report("Failed to open %s: %m", device);
+ error_report("Failed to open %s: %m", opts->device);
goto out;
}
@@ -311,11 +315,11 @@ static void *nbd_client_thread(void *arg)
}
/* update partition table */
- pthread_create(&show_parts_thread, NULL, show_parts, device);
+ pthread_create(&show_parts_thread, NULL, show_parts, opts->device);
if (verbose) {
fprintf(stderr, "NBD device %s is now connected to %s\n",
- device, srcpath);
+ opts->device, srcpath);
} else {
/* Close stderr so that the qemu-nbd process exits. */
dup2(STDOUT_FILENO, STDERR_FILENO);
@@ -1125,8 +1129,11 @@ int main(int argc, char **argv)
if (device) {
#if HAVE_NBD_DEVICE
int ret;
+ struct NbdClientOpts opts = {
+ .device = device,
+ };
- ret = pthread_create(&client_thread, NULL, nbd_client_thread, device);
+ ret = pthread_create(&client_thread, NULL, nbd_client_thread, &opts);
if (ret != 0) {
error_report("Failed to create client thread: %s", strerror(ret));
exit(EXIT_FAILURE);
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
2023-07-19 20:27 ` [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char* Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed Eric Blake
` (12 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Denis V. Lunev, Vladimir Sementsov-Ogievskiy, Hanna Reitz,
qemu-stable, open list:Network Block Dev...
From: "Denis V. Lunev" <den@openvz.org>
Commit e6df58a5578fee7a50bbf36f4a50a2781cff855d
Author: Hanna Reitz <hreitz@redhat.com>
Date: Wed May 8 23:18:18 2019 +0200
qemu-nbd: Do not close stderr
has introduced an interesting regression. Original behavior of
ssh somehost qemu-nbd /home/den/tmp/file -f raw --fork
was the following:
* qemu-nbd was started as a daemon
* the command execution is done and ssh exited with success
The patch has changed this behavior and 'ssh' command now hangs forever.
According to the normal specification of the daemon() call, we should
endup with STDERR pointing to /dev/null. That should be done at the
very end of the successful startup sequence when the pipe to the
bootstrap process (used for diagnostics) is no longer needed.
This could be achived in the same way as done for 'qemu-nbd -c' case.
That was commit 0eaf453e, also fixing up e6df58a5. STDOUT copying to
STDERR does the trick.
This also leads to proper 'ssh' connection closing which fixes my
original problem.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Hanna Reitz <hreitz@redhat.com>
CC: <qemu-stable@nongnu.org>
Message-ID: <20230717145544.194786-3-den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-nbd.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 77f98c736bb..186ce9474c3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -274,6 +274,7 @@ static void *show_parts(void *arg)
struct NbdClientOpts {
char *device;
+ bool fork_process;
};
static void *nbd_client_thread(void *arg)
@@ -317,7 +318,7 @@ static void *nbd_client_thread(void *arg)
/* update partition table */
pthread_create(&show_parts_thread, NULL, show_parts, opts->device);
- if (verbose) {
+ if (verbose && !opts->fork_process) {
fprintf(stderr, "NBD device %s is now connected to %s\n",
opts->device, srcpath);
} else {
@@ -579,7 +580,6 @@ int main(int argc, char **argv)
bool writethrough = false; /* Client will flush as needed. */
bool fork_process = false;
bool list = false;
- int old_stderr = -1;
unsigned socket_activation;
const char *pid_file_name = NULL;
const char *selinux_label = NULL;
@@ -934,11 +934,6 @@ int main(int argc, char **argv)
} else if (pid == 0) {
close(stderr_fd[0]);
- /* Remember parent's stderr if we will be restoring it. */
- if (fork_process) {
- old_stderr = dup(STDERR_FILENO);
- }
-
ret = qemu_daemon(1, 0);
/* Temporarily redirect stderr to the parent's pipe... */
@@ -1131,6 +1126,7 @@ int main(int argc, char **argv)
int ret;
struct NbdClientOpts opts = {
.device = device,
+ .fork_process = fork_process,
};
ret = pthread_create(&client_thread, NULL, nbd_client_thread, &opts);
@@ -1159,8 +1155,7 @@ int main(int argc, char **argv)
}
if (fork_process) {
- dup2(old_stderr, STDERR_FILENO);
- close(old_stderr);
+ dup2(STDOUT_FILENO, STDERR_FILENO);
}
state = RUNNING;
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
2023-07-19 20:27 ` [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char* Eric Blake
2023-07-19 20:27 ` [PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon() Eric Blake
` (11 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
open list:Network Block Dev...
From: "Denis V. Lunev" <den@openvz.org>
errno has been overwritten by dup2() just below qemu_daemon() and thus
improperly returned to the caller. Fix accordingly.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230717145544.194786-5-den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: reorder patch series]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-nbd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 186ce9474c3..5a8ae1f7472 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -932,14 +932,17 @@ int main(int argc, char **argv)
error_report("Failed to fork: %s", strerror(errno));
exit(EXIT_FAILURE);
} else if (pid == 0) {
+ int saved_errno;
+
close(stderr_fd[0]);
ret = qemu_daemon(1, 0);
+ saved_errno = errno; /* dup2 will overwrite error below */
/* Temporarily redirect stderr to the parent's pipe... */
dup2(stderr_fd[1], STDERR_FILENO);
if (ret < 0) {
- error_report("Failed to daemonize: %s", strerror(errno));
+ error_report("Failed to daemonize: %s", strerror(saved_errno));
exit(EXIT_FAILURE);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon()
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (2 preceding siblings ...)
2023-07-19 20:27 ` [PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process Eric Blake
` (10 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
open list:Network Block Dev...
From: "Denis V. Lunev" <den@openvz.org>
We are trying to temporarily redirect stderr of daemonized process to
a pipe to report a error and get failed. In that case we could not
use error_report() helper, but should write the message directly into
the problematic pipe.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230717145544.194786-4-den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rearrange patch series, fix typo]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-nbd.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 5a8ae1f7472..f27613cb572 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -940,7 +940,20 @@ int main(int argc, char **argv)
saved_errno = errno; /* dup2 will overwrite error below */
/* Temporarily redirect stderr to the parent's pipe... */
- dup2(stderr_fd[1], STDERR_FILENO);
+ if (dup2(stderr_fd[1], STDERR_FILENO) < 0) {
+ char str[256];
+ snprintf(str, sizeof(str),
+ "%s: Failed to link stderr to the pipe: %s\n",
+ g_get_prgname(), strerror(errno));
+ /*
+ * We are unable to use error_report() here as we need to get
+ * stderr pointed to the parent's pipe. Write to that pipe
+ * manually.
+ */
+ ret = write(stderr_fd[1], str, strlen(str));
+ exit(EXIT_FAILURE);
+ }
+
if (ret < 0) {
error_report("Failed to daemonize: %s", strerror(saved_errno));
exit(EXIT_FAILURE);
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (3 preceding siblings ...)
2023-07-19 20:27 ` [PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon() Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 06/14] qemu-nbd: make verbose bool and local variable in main() Eric Blake
` (9 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
open list:Network Block Dev...
From: "Denis V. Lunev" <den@openvz.org>
Fail on error, we are in trouble.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230717145544.194786-6-den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: avoid intermediate variable]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-nbd.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index f27613cb572..e30c9ac1793 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -323,7 +323,11 @@ static void *nbd_client_thread(void *arg)
opts->device, srcpath);
} else {
/* Close stderr so that the qemu-nbd process exits. */
- dup2(STDOUT_FILENO, STDERR_FILENO);
+ if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
+ error_report("Could not set stderr to /dev/null: %s",
+ strerror(errno));
+ exit(EXIT_FAILURE);
+ }
}
if (nbd_client(fd) < 0) {
@@ -1171,7 +1175,11 @@ int main(int argc, char **argv)
}
if (fork_process) {
- dup2(STDOUT_FILENO, STDERR_FILENO);
+ if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
+ error_report("Could not set stderr to /dev/null: %s",
+ strerror(errno));
+ exit(EXIT_FAILURE);
+ }
}
state = RUNNING;
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 06/14] qemu-nbd: make verbose bool and local variable in main()
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (4 preceding siblings ...)
2023-07-19 20:27 ` [PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 07/14] nbd/client: Use smarter assert Eric Blake
` (8 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
open list:Network Block Dev...
From: "Denis V. Lunev" <den@openvz.org>
Pass 'verbose' to nbd_client_thread() inside NbdClientOpts which looks
a little bit cleaner and make it bool as it is used as bool actually.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230717202520.236999-1-den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-nbd.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index e30c9ac1793..5b2757920c1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -73,7 +73,6 @@
#define MBR_SIZE 512
-static int verbose;
static char *srcpath;
static SocketAddress *saddr;
static int persistent = 0;
@@ -275,6 +274,7 @@ static void *show_parts(void *arg)
struct NbdClientOpts {
char *device;
bool fork_process;
+ bool verbose;
};
static void *nbd_client_thread(void *arg)
@@ -318,7 +318,7 @@ static void *nbd_client_thread(void *arg)
/* update partition table */
pthread_create(&show_parts_thread, NULL, show_parts, opts->device);
- if (verbose && !opts->fork_process) {
+ if (opts->verbose && !opts->fork_process) {
fprintf(stderr, "NBD device %s is now connected to %s\n",
opts->device, srcpath);
} else {
@@ -582,6 +582,7 @@ int main(int argc, char **argv)
const char *tlshostname = NULL;
bool imageOpts = false;
bool writethrough = false; /* Client will flush as needed. */
+ bool verbose = false;
bool fork_process = false;
bool list = false;
unsigned socket_activation;
@@ -746,7 +747,7 @@ int main(int argc, char **argv)
}
break;
case 'v':
- verbose = 1;
+ verbose = true;
break;
case 'V':
version(argv[0]);
@@ -1147,6 +1148,7 @@ int main(int argc, char **argv)
struct NbdClientOpts opts = {
.device = device,
.fork_process = fork_process,
+ .verbose = verbose,
};
ret = pthread_create(&client_thread, NULL, nbd_client_thread, &opts);
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 07/14] nbd/client: Use smarter assert
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (5 preceding siblings ...)
2023-07-19 20:27 ` [PULL 06/14] qemu-nbd: make verbose bool and local variable in main() Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 08/14] nbd: Consistent typedef usage in header Eric Blake
` (7 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Dr . David Alan Gilbert, Vladimir Sementsov-Ogievskiy,
open list:Network Block Dev...
Assigning strlen() to a uint32_t and then asserting that it isn't too
large doesn't catch the case of an input string 4G in length.
Thankfully, the incoming strings can never be that large: if the
export name or query is reflecting a string the client got from the
server, we already guarantee that we dropped the NBD connection if the
server sent more than 32M in a single reply to our NBD_OPT_* request;
if the export name is coming from qemu, nbd_receive_negotiate()
asserted that strlen(info->name) <= NBD_MAX_STRING_SIZE; and
similarly, a query string via x->dirty_bitmap coming from the user was
bounds-checked in either qemu-nbd or by the limitations of QMP.
Still, it doesn't hurt to be more explicit in how we write our
assertions to not have to analyze whether inadvertent wraparound is
possible.
Fixes: 93676c88 ("nbd: Don't send oversize strings", v4.2.0)
Reported-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230608135653.2918540-2-eblake@redhat.com>
---
nbd/client.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 30d5383cb19..ff75722e487 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -650,19 +650,20 @@ static int nbd_send_meta_query(QIOChannel *ioc, uint32_t opt,
Error **errp)
{
int ret;
- uint32_t export_len = strlen(export);
+ uint32_t export_len;
uint32_t queries = !!query;
uint32_t query_len = 0;
uint32_t data_len;
char *data;
char *p;
+ assert(strnlen(export, NBD_MAX_STRING_SIZE + 1) <= NBD_MAX_STRING_SIZE);
+ export_len = strlen(export);
data_len = sizeof(export_len) + export_len + sizeof(queries);
- assert(export_len <= NBD_MAX_STRING_SIZE);
if (query) {
+ assert(strnlen(query, NBD_MAX_STRING_SIZE + 1) <= NBD_MAX_STRING_SIZE);
query_len = strlen(query);
data_len += sizeof(query_len) + query_len;
- assert(query_len <= NBD_MAX_STRING_SIZE);
} else {
assert(opt == NBD_OPT_LIST_META_CONTEXT);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 08/14] nbd: Consistent typedef usage in header
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (6 preceding siblings ...)
2023-07-19 20:27 ` [PULL 07/14] nbd/client: Use smarter assert Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 09/14] nbd/server: Prepare for alternate-size headers Eric Blake
` (6 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Kevin Wolf, Hanna Reitz,
open list:Network Block Dev...
We had a mix of struct declarations followed by typedefs, and direct
struct definitions as part of a typedef. Pick a single style. Also
float forward declarations of opaque types to the top of the file,
rather than interspersed with function declarations, which will help a
future patch that wants to expose yet another opaque type that will be
referenced in NBDRequest. No semantic impact.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230608135653.2918540-3-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
[eblake: alter patch per mailing list feedback]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/nbd.h | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index a4c98169c39..9dcb5357d15 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2022 Red Hat, Inc.
+ * Copyright Red Hat
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
* Network Block Device
@@ -26,24 +26,26 @@
#include "qapi/error.h"
#include "qemu/bswap.h"
+typedef struct NBDExport NBDExport;
+typedef struct NBDClient NBDClient;
+typedef struct NBDClientConnection NBDClientConnection;
+
extern const BlockExportDriver blk_exp_nbd;
/* Handshake phase structs - this struct is passed on the wire */
-struct NBDOption {
+typedef struct NBDOption {
uint64_t magic; /* NBD_OPTS_MAGIC */
uint32_t option; /* NBD_OPT_* */
uint32_t length;
-} QEMU_PACKED;
-typedef struct NBDOption NBDOption;
+} QEMU_PACKED NBDOption;
-struct NBDOptionReply {
+typedef struct NBDOptionReply {
uint64_t magic; /* NBD_REP_MAGIC */
uint32_t option; /* NBD_OPT_* */
uint32_t type; /* NBD_REP_* */
uint32_t length;
-} QEMU_PACKED;
-typedef struct NBDOptionReply NBDOptionReply;
+} QEMU_PACKED NBDOptionReply;
typedef struct NBDOptionReplyMetaContext {
NBDOptionReply h; /* h.type = NBD_REP_META_CONTEXT, h.length > 4 */
@@ -56,14 +58,13 @@ typedef struct NBDOptionReplyMetaContext {
* Note: these are _NOT_ the same as the network representation of an NBD
* request and reply!
*/
-struct NBDRequest {
+typedef struct NBDRequest {
uint64_t handle;
uint64_t from;
uint32_t len;
uint16_t flags; /* NBD_CMD_FLAG_* */
uint16_t type; /* NBD_CMD_* */
-};
-typedef struct NBDRequest NBDRequest;
+} NBDRequest;
typedef struct NBDSimpleReply {
uint32_t magic; /* NBD_SIMPLE_REPLY_MAGIC */
@@ -282,7 +283,7 @@ static inline bool nbd_reply_type_is_error(int type)
#define NBD_ESHUTDOWN 108
/* Details collected by NBD_OPT_EXPORT_NAME and NBD_OPT_GO */
-struct NBDExportInfo {
+typedef struct NBDExportInfo {
/* Set by client before nbd_receive_negotiate() */
bool request_sizes;
char *x_dirty_bitmap;
@@ -310,8 +311,7 @@ struct NBDExportInfo {
char *description;
int n_contexts;
char **contexts;
-};
-typedef struct NBDExportInfo NBDExportInfo;
+} NBDExportInfo;
int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
@@ -330,9 +330,6 @@ int nbd_client(int fd);
int nbd_disconnect(int fd);
int nbd_errno_to_system_errno(int err);
-typedef struct NBDExport NBDExport;
-typedef struct NBDClient NBDClient;
-
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
AioContext *nbd_export_aio_context(NBDExport *exp);
@@ -409,8 +406,6 @@ const char *nbd_cmd_lookup(uint16_t info);
const char *nbd_err_lookup(int err);
/* nbd/client-connection.c */
-typedef struct NBDClientConnection NBDClientConnection;
-
void nbd_client_connection_enable_retry(NBDClientConnection *conn);
NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 09/14] nbd/server: Prepare for alternate-size headers
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (7 preceding siblings ...)
2023-07-19 20:27 ` [PULL 08/14] nbd: Consistent typedef usage in header Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 10/14] nbd/server: Refactor to pass full request around Eric Blake
` (5 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Kevin Wolf, Hanna Reitz,
open list:Network Block Dev...
Upstream NBD now documents[1] an extension that supports 64-bit effect
lengths in requests. As part of that extension, the size of the reply
headers will change in order to permit a 64-bit length in the reply
for symmetry[2]. Additionally, where the reply header is currently 16
bytes for simple reply, and 20 bytes for structured reply; with the
extension enabled, there will only be one extended reply header, of 32
bytes, with both structured and extended modes sending identical
payloads for chunked replies.
Since we are already wired up to use iovecs, it is easiest to allow
for this change in header size by splitting each structured reply
across multiple iovecs, one for the header (which will become wider in
a future patch according to client negotiation), and the other(s) for
the chunk payload, and removing the header from the payload struct
definitions. Rename the affected functions with s/structured/chunk/
to make it obvious that the code will be reused in extended mode.
Interestingly, the client side code never utilized the packed types,
so only the server code needs to be updated.
[1] https://github.com/NetworkBlockDevice/nbd/blob/extension-ext-header/doc/proto.md
as of NBD commit e6f3b94a934
[2] Note that on the surface, this is because some future server might
permit a 4G+ NBD_CMD_READ and need to reply with that much data in one
transaction. But even though the extended reply length is widened to
64 bits, for now the NBD spec is clear that servers will not reply
with more than a maximum payload bounded by the 32-bit
NBD_INFO_BLOCK_SIZE field; allowing a client and server to mutually
agree to transactions larger than 4G would require yet another
extension.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230608135653.2918540-4-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
include/block/nbd.h | 8 +--
nbd/server.c | 137 ++++++++++++++++++++++++++------------------
nbd/trace-events | 8 +--
3 files changed, 88 insertions(+), 65 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 9dcb5357d15..ee71af099a3 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -97,28 +97,28 @@ typedef union NBDReply {
/* Header of chunk for NBD_REPLY_TYPE_OFFSET_DATA */
typedef struct NBDStructuredReadData {
- NBDStructuredReplyChunk h; /* h.length >= 9 */
+ /* header's .length >= 9 */
uint64_t offset;
/* At least one byte of data payload follows, calculated from h.length */
} QEMU_PACKED NBDStructuredReadData;
/* Complete chunk for NBD_REPLY_TYPE_OFFSET_HOLE */
typedef struct NBDStructuredReadHole {
- NBDStructuredReplyChunk h; /* h.length == 12 */
+ /* header's length == 12 */
uint64_t offset;
uint32_t length;
} QEMU_PACKED NBDStructuredReadHole;
/* Header of all NBD_REPLY_TYPE_ERROR* errors */
typedef struct NBDStructuredError {
- NBDStructuredReplyChunk h; /* h.length >= 6 */
+ /* header's length >= 6 */
uint32_t error;
uint16_t message_length;
} QEMU_PACKED NBDStructuredError;
/* Header of NBD_REPLY_TYPE_BLOCK_STATUS */
typedef struct NBDStructuredMeta {
- NBDStructuredReplyChunk h; /* h.length >= 12 (at least one extent) */
+ /* header's length >= 12 (at least one extent) */
uint32_t context_id;
/* extents follows */
} QEMU_PACKED NBDStructuredMeta;
diff --git a/nbd/server.c b/nbd/server.c
index febe001a399..6698ab46365 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2022 Red Hat, Inc.
+ * Copyright Red Hat
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
* Network Block Device Server Side
@@ -1906,16 +1906,36 @@ static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
{.iov_base = data, .iov_len = len}
};
+ assert(!len || !nbd_err);
trace_nbd_co_send_simple_reply(handle, nbd_err, nbd_err_lookup(nbd_err),
len);
set_be_simple_reply(&reply, nbd_err, handle);
- return nbd_co_send_iov(client, iov, len ? 2 : 1, errp);
+ return nbd_co_send_iov(client, iov, 2, errp);
}
-static inline void set_be_chunk(NBDStructuredReplyChunk *chunk, uint16_t flags,
- uint16_t type, uint64_t handle, uint32_t length)
+/*
+ * Prepare the header of a reply chunk for network transmission.
+ *
+ * On input, @iov is partially initialized: iov[0].iov_base must point
+ * to an uninitialized NBDReply, while the remaining @niov elements
+ * (if any) must be ready for transmission. This function then
+ * populates iov[0] for transmission.
+ */
+static inline void set_be_chunk(NBDClient *client, struct iovec *iov,
+ size_t niov, uint16_t flags, uint16_t type,
+ uint64_t handle)
{
+ /* TODO - handle structured vs. extended replies */
+ NBDStructuredReplyChunk *chunk = iov->iov_base;
+ size_t i, length = 0;
+
+ for (i = 1; i < niov; i++) {
+ length += iov[i].iov_len;
+ }
+ assert(length <= NBD_MAX_BUFFER_SIZE + sizeof(NBDStructuredReadData));
+
+ iov[0].iov_len = sizeof(*chunk);
stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
stw_be_p(&chunk->flags, flags);
stw_be_p(&chunk->type, type);
@@ -1923,67 +1943,71 @@ static inline void set_be_chunk(NBDStructuredReplyChunk *chunk, uint16_t flags,
stl_be_p(&chunk->length, length);
}
-static int coroutine_fn nbd_co_send_structured_done(NBDClient *client,
- uint64_t handle,
- Error **errp)
+static int coroutine_fn nbd_co_send_chunk_done(NBDClient *client,
+ uint64_t handle,
+ Error **errp)
{
- NBDStructuredReplyChunk chunk;
+ NBDReply hdr;
struct iovec iov[] = {
- {.iov_base = &chunk, .iov_len = sizeof(chunk)},
+ {.iov_base = &hdr},
};
- trace_nbd_co_send_structured_done(handle);
- set_be_chunk(&chunk, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_NONE, handle, 0);
+ trace_nbd_co_send_chunk_done(handle);
+ set_be_chunk(client, iov, 1, NBD_REPLY_FLAG_DONE,
+ NBD_REPLY_TYPE_NONE, handle);
return nbd_co_send_iov(client, iov, 1, errp);
}
-static int coroutine_fn nbd_co_send_structured_read(NBDClient *client,
- uint64_t handle,
- uint64_t offset,
- void *data,
- size_t size,
- bool final,
- Error **errp)
+static int coroutine_fn nbd_co_send_chunk_read(NBDClient *client,
+ uint64_t handle,
+ uint64_t offset,
+ void *data,
+ size_t size,
+ bool final,
+ Error **errp)
{
+ NBDReply hdr;
NBDStructuredReadData chunk;
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
{.iov_base = data, .iov_len = size}
};
assert(size);
- trace_nbd_co_send_structured_read(handle, offset, data, size);
- set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_OFFSET_DATA, handle,
- sizeof(chunk) - sizeof(chunk.h) + size);
+ trace_nbd_co_send_chunk_read(handle, offset, data, size);
+ set_be_chunk(client, iov, 3, final ? NBD_REPLY_FLAG_DONE : 0,
+ NBD_REPLY_TYPE_OFFSET_DATA, handle);
stq_be_p(&chunk.offset, offset);
- return nbd_co_send_iov(client, iov, 2, errp);
+ return nbd_co_send_iov(client, iov, 3, errp);
}
-static int coroutine_fn nbd_co_send_structured_error(NBDClient *client,
- uint64_t handle,
- uint32_t error,
- const char *msg,
- Error **errp)
+static int coroutine_fn nbd_co_send_chunk_error(NBDClient *client,
+ uint64_t handle,
+ uint32_t error,
+ const char *msg,
+ Error **errp)
{
+ NBDReply hdr;
NBDStructuredError chunk;
int nbd_err = system_errno_to_nbd_errno(error);
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
{.iov_base = (char *)msg, .iov_len = msg ? strlen(msg) : 0},
};
assert(nbd_err);
- trace_nbd_co_send_structured_error(handle, nbd_err,
- nbd_err_lookup(nbd_err), msg ? msg : "");
- set_be_chunk(&chunk.h, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_ERROR, handle,
- sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
+ trace_nbd_co_send_chunk_error(handle, nbd_err,
+ nbd_err_lookup(nbd_err), msg ? msg : "");
+ set_be_chunk(client, iov, 3, NBD_REPLY_FLAG_DONE,
+ NBD_REPLY_TYPE_ERROR, handle);
stl_be_p(&chunk.error, nbd_err);
- stw_be_p(&chunk.message_length, iov[1].iov_len);
+ stw_be_p(&chunk.message_length, iov[2].iov_len);
- return nbd_co_send_iov(client, iov, 1 + !!iov[1].iov_len, errp);
+ return nbd_co_send_iov(client, iov, 3, errp);
}
/* Do a sparse read and send the structured reply to the client.
@@ -2013,27 +2037,27 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
char *msg = g_strdup_printf("unable to check for holes: %s",
strerror(-status));
- ret = nbd_co_send_structured_error(client, handle, -status, msg,
- errp);
+ ret = nbd_co_send_chunk_error(client, handle, -status, msg, errp);
g_free(msg);
return ret;
}
assert(pnum && pnum <= size - progress);
final = progress + pnum == size;
if (status & BDRV_BLOCK_ZERO) {
+ NBDReply hdr;
NBDStructuredReadHole chunk;
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
};
- trace_nbd_co_send_structured_read_hole(handle, offset + progress,
- pnum);
- set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_OFFSET_HOLE,
- handle, sizeof(chunk) - sizeof(chunk.h));
+ trace_nbd_co_send_chunk_read_hole(handle, offset + progress, pnum);
+ set_be_chunk(client, iov, 2,
+ final ? NBD_REPLY_FLAG_DONE : 0,
+ NBD_REPLY_TYPE_OFFSET_HOLE, handle);
stq_be_p(&chunk.offset, offset + progress);
stl_be_p(&chunk.length, pnum);
- ret = nbd_co_send_iov(client, iov, 1, errp);
+ ret = nbd_co_send_iov(client, iov, 2, errp);
} else {
ret = blk_co_pread(exp->common.blk, offset + progress, pnum,
data + progress, 0);
@@ -2041,9 +2065,8 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
error_setg_errno(errp, -ret, "reading from file failed");
break;
}
- ret = nbd_co_send_structured_read(client, handle, offset + progress,
- data + progress, pnum, final,
- errp);
+ ret = nbd_co_send_chunk_read(client, handle, offset + progress,
+ data + progress, pnum, final, errp);
}
if (ret < 0) {
@@ -2199,8 +2222,10 @@ static int coroutine_fn
nbd_co_send_extents(NBDClient *client, uint64_t handle, NBDExtentArray *ea,
bool last, uint32_t context_id, Error **errp)
{
+ NBDReply hdr;
NBDStructuredMeta chunk;
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
{.iov_base = ea->extents, .iov_len = ea->count * sizeof(ea->extents[0])}
};
@@ -2209,12 +2234,11 @@ nbd_co_send_extents(NBDClient *client, uint64_t handle, NBDExtentArray *ea,
trace_nbd_co_send_extents(handle, ea->count, context_id, ea->total_length,
last);
- set_be_chunk(&chunk.h, last ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_BLOCK_STATUS,
- handle, sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
+ set_be_chunk(client, iov, 3, last ? NBD_REPLY_FLAG_DONE : 0,
+ NBD_REPLY_TYPE_BLOCK_STATUS, handle);
stl_be_p(&chunk.context_id, context_id);
- return nbd_co_send_iov(client, iov, 2, errp);
+ return nbd_co_send_iov(client, iov, 3, errp);
}
/* Get block status from the exported device and send it to the client */
@@ -2235,8 +2259,8 @@ coroutine_fn nbd_co_send_block_status(NBDClient *client, uint64_t handle,
ret = blockalloc_to_extents(blk, offset, length, ea);
}
if (ret < 0) {
- return nbd_co_send_structured_error(
- client, handle, -ret, "can't get block status", errp);
+ return nbd_co_send_chunk_error(client, handle, -ret,
+ "can't get block status", errp);
}
return nbd_co_send_extents(client, handle, ea, last, context_id, errp);
@@ -2408,8 +2432,7 @@ static coroutine_fn int nbd_send_generic_reply(NBDClient *client,
Error **errp)
{
if (client->structured_reply && ret < 0) {
- return nbd_co_send_structured_error(client, handle, -ret, error_msg,
- errp);
+ return nbd_co_send_chunk_error(client, handle, -ret, error_msg, errp);
} else {
return nbd_co_send_simple_reply(client, handle, ret < 0 ? -ret : 0,
NULL, 0, errp);
@@ -2451,11 +2474,11 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
if (client->structured_reply) {
if (request->len) {
- return nbd_co_send_structured_read(client, request->handle,
- request->from, data,
- request->len, true, errp);
+ return nbd_co_send_chunk_read(client, request->handle,
+ request->from, data,
+ request->len, true, errp);
} else {
- return nbd_co_send_structured_done(client, request->handle, errp);
+ return nbd_co_send_chunk_done(client, request->handle, errp);
}
} else {
return nbd_co_send_simple_reply(client, request->handle, 0,
diff --git a/nbd/trace-events b/nbd/trace-events
index b7032ca2778..50ca05a9e22 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -64,11 +64,11 @@ nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from
nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p"
nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p"
nbd_co_send_simple_reply(uint64_t handle, uint32_t error, const char *errname, int len) "Send simple reply: handle = %" PRIu64 ", error = %" PRIu32 " (%s), len = %d"
-nbd_co_send_structured_done(uint64_t handle) "Send structured reply done: handle = %" PRIu64
-nbd_co_send_structured_read(uint64_t handle, uint64_t offset, void *data, size_t size) "Send structured read data reply: handle = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
-nbd_co_send_structured_read_hole(uint64_t handle, uint64_t offset, size_t size) "Send structured read hole reply: handle = %" PRIu64 ", offset = %" PRIu64 ", len = %zu"
+nbd_co_send_chunk_done(uint64_t handle) "Send structured reply done: handle = %" PRIu64
+nbd_co_send_chunk_read(uint64_t handle, uint64_t offset, void *data, size_t size) "Send structured read data reply: handle = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
+nbd_co_send_chunk_read_hole(uint64_t handle, uint64_t offset, size_t size) "Send structured read hole reply: handle = %" PRIu64 ", offset = %" PRIu64 ", len = %zu"
nbd_co_send_extents(uint64_t handle, unsigned int extents, uint32_t id, uint64_t length, int last) "Send block status reply: handle = %" PRIu64 ", extents = %u, context = %d (extents cover %" PRIu64 " bytes, last chunk = %d)"
-nbd_co_send_structured_error(uint64_t handle, int err, const char *errname, const char *msg) "Send structured error reply: handle = %" PRIu64 ", error = %d (%s), msg = '%s'"
+nbd_co_send_chunk_error(uint64_t handle, int err, const char *errname, const char *msg) "Send structured error reply: handle = %" PRIu64 ", error = %d (%s), msg = '%s'"
nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *name) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16 " (%s)"
nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
nbd_co_receive_align_compliance(const char *op, uint64_t from, uint32_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx32 ", align=0x%" PRIx32
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 10/14] nbd/server: Refactor to pass full request around
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (8 preceding siblings ...)
2023-07-19 20:27 ` [PULL 09/14] nbd/server: Prepare for alternate-size headers Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 11/14] nbd: s/handle/cookie/ to match NBD spec Eric Blake
` (4 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Vladimir Sementsov-Ogievskiy, open list:Network Block Dev...
Part of NBD's 64-bit headers extension involves passing the client's
requested offset back as part of the reply header (one reason it
stated for this change: converting absolute offsets stored in
NBD_REPLY_TYPE_OFFSET_DATA to relative offsets within the buffer is
easier if the absolute offset of the buffer is also available). This
is a refactoring patch to pass the full request around the reply
stack, rather than just the handle, so that later patches can then
access request->from when extended headers are active. Meanwhile,
this patch enables us to now assert that simple replies are only
attempted when appropriate, and otherwise has no semantic change.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230608135653.2918540-5-eblake@redhat.com>
---
nbd/server.c | 114 ++++++++++++++++++++++++++-------------------------
1 file changed, 59 insertions(+), 55 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 6698ab46365..26b27d69202 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1893,7 +1893,7 @@ static inline void set_be_simple_reply(NBDSimpleReply *reply, uint64_t error,
}
static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
- uint64_t handle,
+ NBDRequest *request,
uint32_t error,
void *data,
size_t len,
@@ -1907,9 +1907,10 @@ static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
};
assert(!len || !nbd_err);
- trace_nbd_co_send_simple_reply(handle, nbd_err, nbd_err_lookup(nbd_err),
- len);
- set_be_simple_reply(&reply, nbd_err, handle);
+ assert(!client->structured_reply || request->type != NBD_CMD_READ);
+ trace_nbd_co_send_simple_reply(request->handle, nbd_err,
+ nbd_err_lookup(nbd_err), len);
+ set_be_simple_reply(&reply, nbd_err, request->handle);
return nbd_co_send_iov(client, iov, 2, errp);
}
@@ -1924,7 +1925,7 @@ static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
*/
static inline void set_be_chunk(NBDClient *client, struct iovec *iov,
size_t niov, uint16_t flags, uint16_t type,
- uint64_t handle)
+ NBDRequest *request)
{
/* TODO - handle structured vs. extended replies */
NBDStructuredReplyChunk *chunk = iov->iov_base;
@@ -1939,12 +1940,12 @@ static inline void set_be_chunk(NBDClient *client, struct iovec *iov,
stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
stw_be_p(&chunk->flags, flags);
stw_be_p(&chunk->type, type);
- stq_be_p(&chunk->handle, handle);
+ stq_be_p(&chunk->handle, request->handle);
stl_be_p(&chunk->length, length);
}
static int coroutine_fn nbd_co_send_chunk_done(NBDClient *client,
- uint64_t handle,
+ NBDRequest *request,
Error **errp)
{
NBDReply hdr;
@@ -1952,15 +1953,15 @@ static int coroutine_fn nbd_co_send_chunk_done(NBDClient *client,
{.iov_base = &hdr},
};
- trace_nbd_co_send_chunk_done(handle);
+ trace_nbd_co_send_chunk_done(request->handle);
set_be_chunk(client, iov, 1, NBD_REPLY_FLAG_DONE,
- NBD_REPLY_TYPE_NONE, handle);
+ NBD_REPLY_TYPE_NONE, request);
return nbd_co_send_iov(client, iov, 1, errp);
}
static int coroutine_fn nbd_co_send_chunk_read(NBDClient *client,
- uint64_t handle,
+ NBDRequest *request,
uint64_t offset,
void *data,
size_t size,
@@ -1976,16 +1977,16 @@ static int coroutine_fn nbd_co_send_chunk_read(NBDClient *client,
};
assert(size);
- trace_nbd_co_send_chunk_read(handle, offset, data, size);
+ trace_nbd_co_send_chunk_read(request->handle, offset, data, size);
set_be_chunk(client, iov, 3, final ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_OFFSET_DATA, handle);
+ NBD_REPLY_TYPE_OFFSET_DATA, request);
stq_be_p(&chunk.offset, offset);
return nbd_co_send_iov(client, iov, 3, errp);
}
-
+/*ebb*/
static int coroutine_fn nbd_co_send_chunk_error(NBDClient *client,
- uint64_t handle,
+ NBDRequest *request,
uint32_t error,
const char *msg,
Error **errp)
@@ -2000,10 +2001,10 @@ static int coroutine_fn nbd_co_send_chunk_error(NBDClient *client,
};
assert(nbd_err);
- trace_nbd_co_send_chunk_error(handle, nbd_err,
+ trace_nbd_co_send_chunk_error(request->handle, nbd_err,
nbd_err_lookup(nbd_err), msg ? msg : "");
set_be_chunk(client, iov, 3, NBD_REPLY_FLAG_DONE,
- NBD_REPLY_TYPE_ERROR, handle);
+ NBD_REPLY_TYPE_ERROR, request);
stl_be_p(&chunk.error, nbd_err);
stw_be_p(&chunk.message_length, iov[2].iov_len);
@@ -2015,7 +2016,7 @@ static int coroutine_fn nbd_co_send_chunk_error(NBDClient *client,
* reported to the client, at which point this function succeeds.
*/
static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
- uint64_t handle,
+ NBDRequest *request,
uint64_t offset,
uint8_t *data,
size_t size,
@@ -2037,7 +2038,7 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
char *msg = g_strdup_printf("unable to check for holes: %s",
strerror(-status));
- ret = nbd_co_send_chunk_error(client, handle, -status, msg, errp);
+ ret = nbd_co_send_chunk_error(client, request, -status, msg, errp);
g_free(msg);
return ret;
}
@@ -2051,10 +2052,11 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
};
- trace_nbd_co_send_chunk_read_hole(handle, offset + progress, pnum);
+ trace_nbd_co_send_chunk_read_hole(request->handle,
+ offset + progress, pnum);
set_be_chunk(client, iov, 2,
final ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_OFFSET_HOLE, handle);
+ NBD_REPLY_TYPE_OFFSET_HOLE, request);
stq_be_p(&chunk.offset, offset + progress);
stl_be_p(&chunk.length, pnum);
ret = nbd_co_send_iov(client, iov, 2, errp);
@@ -2065,7 +2067,7 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
error_setg_errno(errp, -ret, "reading from file failed");
break;
}
- ret = nbd_co_send_chunk_read(client, handle, offset + progress,
+ ret = nbd_co_send_chunk_read(client, request, offset + progress,
data + progress, pnum, final, errp);
}
@@ -2219,7 +2221,7 @@ static int coroutine_fn blockalloc_to_extents(BlockBackend *blk,
* @last controls whether NBD_REPLY_FLAG_DONE is sent.
*/
static int coroutine_fn
-nbd_co_send_extents(NBDClient *client, uint64_t handle, NBDExtentArray *ea,
+nbd_co_send_extents(NBDClient *client, NBDRequest *request, NBDExtentArray *ea,
bool last, uint32_t context_id, Error **errp)
{
NBDReply hdr;
@@ -2232,10 +2234,10 @@ nbd_co_send_extents(NBDClient *client, uint64_t handle, NBDExtentArray *ea,
nbd_extent_array_convert_to_be(ea);
- trace_nbd_co_send_extents(handle, ea->count, context_id, ea->total_length,
- last);
+ trace_nbd_co_send_extents(request->handle, ea->count, context_id,
+ ea->total_length, last);
set_be_chunk(client, iov, 3, last ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_BLOCK_STATUS, handle);
+ NBD_REPLY_TYPE_BLOCK_STATUS, request);
stl_be_p(&chunk.context_id, context_id);
return nbd_co_send_iov(client, iov, 3, errp);
@@ -2243,7 +2245,7 @@ nbd_co_send_extents(NBDClient *client, uint64_t handle, NBDExtentArray *ea,
/* Get block status from the exported device and send it to the client */
static int
-coroutine_fn nbd_co_send_block_status(NBDClient *client, uint64_t handle,
+coroutine_fn nbd_co_send_block_status(NBDClient *client, NBDRequest *request,
BlockBackend *blk, uint64_t offset,
uint32_t length, bool dont_fragment,
bool last, uint32_t context_id,
@@ -2259,11 +2261,11 @@ coroutine_fn nbd_co_send_block_status(NBDClient *client, uint64_t handle,
ret = blockalloc_to_extents(blk, offset, length, ea);
}
if (ret < 0) {
- return nbd_co_send_chunk_error(client, handle, -ret,
+ return nbd_co_send_chunk_error(client, request, -ret,
"can't get block status", errp);
}
- return nbd_co_send_extents(client, handle, ea, last, context_id, errp);
+ return nbd_co_send_extents(client, request, ea, last, context_id, errp);
}
/* Populate @ea from a dirty bitmap. */
@@ -2298,17 +2300,20 @@ static void bitmap_to_extents(BdrvDirtyBitmap *bitmap,
bdrv_dirty_bitmap_unlock(bitmap);
}
-static int coroutine_fn nbd_co_send_bitmap(NBDClient *client, uint64_t handle,
- BdrvDirtyBitmap *bitmap, uint64_t offset,
- uint32_t length, bool dont_fragment, bool last,
- uint32_t context_id, Error **errp)
+static int coroutine_fn nbd_co_send_bitmap(NBDClient *client,
+ NBDRequest *request,
+ BdrvDirtyBitmap *bitmap,
+ uint64_t offset,
+ uint32_t length, bool dont_fragment,
+ bool last, uint32_t context_id,
+ Error **errp)
{
unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS;
g_autoptr(NBDExtentArray) ea = nbd_extent_array_new(nb_extents);
bitmap_to_extents(bitmap, offset, length, ea);
- return nbd_co_send_extents(client, handle, ea, last, context_id, errp);
+ return nbd_co_send_extents(client, request, ea, last, context_id, errp);
}
/* nbd_co_receive_request
@@ -2426,15 +2431,15 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
* Returns 0 if connection is still live, -errno on failure to talk to client
*/
static coroutine_fn int nbd_send_generic_reply(NBDClient *client,
- uint64_t handle,
+ NBDRequest *request,
int ret,
const char *error_msg,
Error **errp)
{
if (client->structured_reply && ret < 0) {
- return nbd_co_send_chunk_error(client, handle, -ret, error_msg, errp);
+ return nbd_co_send_chunk_error(client, request, -ret, error_msg, errp);
} else {
- return nbd_co_send_simple_reply(client, handle, ret < 0 ? -ret : 0,
+ return nbd_co_send_simple_reply(client, request, ret < 0 ? -ret : 0,
NULL, 0, errp);
}
}
@@ -2454,7 +2459,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
if (request->flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->common.blk);
if (ret < 0) {
- return nbd_send_generic_reply(client, request->handle, ret,
+ return nbd_send_generic_reply(client, request, ret,
"flush failed", errp);
}
}
@@ -2462,26 +2467,25 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
if (client->structured_reply && !(request->flags & NBD_CMD_FLAG_DF) &&
request->len)
{
- return nbd_co_send_sparse_read(client, request->handle, request->from,
+ return nbd_co_send_sparse_read(client, request, request->from,
data, request->len, errp);
}
ret = blk_co_pread(exp->common.blk, request->from, request->len, data, 0);
if (ret < 0) {
- return nbd_send_generic_reply(client, request->handle, ret,
+ return nbd_send_generic_reply(client, request, ret,
"reading from file failed", errp);
}
if (client->structured_reply) {
if (request->len) {
- return nbd_co_send_chunk_read(client, request->handle,
- request->from, data,
+ return nbd_co_send_chunk_read(client, request, request->from, data,
request->len, true, errp);
} else {
- return nbd_co_send_chunk_done(client, request->handle, errp);
+ return nbd_co_send_chunk_done(client, request, errp);
}
} else {
- return nbd_co_send_simple_reply(client, request->handle, 0,
+ return nbd_co_send_simple_reply(client, request, 0,
data, request->len, errp);
}
}
@@ -2504,7 +2508,7 @@ static coroutine_fn int nbd_do_cmd_cache(NBDClient *client, NBDRequest *request,
ret = blk_co_preadv(exp->common.blk, request->from, request->len,
NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH);
- return nbd_send_generic_reply(client, request->handle, ret,
+ return nbd_send_generic_reply(client, request, ret,
"caching data failed", errp);
}
@@ -2535,7 +2539,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
}
ret = blk_co_pwrite(exp->common.blk, request->from, request->len, data,
flags);
- return nbd_send_generic_reply(client, request->handle, ret,
+ return nbd_send_generic_reply(client, request, ret,
"writing to file failed", errp);
case NBD_CMD_WRITE_ZEROES:
@@ -2551,7 +2555,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
}
ret = blk_co_pwrite_zeroes(exp->common.blk, request->from, request->len,
flags);
- return nbd_send_generic_reply(client, request->handle, ret,
+ return nbd_send_generic_reply(client, request, ret,
"writing to file failed", errp);
case NBD_CMD_DISC:
@@ -2560,7 +2564,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
case NBD_CMD_FLUSH:
ret = blk_co_flush(exp->common.blk);
- return nbd_send_generic_reply(client, request->handle, ret,
+ return nbd_send_generic_reply(client, request, ret,
"flush failed", errp);
case NBD_CMD_TRIM:
@@ -2568,12 +2572,12 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (ret >= 0 && request->flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->common.blk);
}
- return nbd_send_generic_reply(client, request->handle, ret,
+ return nbd_send_generic_reply(client, request, ret,
"discard failed", errp);
case NBD_CMD_BLOCK_STATUS:
if (!request->len) {
- return nbd_send_generic_reply(client, request->handle, -EINVAL,
+ return nbd_send_generic_reply(client, request, -EINVAL,
"need non-zero length", errp);
}
if (client->export_meta.count) {
@@ -2581,7 +2585,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
int contexts_remaining = client->export_meta.count;
if (client->export_meta.base_allocation) {
- ret = nbd_co_send_block_status(client, request->handle,
+ ret = nbd_co_send_block_status(client, request,
exp->common.blk,
request->from,
request->len, dont_fragment,
@@ -2594,7 +2598,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
}
if (client->export_meta.allocation_depth) {
- ret = nbd_co_send_block_status(client, request->handle,
+ ret = nbd_co_send_block_status(client, request,
exp->common.blk,
request->from, request->len,
dont_fragment,
@@ -2610,7 +2614,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (!client->export_meta.bitmaps[i]) {
continue;
}
- ret = nbd_co_send_bitmap(client, request->handle,
+ ret = nbd_co_send_bitmap(client, request,
client->exp->export_bitmaps[i],
request->from, request->len,
dont_fragment, !--contexts_remaining,
@@ -2624,7 +2628,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
return 0;
} else {
- return nbd_send_generic_reply(client, request->handle, -EINVAL,
+ return nbd_send_generic_reply(client, request, -EINVAL,
"CMD_BLOCK_STATUS not negotiated",
errp);
}
@@ -2632,7 +2636,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
default:
msg = g_strdup_printf("invalid request type (%" PRIu32 ") received",
request->type);
- ret = nbd_send_generic_reply(client, request->handle, -EINVAL, msg,
+ ret = nbd_send_generic_reply(client, request, -EINVAL, msg,
errp);
g_free(msg);
return ret;
@@ -2695,7 +2699,7 @@ static coroutine_fn void nbd_trip(void *opaque)
Error *export_err = local_err;
local_err = NULL;
- ret = nbd_send_generic_reply(client, request.handle, -EINVAL,
+ ret = nbd_send_generic_reply(client, &request, -EINVAL,
error_get_pretty(export_err), &local_err);
error_free(export_err);
} else {
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 11/14] nbd: s/handle/cookie/ to match NBD spec
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (9 preceding siblings ...)
2023-07-19 20:27 ` [PULL 10/14] nbd/server: Refactor to pass full request around Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 12/14] nbd/client: Simplify cookie vs. index computation Eric Blake
` (3 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Kevin Wolf, Hanna Reitz,
open list:Network Block Dev...
Externally, libnbd exposed the 64-bit opaque marker for each client
NBD packet as the "cookie", because it was less confusing when
contrasted with 'struct nbd_handle *' holding all libnbd state. It
also avoids confusion between the noun 'handle' as a way to identify a
packet and the verb 'handle' for reacting to things like signals.
Upstream NBD changed their spec to favor the name "cookie" based on
libnbd's recommendations[1], so we can do likewise.
[1] https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230608135653.2918540-6-eblake@redhat.com>
[eblake: typo fix]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
include/block/nbd.h | 11 +++---
block/nbd.c | 96 +++++++++++++++++++++++----------------------
nbd/client.c | 14 +++----
nbd/server.c | 29 +++++++-------
nbd/trace-events | 22 +++++------
5 files changed, 87 insertions(+), 85 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index ee71af099a3..fb935d56e57 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -59,7 +59,7 @@ typedef struct NBDOptionReplyMetaContext {
* request and reply!
*/
typedef struct NBDRequest {
- uint64_t handle;
+ uint64_t cookie;
uint64_t from;
uint32_t len;
uint16_t flags; /* NBD_CMD_FLAG_* */
@@ -69,7 +69,7 @@ typedef struct NBDRequest {
typedef struct NBDSimpleReply {
uint32_t magic; /* NBD_SIMPLE_REPLY_MAGIC */
uint32_t error;
- uint64_t handle;
+ uint64_t cookie;
} QEMU_PACKED NBDSimpleReply;
/* Header of all structured replies */
@@ -77,7 +77,7 @@ typedef struct NBDStructuredReplyChunk {
uint32_t magic; /* NBD_STRUCTURED_REPLY_MAGIC */
uint16_t flags; /* combination of NBD_REPLY_FLAG_* */
uint16_t type; /* NBD_REPLY_TYPE_* */
- uint64_t handle; /* request handle */
+ uint64_t cookie; /* request handle */
uint32_t length; /* length of payload */
} QEMU_PACKED NBDStructuredReplyChunk;
@@ -85,13 +85,14 @@ typedef union NBDReply {
NBDSimpleReply simple;
NBDStructuredReplyChunk structured;
struct {
- /* @magic and @handle fields have the same offset and size both in
+ /*
+ * @magic and @cookie fields have the same offset and size both in
* simple reply and structured reply chunk, so let them be accessible
* without ".simple." or ".structured." specification
*/
uint32_t magic;
uint32_t _skip;
- uint64_t handle;
+ uint64_t cookie;
} QEMU_PACKED;
} NBDReply;
diff --git a/block/nbd.c b/block/nbd.c
index 5aef5cb6bd5..be3c46c6fee 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1,8 +1,8 @@
/*
- * QEMU Block driver for NBD
+ * QEMU Block driver for NBD
*
* Copyright (c) 2019 Virtuozzo International GmbH.
- * Copyright (C) 2016 Red Hat, Inc.
+ * Copyright Red Hat
* Copyright (C) 2008 Bull S.A.S.
* Author: Laurent Vivier <Laurent.Vivier@bull.net>
*
@@ -50,8 +50,8 @@
#define EN_OPTSTR ":exportname="
#define MAX_NBD_REQUESTS 16
-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
-#define INDEX_TO_HANDLE(bs, index) ((index) ^ (uint64_t)(intptr_t)(bs))
+#define COOKIE_TO_INDEX(bs, cookie) ((cookie) ^ (uint64_t)(intptr_t)(bs))
+#define INDEX_TO_COOKIE(bs, index) ((index) ^ (uint64_t)(intptr_t)(bs))
typedef struct {
Coroutine *coroutine;
@@ -417,25 +417,25 @@ static void coroutine_fn GRAPH_RDLOCK nbd_reconnect_attempt(BDRVNBDState *s)
reconnect_delay_timer_del(s);
}
-static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
+static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
{
int ret;
- uint64_t ind = HANDLE_TO_INDEX(s, handle), ind2;
+ uint64_t ind = COOKIE_TO_INDEX(s, cookie), ind2;
QEMU_LOCK_GUARD(&s->receive_mutex);
while (true) {
- if (s->reply.handle == handle) {
+ if (s->reply.cookie == cookie) {
/* We are done */
return 0;
}
- if (s->reply.handle != 0) {
+ if (s->reply.cookie != 0) {
/*
* Some other request is being handled now. It should already be
- * woken by whoever set s->reply.handle (or never wait in this
+ * woken by whoever set s->reply.cookie (or never wait in this
* yield). So, we should not wake it here.
*/
- ind2 = HANDLE_TO_INDEX(s, s->reply.handle);
+ ind2 = COOKIE_TO_INDEX(s, s->reply.cookie);
assert(!s->requests[ind2].receiving);
s->requests[ind].receiving = true;
@@ -445,9 +445,9 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
/*
* We may be woken for 2 reasons:
* 1. From this function, executing in parallel coroutine, when our
- * handle is received.
+ * cookie is received.
* 2. From nbd_co_receive_one_chunk(), when previous request is
- * finished and s->reply.handle set to 0.
+ * finished and s->reply.cookie set to 0.
* Anyway, it's OK to lock the mutex and go to the next iteration.
*/
@@ -456,8 +456,8 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
continue;
}
- /* We are under mutex and handle is 0. We have to do the dirty work. */
- assert(s->reply.handle == 0);
+ /* We are under mutex and cookie is 0. We have to do the dirty work. */
+ assert(s->reply.cookie == 0);
ret = nbd_receive_reply(s->bs, s->ioc, &s->reply, NULL);
if (ret <= 0) {
ret = ret ? ret : -EIO;
@@ -468,12 +468,12 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
nbd_channel_error(s, -EINVAL);
return -EINVAL;
}
- ind2 = HANDLE_TO_INDEX(s, s->reply.handle);
+ ind2 = COOKIE_TO_INDEX(s, s->reply.cookie);
if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].coroutine) {
nbd_channel_error(s, -EINVAL);
return -EINVAL;
}
- if (s->reply.handle == handle) {
+ if (s->reply.cookie == cookie) {
/* We are done */
return 0;
}
@@ -519,7 +519,7 @@ nbd_co_send_request(BlockDriverState *bs, NBDRequest *request,
qemu_mutex_unlock(&s->requests_lock);
qemu_co_mutex_lock(&s->send_mutex);
- request->handle = INDEX_TO_HANDLE(s, i);
+ request->cookie = INDEX_TO_COOKIE(s, i);
assert(s->ioc);
@@ -828,11 +828,11 @@ static coroutine_fn int nbd_co_receive_structured_payload(
* corresponding to the server's error reply), and errp is unchanged.
*/
static coroutine_fn int nbd_co_do_receive_one_chunk(
- BDRVNBDState *s, uint64_t handle, bool only_structured,
+ BDRVNBDState *s, uint64_t cookie, bool only_structured,
int *request_ret, QEMUIOVector *qiov, void **payload, Error **errp)
{
int ret;
- int i = HANDLE_TO_INDEX(s, handle);
+ int i = COOKIE_TO_INDEX(s, cookie);
void *local_payload = NULL;
NBDStructuredReplyChunk *chunk;
@@ -841,14 +841,14 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
}
*request_ret = 0;
- ret = nbd_receive_replies(s, handle);
+ ret = nbd_receive_replies(s, cookie);
if (ret < 0) {
error_setg(errp, "Connection closed");
return -EIO;
}
assert(s->ioc);
- assert(s->reply.handle == handle);
+ assert(s->reply.cookie == cookie);
if (nbd_reply_is_simple(&s->reply)) {
if (only_structured) {
@@ -918,11 +918,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
* Return value is a fatal error code or normal nbd reply error code
*/
static coroutine_fn int nbd_co_receive_one_chunk(
- BDRVNBDState *s, uint64_t handle, bool only_structured,
+ BDRVNBDState *s, uint64_t cookie, bool only_structured,
int *request_ret, QEMUIOVector *qiov, NBDReply *reply, void **payload,
Error **errp)
{
- int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured,
+ int ret = nbd_co_do_receive_one_chunk(s, cookie, only_structured,
request_ret, qiov, payload, errp);
if (ret < 0) {
@@ -932,7 +932,7 @@ static coroutine_fn int nbd_co_receive_one_chunk(
/* For assert at loop start in nbd_connection_entry */
*reply = s->reply;
}
- s->reply.handle = 0;
+ s->reply.cookie = 0;
nbd_recv_coroutines_wake(s);
@@ -975,10 +975,10 @@ static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
* NBD_FOREACH_REPLY_CHUNK
* The pointer stored in @payload requires g_free() to free it.
*/
-#define NBD_FOREACH_REPLY_CHUNK(s, iter, handle, structured, \
+#define NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, structured, \
qiov, reply, payload) \
for (iter = (NBDReplyChunkIter) { .only_structured = structured }; \
- nbd_reply_chunk_iter_receive(s, &iter, handle, qiov, reply, payload);)
+ nbd_reply_chunk_iter_receive(s, &iter, cookie, qiov, reply, payload);)
/*
* nbd_reply_chunk_iter_receive
@@ -986,7 +986,7 @@ static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
*/
static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
NBDReplyChunkIter *iter,
- uint64_t handle,
+ uint64_t cookie,
QEMUIOVector *qiov,
NBDReply *reply,
void **payload)
@@ -1005,7 +1005,7 @@ static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
reply = &local_reply;
}
- ret = nbd_co_receive_one_chunk(s, handle, iter->only_structured,
+ ret = nbd_co_receive_one_chunk(s, cookie, iter->only_structured,
&request_ret, qiov, reply, payload,
&local_err);
if (ret < 0) {
@@ -1038,7 +1038,7 @@ static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
break_loop:
qemu_mutex_lock(&s->requests_lock);
- s->requests[HANDLE_TO_INDEX(s, handle)].coroutine = NULL;
+ s->requests[COOKIE_TO_INDEX(s, cookie)].coroutine = NULL;
s->in_flight--;
qemu_co_queue_next(&s->free_sema);
qemu_mutex_unlock(&s->requests_lock);
@@ -1046,12 +1046,13 @@ break_loop:
return false;
}
-static int coroutine_fn nbd_co_receive_return_code(BDRVNBDState *s, uint64_t handle,
- int *request_ret, Error **errp)
+static int coroutine_fn
+nbd_co_receive_return_code(BDRVNBDState *s, uint64_t cookie,
+ int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
- NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, NULL, NULL) {
+ NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, false, NULL, NULL, NULL) {
/* nbd_reply_chunk_iter_receive does all the work */
}
@@ -1060,16 +1061,17 @@ static int coroutine_fn nbd_co_receive_return_code(BDRVNBDState *s, uint64_t han
return iter.ret;
}
-static int coroutine_fn nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t handle,
- uint64_t offset, QEMUIOVector *qiov,
- int *request_ret, Error **errp)
+static int coroutine_fn
+nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t cookie,
+ uint64_t offset, QEMUIOVector *qiov,
+ int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
NBDReply reply;
void *payload = NULL;
Error *local_err = NULL;
- NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply,
+ NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, s->info.structured_reply,
qiov, &reply, &payload)
{
int ret;
@@ -1112,10 +1114,10 @@ static int coroutine_fn nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t h
return iter.ret;
}
-static int coroutine_fn nbd_co_receive_blockstatus_reply(BDRVNBDState *s,
- uint64_t handle, uint64_t length,
- NBDExtent *extent,
- int *request_ret, Error **errp)
+static int coroutine_fn
+nbd_co_receive_blockstatus_reply(BDRVNBDState *s, uint64_t cookie,
+ uint64_t length, NBDExtent *extent,
+ int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
NBDReply reply;
@@ -1124,7 +1126,7 @@ static int coroutine_fn nbd_co_receive_blockstatus_reply(BDRVNBDState *s,
bool received = false;
assert(!extent->length);
- NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, &reply, &payload) {
+ NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, false, NULL, &reply, &payload) {
int ret;
NBDStructuredReplyChunk *chunk = &reply.structured;
@@ -1194,11 +1196,11 @@ nbd_co_request(BlockDriverState *bs, NBDRequest *request,
continue;
}
- ret = nbd_co_receive_return_code(s, request->handle,
+ ret = nbd_co_receive_return_code(s, request->cookie,
&request_ret, &local_err);
if (local_err) {
trace_nbd_co_request_fail(request->from, request->len,
- request->handle, request->flags,
+ request->cookie, request->flags,
request->type,
nbd_cmd_lookup(request->type),
ret, error_get_pretty(local_err));
@@ -1253,10 +1255,10 @@ nbd_client_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
continue;
}
- ret = nbd_co_receive_cmdread_reply(s, request.handle, offset, qiov,
+ ret = nbd_co_receive_cmdread_reply(s, request.cookie, offset, qiov,
&request_ret, &local_err);
if (local_err) {
- trace_nbd_co_request_fail(request.from, request.len, request.handle,
+ trace_nbd_co_request_fail(request.from, request.len, request.cookie,
request.flags, request.type,
nbd_cmd_lookup(request.type),
ret, error_get_pretty(local_err));
@@ -1411,11 +1413,11 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
continue;
}
- ret = nbd_co_receive_blockstatus_reply(s, request.handle, bytes,
+ ret = nbd_co_receive_blockstatus_reply(s, request.cookie, bytes,
&extent, &request_ret,
&local_err);
if (local_err) {
- trace_nbd_co_request_fail(request.from, request.len, request.handle,
+ trace_nbd_co_request_fail(request.from, request.len, request.cookie,
request.flags, request.type,
nbd_cmd_lookup(request.type),
ret, error_get_pretty(local_err));
diff --git a/nbd/client.c b/nbd/client.c
index ff75722e487..ea3590ca3d0 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2019 Red Hat, Inc.
+ * Copyright Red Hat
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
* Network Block Device Client Side
@@ -1350,14 +1350,14 @@ int nbd_send_request(QIOChannel *ioc, NBDRequest *request)
{
uint8_t buf[NBD_REQUEST_SIZE];
- trace_nbd_send_request(request->from, request->len, request->handle,
+ trace_nbd_send_request(request->from, request->len, request->cookie,
request->flags, request->type,
nbd_cmd_lookup(request->type));
stl_be_p(buf, NBD_REQUEST_MAGIC);
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 + 8, request->cookie);
stq_be_p(buf + 16, request->from);
stl_be_p(buf + 24, request->len);
@@ -1383,7 +1383,7 @@ static int nbd_receive_simple_reply(QIOChannel *ioc, NBDSimpleReply *reply,
}
reply->error = be32_to_cpu(reply->error);
- reply->handle = be64_to_cpu(reply->handle);
+ reply->cookie = be64_to_cpu(reply->cookie);
return 0;
}
@@ -1410,7 +1410,7 @@ static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
chunk->flags = be16_to_cpu(chunk->flags);
chunk->type = be16_to_cpu(chunk->type);
- chunk->handle = be64_to_cpu(chunk->handle);
+ chunk->cookie = be64_to_cpu(chunk->cookie);
chunk->length = be32_to_cpu(chunk->length);
return 0;
@@ -1487,7 +1487,7 @@ int coroutine_fn nbd_receive_reply(BlockDriverState *bs, QIOChannel *ioc,
}
trace_nbd_receive_simple_reply(reply->simple.error,
nbd_err_lookup(reply->simple.error),
- reply->handle);
+ reply->cookie);
break;
case NBD_STRUCTURED_REPLY_MAGIC:
ret = nbd_receive_structured_reply_chunk(ioc, &reply->structured, errp);
@@ -1497,7 +1497,7 @@ int coroutine_fn nbd_receive_reply(BlockDriverState *bs, QIOChannel *ioc,
type = nbd_reply_type_lookup(reply->structured.type);
trace_nbd_receive_structured_reply_chunk(reply->structured.flags,
reply->structured.type, type,
- reply->structured.handle,
+ reply->structured.cookie,
reply->structured.length);
break;
default:
diff --git a/nbd/server.c b/nbd/server.c
index 26b27d69202..8486b64b15d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1428,7 +1428,7 @@ static int coroutine_fn nbd_receive_request(NBDClient *client, NBDRequest *reque
[ 0 .. 3] magic (NBD_REQUEST_MAGIC)
[ 4 .. 5] flags (NBD_CMD_FLAG_FUA, ...)
[ 6 .. 7] type (NBD_CMD_READ, ...)
- [ 8 .. 15] handle
+ [ 8 .. 15] cookie
[16 .. 23] from
[24 .. 27] len
*/
@@ -1436,7 +1436,7 @@ static int coroutine_fn nbd_receive_request(NBDClient *client, NBDRequest *reque
magic = ldl_be_p(buf);
request->flags = lduw_be_p(buf + 4);
request->type = lduw_be_p(buf + 6);
- request->handle = ldq_be_p(buf + 8);
+ request->cookie = ldq_be_p(buf + 8);
request->from = ldq_be_p(buf + 16);
request->len = ldl_be_p(buf + 24);
@@ -1885,11 +1885,11 @@ static int coroutine_fn nbd_co_send_iov(NBDClient *client, struct iovec *iov,
}
static inline void set_be_simple_reply(NBDSimpleReply *reply, uint64_t error,
- uint64_t handle)
+ uint64_t cookie)
{
stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_MAGIC);
stl_be_p(&reply->error, error);
- stq_be_p(&reply->handle, handle);
+ stq_be_p(&reply->cookie, cookie);
}
static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
@@ -1908,9 +1908,9 @@ static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
assert(!len || !nbd_err);
assert(!client->structured_reply || request->type != NBD_CMD_READ);
- trace_nbd_co_send_simple_reply(request->handle, nbd_err,
+ trace_nbd_co_send_simple_reply(request->cookie, nbd_err,
nbd_err_lookup(nbd_err), len);
- set_be_simple_reply(&reply, nbd_err, request->handle);
+ set_be_simple_reply(&reply, nbd_err, request->cookie);
return nbd_co_send_iov(client, iov, 2, errp);
}
@@ -1940,7 +1940,7 @@ static inline void set_be_chunk(NBDClient *client, struct iovec *iov,
stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
stw_be_p(&chunk->flags, flags);
stw_be_p(&chunk->type, type);
- stq_be_p(&chunk->handle, request->handle);
+ stq_be_p(&chunk->cookie, request->cookie);
stl_be_p(&chunk->length, length);
}
@@ -1953,10 +1953,9 @@ static int coroutine_fn nbd_co_send_chunk_done(NBDClient *client,
{.iov_base = &hdr},
};
- trace_nbd_co_send_chunk_done(request->handle);
+ trace_nbd_co_send_chunk_done(request->cookie);
set_be_chunk(client, iov, 1, NBD_REPLY_FLAG_DONE,
NBD_REPLY_TYPE_NONE, request);
-
return nbd_co_send_iov(client, iov, 1, errp);
}
@@ -1977,7 +1976,7 @@ static int coroutine_fn nbd_co_send_chunk_read(NBDClient *client,
};
assert(size);
- trace_nbd_co_send_chunk_read(request->handle, offset, data, size);
+ trace_nbd_co_send_chunk_read(request->cookie, offset, data, size);
set_be_chunk(client, iov, 3, final ? NBD_REPLY_FLAG_DONE : 0,
NBD_REPLY_TYPE_OFFSET_DATA, request);
stq_be_p(&chunk.offset, offset);
@@ -2001,7 +2000,7 @@ static int coroutine_fn nbd_co_send_chunk_error(NBDClient *client,
};
assert(nbd_err);
- trace_nbd_co_send_chunk_error(request->handle, nbd_err,
+ trace_nbd_co_send_chunk_error(request->cookie, nbd_err,
nbd_err_lookup(nbd_err), msg ? msg : "");
set_be_chunk(client, iov, 3, NBD_REPLY_FLAG_DONE,
NBD_REPLY_TYPE_ERROR, request);
@@ -2052,7 +2051,7 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
};
- trace_nbd_co_send_chunk_read_hole(request->handle,
+ trace_nbd_co_send_chunk_read_hole(request->cookie,
offset + progress, pnum);
set_be_chunk(client, iov, 2,
final ? NBD_REPLY_FLAG_DONE : 0,
@@ -2234,7 +2233,7 @@ nbd_co_send_extents(NBDClient *client, NBDRequest *request, NBDExtentArray *ea,
nbd_extent_array_convert_to_be(ea);
- trace_nbd_co_send_extents(request->handle, ea->count, context_id,
+ trace_nbd_co_send_extents(request->cookie, ea->count, context_id,
ea->total_length, last);
set_be_chunk(client, iov, 3, last ? NBD_REPLY_FLAG_DONE : 0,
NBD_REPLY_TYPE_BLOCK_STATUS, request);
@@ -2337,7 +2336,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
return ret;
}
- trace_nbd_co_receive_request_decode_type(request->handle, request->type,
+ trace_nbd_co_receive_request_decode_type(request->cookie, request->type,
nbd_cmd_lookup(request->type));
if (request->type != NBD_CMD_WRITE) {
@@ -2378,7 +2377,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
}
req->complete = true;
- trace_nbd_co_receive_request_payload_received(request->handle,
+ trace_nbd_co_receive_request_payload_received(request->cookie,
request->len);
}
diff --git a/nbd/trace-events b/nbd/trace-events
index 50ca05a9e22..f19a4d0db39 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -31,9 +31,9 @@ nbd_client_loop(void) "Doing NBD loop"
nbd_client_loop_ret(int ret, const char *error) "NBD loop returned %d: %s"
nbd_client_clear_queue(void) "Clearing NBD queue"
nbd_client_clear_socket(void) "Clearing NBD socket"
-nbd_send_request(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name) "Sending request to server: { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) }"
-nbd_receive_simple_reply(int32_t error, const char *errname, uint64_t handle) "Got simple reply: { .error = %" PRId32 " (%s), handle = %" PRIu64" }"
-nbd_receive_structured_reply_chunk(uint16_t flags, uint16_t type, const char *name, uint64_t handle, uint32_t length) "Got structured reply chunk: { flags = 0x%" PRIx16 ", type = %d (%s), handle = %" PRIu64 ", length = %" PRIu32 " }"
+nbd_send_request(uint64_t from, uint32_t len, uint64_t cookie, uint16_t flags, uint16_t type, const char *name) "Sending request to server: { .from = %" PRIu64", .len = %" PRIu32 ", .cookie = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) }"
+nbd_receive_simple_reply(int32_t error, const char *errname, uint64_t cookie) "Got simple reply: { .error = %" PRId32 " (%s), cookie = %" PRIu64" }"
+nbd_receive_structured_reply_chunk(uint16_t flags, uint16_t type, const char *name, uint64_t cookie, uint32_t length) "Got structured reply chunk: { flags = 0x%" PRIx16 ", type = %d (%s), cookie = %" PRIu64 ", length = %" PRIu32 " }"
# common.c
nbd_unknown_error(int err) "Squashing unexpected error %d to EINVAL"
@@ -63,14 +63,14 @@ nbd_negotiate_success(void) "Negotiation succeeded"
nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from, uint32_t len) "Got request: { magic = 0x%" PRIx32 ", .flags = 0x%" PRIx16 ", .type = 0x%" PRIx16 ", from = %" PRIu64 ", len = %" PRIu32 " }"
nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p"
nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p"
-nbd_co_send_simple_reply(uint64_t handle, uint32_t error, const char *errname, int len) "Send simple reply: handle = %" PRIu64 ", error = %" PRIu32 " (%s), len = %d"
-nbd_co_send_chunk_done(uint64_t handle) "Send structured reply done: handle = %" PRIu64
-nbd_co_send_chunk_read(uint64_t handle, uint64_t offset, void *data, size_t size) "Send structured read data reply: handle = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
-nbd_co_send_chunk_read_hole(uint64_t handle, uint64_t offset, size_t size) "Send structured read hole reply: handle = %" PRIu64 ", offset = %" PRIu64 ", len = %zu"
-nbd_co_send_extents(uint64_t handle, unsigned int extents, uint32_t id, uint64_t length, int last) "Send block status reply: handle = %" PRIu64 ", extents = %u, context = %d (extents cover %" PRIu64 " bytes, last chunk = %d)"
-nbd_co_send_chunk_error(uint64_t handle, int err, const char *errname, const char *msg) "Send structured error reply: handle = %" PRIu64 ", error = %d (%s), msg = '%s'"
-nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *name) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16 " (%s)"
-nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
+nbd_co_send_simple_reply(uint64_t cookie, uint32_t error, const char *errname, int len) "Send simple reply: cookie = %" PRIu64 ", error = %" PRIu32 " (%s), len = %d"
+nbd_co_send_chunk_done(uint64_t cookie) "Send structured reply done: cookie = %" PRIu64
+nbd_co_send_chunk_read(uint64_t cookie, uint64_t offset, void *data, size_t size) "Send structured read data reply: cookie = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
+nbd_co_send_chunk_read_hole(uint64_t cookie, uint64_t offset, size_t size) "Send structured read hole reply: cookie = %" PRIu64 ", offset = %" PRIu64 ", len = %zu"
+nbd_co_send_extents(uint64_t cookie, unsigned int extents, uint32_t id, uint64_t length, int last) "Send block status reply: cookie = %" PRIu64 ", extents = %u, context = %d (extents cover %" PRIu64 " bytes, last chunk = %d)"
+nbd_co_send_chunk_error(uint64_t cookie, int err, const char *errname, const char *msg) "Send structured error reply: cookie = %" PRIu64 ", error = %d (%s), msg = '%s'"
+nbd_co_receive_request_decode_type(uint64_t cookie, uint16_t type, const char *name) "Decoding type: cookie = %" PRIu64 ", type = %" PRIu16 " (%s)"
+nbd_co_receive_request_payload_received(uint64_t cookie, uint32_t len) "Payload received: cookie = %" PRIu64 ", len = %" PRIu32
nbd_co_receive_align_compliance(const char *op, uint64_t from, uint32_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx32 ", align=0x%" PRIx32
nbd_trip(void) "Reading request"
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 12/14] nbd/client: Simplify cookie vs. index computation
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (10 preceding siblings ...)
2023-07-19 20:27 ` [PULL 11/14] nbd: s/handle/cookie/ to match NBD spec Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 13/14] nbd/client: Add safety check on chunk payload length Eric Blake
` (2 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Kevin Wolf, Hanna Reitz,
open list:Network Block Dev...
Our code relies on a sentinel cookie value of zero for deciding when a
packet has been handled, as well as relying on array indices between 0
and MAX_NBD_REQUESTS-1 for dereferencing purposes. As long as we can
symmetrically convert between two forms, there is no reason to go with
the odd choice of using XOR with a random pointer, when we can instead
simplify the mappings with a mere offset of 1.
Using ((uint64_t)-1) as the sentinel instead of NULL such that the two
macros could be entirely eliminated might also be possible, but would
require a more careful audit to find places where we currently rely on
zero-initialization to be interpreted as the sentinel value, so I did
not pursue that course.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230608135653.2918540-7-eblake@redhat.com>
[eblake: enhance commit message]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/nbd.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index be3c46c6fee..5322e66166c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -50,8 +50,8 @@
#define EN_OPTSTR ":exportname="
#define MAX_NBD_REQUESTS 16
-#define COOKIE_TO_INDEX(bs, cookie) ((cookie) ^ (uint64_t)(intptr_t)(bs))
-#define INDEX_TO_COOKIE(bs, index) ((index) ^ (uint64_t)(intptr_t)(bs))
+#define COOKIE_TO_INDEX(cookie) ((cookie) - 1)
+#define INDEX_TO_COOKIE(index) ((index) + 1)
typedef struct {
Coroutine *coroutine;
@@ -420,7 +420,7 @@ static void coroutine_fn GRAPH_RDLOCK nbd_reconnect_attempt(BDRVNBDState *s)
static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
{
int ret;
- uint64_t ind = COOKIE_TO_INDEX(s, cookie), ind2;
+ uint64_t ind = COOKIE_TO_INDEX(cookie), ind2;
QEMU_LOCK_GUARD(&s->receive_mutex);
while (true) {
@@ -435,7 +435,7 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
* woken by whoever set s->reply.cookie (or never wait in this
* yield). So, we should not wake it here.
*/
- ind2 = COOKIE_TO_INDEX(s, s->reply.cookie);
+ ind2 = COOKIE_TO_INDEX(s->reply.cookie);
assert(!s->requests[ind2].receiving);
s->requests[ind].receiving = true;
@@ -468,7 +468,7 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
nbd_channel_error(s, -EINVAL);
return -EINVAL;
}
- ind2 = COOKIE_TO_INDEX(s, s->reply.cookie);
+ ind2 = COOKIE_TO_INDEX(s->reply.cookie);
if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].coroutine) {
nbd_channel_error(s, -EINVAL);
return -EINVAL;
@@ -519,7 +519,7 @@ nbd_co_send_request(BlockDriverState *bs, NBDRequest *request,
qemu_mutex_unlock(&s->requests_lock);
qemu_co_mutex_lock(&s->send_mutex);
- request->cookie = INDEX_TO_COOKIE(s, i);
+ request->cookie = INDEX_TO_COOKIE(i);
assert(s->ioc);
@@ -832,7 +832,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
int *request_ret, QEMUIOVector *qiov, void **payload, Error **errp)
{
int ret;
- int i = COOKIE_TO_INDEX(s, cookie);
+ int i = COOKIE_TO_INDEX(cookie);
void *local_payload = NULL;
NBDStructuredReplyChunk *chunk;
@@ -1038,7 +1038,7 @@ static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
break_loop:
qemu_mutex_lock(&s->requests_lock);
- s->requests[COOKIE_TO_INDEX(s, cookie)].coroutine = NULL;
+ s->requests[COOKIE_TO_INDEX(cookie)].coroutine = NULL;
s->in_flight--;
qemu_co_queue_next(&s->free_sema);
qemu_mutex_unlock(&s->requests_lock);
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 13/14] nbd/client: Add safety check on chunk payload length
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (11 preceding siblings ...)
2023-07-19 20:27 ` [PULL 12/14] nbd/client: Simplify cookie vs. index computation Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-19 20:27 ` [PULL 14/14] nbd: Use enum for various negotiation modes Eric Blake
2023-07-21 9:52 ` [PULL 00/14] NBD patches for 2023-07-19 Peter Maydell
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Vladimir Sementsov-Ogievskiy, open list:Network Block Dev...
Our existing use of structured replies either reads into a qiov capped
at 32M (NBD_CMD_READ) or caps allocation to 1000 bytes (see
NBD_MAX_MALLOC_PAYLOAD in block/nbd.c). But the existing length
checks are rather late; if we encounter a buggy (or malicious) server
that sends a super-large payload length, we should drop the connection
right then rather than assuming the layer on top will be careful.
This becomes more important when we permit 64-bit lengths which are
even more likely to have the potential for attempted denial of service
abuse.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230608135653.2918540-8-eblake@redhat.com>
---
nbd/client.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/nbd/client.c b/nbd/client.c
index ea3590ca3d0..1b5569556fe 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1413,6 +1413,18 @@ static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
chunk->cookie = be64_to_cpu(chunk->cookie);
chunk->length = be32_to_cpu(chunk->length);
+ /*
+ * Because we use BLOCK_STATUS with REQ_ONE, and cap READ requests
+ * at 32M, no valid server should send us payload larger than
+ * this. Even if we stopped using REQ_ONE, sane servers will cap
+ * the number of extents they return for block status.
+ */
+ if (chunk->length > NBD_MAX_BUFFER_SIZE + sizeof(NBDStructuredReadData)) {
+ error_setg(errp, "server chunk %" PRIu32 " (%s) payload is too long",
+ chunk->type, nbd_rep_lookup(chunk->type));
+ return -EINVAL;
+ }
+
return 0;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PULL 14/14] nbd: Use enum for various negotiation modes
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (12 preceding siblings ...)
2023-07-19 20:27 ` [PULL 13/14] nbd/client: Add safety check on chunk payload length Eric Blake
@ 2023-07-19 20:27 ` Eric Blake
2023-07-21 9:52 ` [PULL 00/14] NBD patches for 2023-07-19 Peter Maydell
14 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2023-07-19 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Kevin Wolf, Hanna Reitz,
open list:Network Block Dev...
Deciphering the hard-coded list of integer return values from
nbd_start_negotiate() will only get more confusing when adding support
for 64-bit extended headers. Better is to name things in an enum.
Although the function in question is private to client.c, putting the
enum in a public header and including an enum-to-string conversion
will allow its use in more places in upcoming patches.
The enum is intentionally laid out so that operators like <= can be
used to group multiple modes with similar characteristics, and where
the least powerful mode has value 0, even though this patch does not
exploit that. No semantic change intended.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230608135653.2918540-9-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
include/block/nbd.h | 11 +++++++++++
nbd/client.c | 46 ++++++++++++++++++++++++---------------------
nbd/common.c | 17 +++++++++++++++++
3 files changed, 53 insertions(+), 21 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index fb935d56e57..4428bcffbb9 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -53,6 +53,16 @@ typedef struct NBDOptionReplyMetaContext {
/* metadata context name follows */
} QEMU_PACKED NBDOptionReplyMetaContext;
+/* Track results of negotiation */
+typedef enum NBDMode {
+ /* Keep this list in a continuum of increasing features. */
+ NBD_MODE_OLDSTYLE, /* server lacks newstyle negotiation */
+ NBD_MODE_EXPORT_NAME, /* newstyle but only OPT_EXPORT_NAME safe */
+ NBD_MODE_SIMPLE, /* newstyle but only simple replies */
+ NBD_MODE_STRUCTURED, /* newstyle, structured replies enabled */
+ /* TODO add NBD_MODE_EXTENDED */
+} NBDMode;
+
/* Transmission phase structs
*
* Note: these are _NOT_ the same as the network representation of an NBD
@@ -405,6 +415,7 @@ const char *nbd_rep_lookup(uint32_t rep);
const char *nbd_info_lookup(uint16_t info);
const char *nbd_cmd_lookup(uint16_t info);
const char *nbd_err_lookup(int err);
+const char *nbd_mode_lookup(NBDMode mode);
/* nbd/client-connection.c */
void nbd_client_connection_enable_retry(NBDClientConnection *conn);
diff --git a/nbd/client.c b/nbd/client.c
index 1b5569556fe..479208d5d9d 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -875,10 +875,7 @@ static int nbd_list_meta_contexts(QIOChannel *ioc,
* Start the handshake to the server. After a positive return, the server
* is ready to accept additional NBD_OPT requests.
* Returns: negative errno: failure talking to server
- * 0: server is oldstyle, must call nbd_negotiate_finish_oldstyle
- * 1: server is newstyle, but can only accept EXPORT_NAME
- * 2: server is newstyle, but lacks structured replies
- * 3: server is newstyle and set up for structured replies
+ * non-negative: enum NBDMode describing server abilities
*/
static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
@@ -969,16 +966,16 @@ static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc,
return -EINVAL;
}
}
- return 2 + result;
+ return result ? NBD_MODE_STRUCTURED : NBD_MODE_SIMPLE;
} else {
- return 1;
+ return NBD_MODE_EXPORT_NAME;
}
} else if (magic == NBD_CLIENT_MAGIC) {
if (tlscreds) {
error_setg(errp, "Server does not support STARTTLS");
return -EINVAL;
}
- return 0;
+ return NBD_MODE_OLDSTYLE;
} else {
error_setg(errp, "Bad server magic received: 0x%" PRIx64, magic);
return -EINVAL;
@@ -1032,6 +1029,9 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
result = nbd_start_negotiate(aio_context, ioc, tlscreds, hostname, outioc,
info->structured_reply, &zeroes, errp);
+ if (result < 0) {
+ return result;
+ }
info->structured_reply = false;
info->base_allocation = false;
@@ -1039,8 +1039,8 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
ioc = *outioc;
}
- switch (result) {
- case 3: /* newstyle, with structured replies */
+ switch ((NBDMode)result) {
+ case NBD_MODE_STRUCTURED:
info->structured_reply = true;
if (base_allocation) {
result = nbd_negotiate_simple_meta_context(ioc, info, errp);
@@ -1050,7 +1050,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
info->base_allocation = result == 1;
}
/* fall through */
- case 2: /* newstyle, try OPT_GO */
+ case NBD_MODE_SIMPLE:
/* Try NBD_OPT_GO first - if it works, we are done (it
* also gives us a good message if the server requires
* TLS). If it is not available, fall back to
@@ -1073,7 +1073,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
return -EINVAL;
}
/* fall through */
- case 1: /* newstyle, but limited to EXPORT_NAME */
+ case NBD_MODE_EXPORT_NAME:
/* write the export name request */
if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, info->name,
errp) < 0) {
@@ -1089,7 +1089,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
return -EINVAL;
}
break;
- case 0: /* oldstyle, parse length and flags */
+ case NBD_MODE_OLDSTYLE:
if (*info->name) {
error_setg(errp, "Server does not support non-empty export names");
return -EINVAL;
@@ -1099,7 +1099,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
}
break;
default:
- return result;
+ g_assert_not_reached();
}
trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
@@ -1155,10 +1155,13 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
if (tlscreds && sioc) {
ioc = sioc;
}
+ if (result < 0) {
+ goto out;
+ }
- switch (result) {
- case 2:
- case 3:
+ switch ((NBDMode)result) {
+ case NBD_MODE_SIMPLE:
+ case NBD_MODE_STRUCTURED:
/* newstyle - use NBD_OPT_LIST to populate array, then try
* NBD_OPT_INFO on each array member. If structured replies
* are enabled, also try NBD_OPT_LIST_META_CONTEXT. */
@@ -1179,7 +1182,7 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
memset(&array[count - 1], 0, sizeof(*array));
array[count - 1].name = name;
array[count - 1].description = desc;
- array[count - 1].structured_reply = result == 3;
+ array[count - 1].structured_reply = result == NBD_MODE_STRUCTURED;
}
for (i = 0; i < count; i++) {
@@ -1195,7 +1198,7 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
break;
}
- if (result == 3 &&
+ if (result == NBD_MODE_STRUCTURED &&
nbd_list_meta_contexts(ioc, &array[i], errp) < 0) {
goto out;
}
@@ -1204,11 +1207,12 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
/* Send NBD_OPT_ABORT as a courtesy before hanging up */
nbd_send_opt_abort(ioc);
break;
- case 1: /* newstyle, but limited to EXPORT_NAME */
+ case NBD_MODE_EXPORT_NAME:
error_setg(errp, "Server does not support export lists");
/* We can't even send NBD_OPT_ABORT, so merely hang up */
goto out;
- case 0: /* oldstyle, parse length and flags */
+ case NBD_MODE_OLDSTYLE:
+ /* Lone export name is implied, but we can parse length and flags */
array = g_new0(NBDExportInfo, 1);
array->name = g_strdup("");
count = 1;
@@ -1226,7 +1230,7 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
}
break;
default:
- goto out;
+ g_assert_not_reached();
}
*info = array;
diff --git a/nbd/common.c b/nbd/common.c
index ddfe7d11837..989fbe54a19 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -248,3 +248,20 @@ int nbd_errno_to_system_errno(int err)
}
return ret;
}
+
+
+const char *nbd_mode_lookup(NBDMode mode)
+{
+ switch (mode) {
+ case NBD_MODE_OLDSTYLE:
+ return "oldstyle";
+ case NBD_MODE_EXPORT_NAME:
+ return "export name only";
+ case NBD_MODE_SIMPLE:
+ return "simple headers";
+ case NBD_MODE_STRUCTURED:
+ return "structured replies";
+ default:
+ return "<unknown>";
+ }
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PULL 00/14] NBD patches for 2023-07-19
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
` (13 preceding siblings ...)
2023-07-19 20:27 ` [PULL 14/14] nbd: Use enum for various negotiation modes Eric Blake
@ 2023-07-21 9:52 ` Peter Maydell
14 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2023-07-21 9:52 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
On Wed, 19 Jul 2023 at 21:39, Eric Blake <eblake@redhat.com> wrote:
>
> The following changes since commit 2c27fdc7a626408ee2cf30d791aa0b63027c7404:
>
> Update version for v8.1.0-rc0 release (2023-07-19 20:31:43 +0100)
>
> are available in the Git repository at:
>
> https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2023-07-19
>
> for you to fetch changes up to bfe04d0a7d5e8a4f4c9014ee7622af2056685974:
>
> nbd: Use enum for various negotiation modes (2023-07-19 15:26:13 -0500)
>
> ----------------------------------------------------------------
> NBD patches through 2023-07-19
>
> - Denis V. Lunev: fix hang with 'ssh ... "qemu-nbd -c"'
> - Eric Blake: preliminary work towards NBD 64-bit extensions
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/8.1
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-07-21 9:52 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
2023-07-19 20:27 ` [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char* Eric Blake
2023-07-19 20:27 ` [PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh Eric Blake
2023-07-19 20:27 ` [PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed Eric Blake
2023-07-19 20:27 ` [PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon() Eric Blake
2023-07-19 20:27 ` [PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process Eric Blake
2023-07-19 20:27 ` [PULL 06/14] qemu-nbd: make verbose bool and local variable in main() Eric Blake
2023-07-19 20:27 ` [PULL 07/14] nbd/client: Use smarter assert Eric Blake
2023-07-19 20:27 ` [PULL 08/14] nbd: Consistent typedef usage in header Eric Blake
2023-07-19 20:27 ` [PULL 09/14] nbd/server: Prepare for alternate-size headers Eric Blake
2023-07-19 20:27 ` [PULL 10/14] nbd/server: Refactor to pass full request around Eric Blake
2023-07-19 20:27 ` [PULL 11/14] nbd: s/handle/cookie/ to match NBD spec Eric Blake
2023-07-19 20:27 ` [PULL 12/14] nbd/client: Simplify cookie vs. index computation Eric Blake
2023-07-19 20:27 ` [PULL 13/14] nbd/client: Add safety check on chunk payload length Eric Blake
2023-07-19 20:27 ` [PULL 14/14] nbd: Use enum for various negotiation modes Eric Blake
2023-07-21 9:52 ` [PULL 00/14] NBD patches for 2023-07-19 Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).