search for: nbd_rep_ack

....or.option_reply.reply); len = be32toh (h->sbuf.or.option_reply.replylen); - if (magic != NBD_REP_MAGIC || option != NBD_OPT_GO) { - SET_NEXT_STATE (%.DEAD); - set_error (0, "handshake: invalid option reply magic or option"); - return -1; - } + switch (reply) { case NBD_REP_ACK: - if (len != 0) { - SET_NEXT_STATE (%.DEAD); - set_error (0, "handshake: invalid option reply length"); - return -1; - } SET_NEXT_STATE (%.READY); return 0; case NBD_REP_INFO: - if (len > maxpayload /* see RECV_NEWSTYLE_OPT_GO_REPLY */) { + if (...

...,6 +105,8 @@ extern const char *name_of_nbd_opt (int); #define NBD_OPT_INFO 6 #define NBD_OPT_GO 7 #define NBD_OPT_STRUCTURED_REPLY 8 +#define NBD_OPT_LIST_META_CONTEXT 9 +#define NBD_OPT_SET_META_CONTEXT 10 extern const char *name_of_nbd_rep (int); #define NBD_REP_ACK 1 diff --git a/server/connections.c b/server/connections.c index aeb27f8..7e32f00 100644 --- a/server/connections.c +++ b/server/connections.c @@ -926,6 +926,90 @@ _negotiate_handshake_newstyle_options (struct connection *conn) conn->structured_replies = true; break; +...

...se affect attempts to connect to older qemu-nbd versions, some of them were triggered by manual edits to qemu-nbd source code to provoke various other compliant (if uncommon) server behaviors. Eric Blake (4): starttls: Skip error payload if falling back to unencrypted states: Reject payload to NBD_REP_ACK meta-context: Skip error payload if server lacks meta_context states: Add NBD_OPT_EXPORT_NAME handling generator/Makefile.am | 1 + generator/generator | 53 ++++++++++++++ generator/states-newstyle-opt-export-name.c | 73 ++++++++++++++++++...

I'm still playing with ideas on how to split rstate from wstate (so that we can send a request without waiting for POLLIN to complete a pending reply), but this is some preliminary refactoring I found useful. I also fixed a couple of bugs while in the area (already pushed). There's a question of whether we want nbd_handle to be nearly 5k, or if we should instead keep it small and add one

...-git a/lib/nbd-protocol.h b/lib/nbd-protocol.h index 405af3e..3e3fb4e 100644 --- a/lib/nbd-protocol.h +++ b/lib/nbd-protocol.h @@ -123,15 +123,19 @@ struct nbd_fixed_new_option_reply { #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -#define NBD_REP_ACK 1 -#define NBD_REP_SERVER 2 -#define NBD_REP_INFO 3 -#define NBD_REP_META_CONTEXT 4 -#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1) -#define NBD_REP_ERR_POLICY NBD_REP_ERR (2) -#define NBD_REP_ERR_INVALID NBD_REP_ERR (3) -#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) -#define...

...gt;reply = be32toh (reply->reply); + reply->replylen = be32toh (reply->replylen); + if (reply->magic != NBD_REP_MAGIC || reply->option != option) { + nbdkit_error ("unexpected option reply"); + return -1; + } + if (reply->replylen) { + if (reply->reply == NBD_REP_ACK) { + nbdkit_error ("NBD_REP_ACK should not have replylen %" PRId32, + reply->replylen); + return -1; + } + if (reply->replylen > 16 * 1024 * 1024) { + nbdkit_error ("option reply length is suspiciously large: %" PRId32, +...

...tion(-) diff --git a/src/connections.c b/src/connections.c index e1ffeff..1b40e46 100644 --- a/src/connections.c +++ b/src/connections.c @@ -705,7 +705,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn) case NBD_OPT_ABORT: if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) return -1; - nbdkit_error ("client sent NBD_OPT_ABORT to abort the connection"); + debug ("client sent NBD_OPT_ABORT to abort the connection"); return -1; case NBD_OPT_LIST: -- 2.17.2

...? offsetof (struct new_handshake_finish, zeroes) - : sizeof handshake_finish) == -1) { - nbdkit_error ("write: %m"); - return -1; - } - break; - - case NBD_OPT_ABORT: - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) - return -1; - debug ("client sent %s to abort the connection", - name_of_nbd_opt (option)); - return -1; - - case NBD_OPT_LIST: - if (optlen != 0) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) - == -1...

...rt export = nbdkit_get_export (exps, i); size_t name_len = strlen (export.name); size_t desc_len = export.description ? strlen (export.description) : 0; @@ -127,7 +132,11 @@ send_newstyle_option_reply_exportnames (uint32_t option) } } - return send_newstyle_option_reply (option, NBD_REP_ACK); + r = send_newstyle_option_reply (option, NBD_REP_ACK); + /* Allow additional per-export NBD_OPT_INFO and friends. */ + if (r == 0) + *nr_options += MAX_NR_OPTIONS * list_len; + return r; } static int @@ -326,6 +335,7 @@ negotiate_handshake_newstyle_options (void) GET_CONN; struct...

