search for: send_newstyle_option_reply

...uint32_t nr_queries; + uint32_t querylen; + const char *what; + + optname = name_of_nbd_opt (option); + if (conn_recv_full (conn, data, optlen, "read: %s: %m", optname) == -1) + return -1; + + if (!conn->structured_replies) { + if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + == -1) + return -1; + continue; + } + + /* Minimum length of the option payload is the 32 bit export + * name plus a zero length export name plus 32 bit number of + * queries followed by no queries....

...(exportsize); - handshake.gflags = htobe16 (gflags); - handshake.eflags = htobe16 (eflags); - - if (conn->send (conn, &handshake, sizeof handshake) == -1) { - nbdkit_error ("write: %m"); - return -1; - } - - return 0; -} - -/* Receive newstyle options. */ - -static int -send_newstyle_option_reply (struct connection *conn, - uint32_t option, uint32_t reply) -{ - struct fixed_new_option_reply fixed_new_option_reply; - - fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); - fixed_new_option_reply.option = htobe32 (option); - fixed_new_option_reply.reply = hto...

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.

...int *err) { diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 7179186f..aa817be7 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -49,9 +49,9 @@ /* Receive newstyle options. */ static int -send_newstyle_option_reply (struct connection *conn, - uint32_t option, uint32_t reply) +send_newstyle_option_reply (uint32_t option, uint32_t reply) { + struct connection *conn = GET_CONN; struct nbd_fixed_new_option_reply fixed_new_option_reply; fixed_new_option_reply.magic = htobe64 (N...

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.

...low before giving up. */ +/* Initial bound of client options we allow before giving up. + * However, a client that issues NBD_OPT_LIST is permitted to follow + * up with another round of options per export listed. + */ #define MAX_NR_OPTIONS 32 /* Receive newstyle options. */ @@ -78,12 +81,13 @@ send_newstyle_option_reply (uint32_t option, uint32_t reply) /* Reply to NBD_OPT_LIST with the plugin's list of export names. */ static int -send_newstyle_option_reply_exportnames (uint32_t option) +send_newstyle_option_reply_exportnames (uint32_t option, size_t *nr_options) { GET_CONN; struct nbd_fixed_new_op...

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

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.

...ad: %m"); return -1; } if (optlen < 6) { /* 32 bit export length + 16 bit nr info */ - debug ("newstyle negotiation: NBD_OPT_GO option length < 6"); + debug ("newstyle negotiation: %s option length < 6", optname); if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) == -1) @@ -800,7 +803,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn) memcpy (&exportnamelen, &data[0], 4); exportnamelen = be32toh (exportnamelen); if (exportnamelen > optlen-6 /* NB optlen...

...eblake@redhat.com> --- src/connections.c | 60 ++++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/src/connections.c b/src/connections.c index 58ed6b0..577f466 100644 --- a/src/connections.c +++ b/src/connections.c @@ -600,6 +600,31 @@ send_newstyle_option_reply_info_export (struct connection *conn, return 0; } +/* Sub-function during _negotiate_handshake_newstyle, to uniformly handle + * a client hanging up on a message boundary. + */ +static int __attribute__ ((format (printf, 4, 5))) +conn_recv_full (struct connection *conn, void *buf, size_t len,...

...ndex ba6e91d..4e9b191 100644 --- a/src/connections.c +++ b/src/connections.c @@ -75,7 +75,9 @@ struct connection { void **handles; size_t nr_handles; + uint32_t cflags; uint64_t exportsize; + uint16_t eflags; int readonly; int can_flush; int is_rotational; @@ -572,6 +574,55 @@ send_newstyle_option_reply_exportname (struct connection *conn, return 0; } +static int +send_newstyle_option_reply_info_export (struct connection *conn, + uint32_t option, uint32_t reply, + uint16_t info) +{ + struct fixed_new_option_reply...

...* or else we drop the support for that context. */ if (check_export_name (conn, option, &data[4], exportnamelen, - optlen - 6, true) == -1) - return -1; + optlen - 6, true) == -1) { + if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + == -1) + return -1; + continue; + } /* The spec is confusing, but it is required that we send back * NBD_INFO_EXPORT, even if the client did not request it! * qemu client in particular does n...

This is only lightly tested (against just qemu NBD client), and the code might be structured a little better as the _negotiate_handshake_newstyle_options function has now grown to be huge. Anyway works for me. Rich.

...t; --- src/connections.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/connections.c b/src/connections.c index 0c93f35..840e315 100644 --- a/src/connections.c +++ b/src/connections.c @@ -334,6 +334,10 @@ _negotiate_handshake_newstyle_options (struct connection *conn) if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) == -1) return -1; + if (xread (conn->sockin, data, optlen) == -1) { + nbdkit_error ("read: %m"); + return -1; + } continue; } @@ -351,6 +355,10 @@ _negotiate_handshake_newstyle_...

Experimental and only very lightly tested so far. Rich.

...+- 1 file changed, 1 insertion(+), 1 deletion(-) 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

...name_of_nbd_opt (option)); + r = backend->can_sr (backend, conn); + if (r == -1) + return -1; + if (!r) { + /* Must fail with ERR_UNSUP for qemu 4.2 to remain happy; + * but failing with ERR_POLICY would have been nicer. + */ + if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) + return -1; + debug ("newstyle negotiation: %s: filter disabled structured replies", + name_of_nbd_opt (option)); + break; + } + if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1)...

...an_multi_conn; bool using_tls; + bool structured_replies; int sockin, sockout; connection_recv_function recv; @@ -905,6 +906,26 @@ _negotiate_handshake_newstyle_options (struct connection *conn) break; + case NBD_OPT_STRUCTURED_REPLY: + if (optlen != 0) { + if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + == -1) + return -1; + if (conn_recv_full (conn, data, optlen, + "read: %s: %m", name_of_nbd_opt (option)) == -1) + return -1; + continue; + } + + debug ("newstyle neg...

Since the v1 series (0/4, at [1]), I've applied patches 1 and 2, rewritten patch 3 [Forbid NUL in export and context names] into patch 4 here, patch 4 there turned into patch 6 here, and everything else here is new. [1]https://www.redhat.com/archives/libguestfs/2019-September/msg00180.html I don't know if there is a handy reusable function for checking whether a string contains valid

...debug ("newstyle negotiation: %s: client requested structured replies", name_of_nbd_opt (option)); + if (no_sr) { + /* Must fail with ERR_UNSUP for qemu 4.2 to remain happy; + * but failing with ERR_POLICY would have been nicer. + */ + if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) + return -1; + debug ("newstyle negotiation: %s: structured replies are disabled", + name_of_nbd_opt (option)); + break; + } + if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1)...