There's no substantial difference over v1, I simply fixed a few whitespace issues, moved one struct around and tidied up the comments. Rich.

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

...ation.) I am not certain if this is correct or not, but I believe it is: the NBD protocol says the only way to exit option mode and enter transmission mode is: "Transmission mode can be entered (by the client sending NBD_OPT_EXPORT_NAME or by the server responding to an NBD_OPT_GO with NBD_REP_ACK)." (The other two ways mentioned in that section are about terminating the connection completely). This also implies that the number of options must never be zero, which the spec does say in passing: "At this point, we move on to option haggling, during which point the client can s...

Updated based on other changes that have happened in the meantime: - rely more on cleanup.h (throughout) - split structured read for easier review (patch 2 and 3 were combined in v1) - rely on nbdkit not leaking a server's partial answer (patch 3) - add tests (patch 5) - other bug fixes I found while testing it - drop EOVERFLOW patch for now; it will be separate once upstream NBD protocol

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

...? offsetof (struct nbd_export_name_option_reply, zeroes) : sizeof handshake_finish, 0) == -1) { @@ -346,7 +346,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) break; case NBD_OPT_ABORT: - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; debug ("client sent %s to abort the connection", name_of_nbd_opt (option)); @@ -354,10 +354,10 @@ negotiate_handshake_newstyle_options (struct connection *conn) case...

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

...uot; + "ignoring NBD_INFO_* request %u", (unsigned) info); + break; + } + } + } + + /* Unlike NBD_OPT_EXPORT_NAME, NBD_OPT_GO sends back an ACK + * or ERROR packet. + */ + if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + return -1; + + break; + default: /* Unknown option. */ if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) @@ -712,10 +860,10 @@ _negotiate_handshake_newstyle_options (struct connection *conn) } /* Note, since it's not ver...

...tocol.h index 792a905..088dcab 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -98,6 +98,7 @@ struct fixed_new_option_reply { #define NBD_OPT_ABORT 2 #define NBD_OPT_LIST 3 #define NBD_OPT_STARTTLS 5 +#define NBD_OPT_INFO 6 #define NBD_OPT_GO 7 #define NBD_REP_ACK 1 diff --git a/src/connections.c b/src/connections.c index 1b40e46..410a893 100644 --- a/src/connections.c +++ b/src/connections.c @@ -636,6 +636,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn) uint32_t optlen; char data[MAX_OPTION_LENGTH+1]; struct new_handsh...

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

...t;nbd-protocol.h"\ +#include "nbd-protocol.h" +EOF -# Match the precise sections of the source file. -/^extern const char \*name_of_/,/^$/ { +declare -A functions=( + [global_flag]=NBD_FLAG_FIXED_NEWSTYLE + [flag]=NBD_FLAG_HAS_FLAGS + [opt]=NBD_OPT_EXPORT_NAME + [rep]=NBD_REP_ACK + [info]=NBD_INFO_EXPORT + [reply]=NBD_REPLY_FLAG_DONE + [reply_type]=NBD_REPLY_TYPE_NONE + [cmd]=NBD_CMD_READ + [cmd_flag]=NBD_CMD_FLAG_FUA + [error]=NBD_SUCCESS +) - # Convert extern function prototype into a definition. - s/extern \(const char \*name_of_.*\) (int);/\1 (int...