search for: handshake_finish

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

...x%x", conn->eflags); + return 0; +} + static int _negotiate_handshake_newstyle_options (struct connection *conn) { @@ -581,6 +632,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn) uint32_t option; uint32_t optlen; char data[MAX_OPTION_LENGTH+1]; + struct new_handshake_finish handshake_finish; for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) { if (conn->recv (conn, &new_option, sizeof new_option) == -1) { @@ -625,7 +677,8 @@ _negotiate_handshake_newstyle_options (struct connection *conn) } /* Apart from printing it, ignor...

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.

...); return -1; } - conn->exportsize = (uint64_t) r; + *exportsize = r; if (protocol_compute_eflags (conn, &conn->eflags) < 0) return -1; @@ -233,6 +233,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) char data[MAX_OPTION_LENGTH+1]; struct new_handshake_finish handshake_finish; const char *optname; + uint64_t exportsize; for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) { if (conn_recv_full (conn, &new_option, sizeof new_option, @@ -281,11 +282,11 @@ negotiate_handshake_newstyle_options (struct connection *conn)...

.../* length of block */ uint32_t status_flags; /* block type (hole etc) */ } __attribute__((packed)); @@ -144,14 +157,14 @@ struct block_descriptor { /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO instead of this. */ -struct new_handshake_finish { +struct nbd_new_handshake_finish { uint64_t exportsize; uint16_t eflags; /* per-export flags */ char zeroes[124]; /* must be sent as zero bytes */ } __attribute__((packed)); /* Request (client -> server). */ -struct request { +struct nbd_request { uint32_t...

Experimental and only very lightly tested so far. Rich.

The upstream protocol recently promoted NBD_CMD_WRITE_ZEROES from experimental to a documented extension. Exposing support for this allows plugin writers to create sparse files when driven by a client that knows how to use the extension; meanwhile, even if a plugin does not support this extension, the server benefits from less network traffic from the client. Eric Blake (5): protocol: Support

...export 0x%x", conn->eflags); - return 0; -} - -static int -_negotiate_handshake_newstyle_options (struct connection *conn) -{ - struct new_option new_option; - size_t nr_options; - uint64_t version; - uint32_t option; - uint32_t optlen; - char data[MAX_OPTION_LENGTH+1]; - struct new_handshake_finish handshake_finish; - const char *optname; - - for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) { - if (conn_recv_full (conn, &new_option, sizeof new_option, - "reading option: conn->recv: %m") == -1) - return -1; - - version = be...

...te_handshake_newstyle_options (struct connection *conn) > uint64_t version; > uint32_t option; > uint32_t optlen; > - char data[MAX_OPTION_LENGTH+1]; > + CLEANUP_FREE char *data = NULL; Even though you have the CLEANUP here ... > struct nbd_export_name_option_reply handshake_finish; > const char *optname; > uint64_t exportsize; > @@ -281,6 +281,11 @@ negotiate_handshake_newstyle_options (struct connection *conn) > nbdkit_error ("client option data too long (%" PRIu32 ")", optlen); > return -1; > } > + data =...

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.

...ewstyle_options (struct connection *conn, uint64_t *exportsize) @@ -322,7 +322,9 @@ negotiate_handshake_newstyle_options (struct connection *conn) if (check_export_name (conn, option, data, optlen, optlen, true) == -1) return -1; - /* We have to finish the handshake by sending handshake_finish. */ + /* We have to finish the handshake by sending handshake_finish. + * On failure, we have to disconnect. + */ if (finish_newstyle_options (conn, &exportsize) == -1) return -1; @@ -460,16 +462,30 @@ negotiate_handshake_newstyle_options (struct connection *co...

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

...t didn't finish with NBD_OPT_GO " > + "or NBD_OPT_EXPORT_NAME"); > + return -1; > + } > + > return 0; Current behaviour in nbdkit is that if the last requested option is a recognized option, but is not NBD_OPT_EXPORT_NAME then nbdkit will send the handshake_finish struct and continue to transport mode. However I don't think nbdkit is currently behaving correctly. In any case the proposed behaviour (above) is that in this case we will close the connection abruptly and record an error in the log. (There's no way to return an error back to the client...

...EP_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_handshake_finish handshake_finish; + const char *optname; for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) { if (conn->recv (conn, &new_option, sizeof new_option) == -1) { @@ -774,14 +775,16 @@ _negotiate_handshake_newstyle_options (struct connection *conn) } break;...

We should have only one NBD protocol file. Let's make nbdkit's version the canonical one, and use it in libnbd. Rich.

...return -1; >> + } As I pointed out in the other mail, this else clause is unreachable. >> + >> return 0; > > Current behaviour in nbdkit is that if the last requested option is a > recognized option, but is not NBD_OPT_EXPORT_NAME then nbdkit will > send the handshake_finish struct and continue to transport mode. In the current code, there is no such thing as a last option except for EXPORT_NAME - you are in a for loop that is only broken when you exceed too many options (in which case you kill the connection) or when you encountered EXPORT_NAME. > > Howeve...

...uint32_t option; >> uint32_t optlen; >> - char data[MAX_OPTION_LENGTH+1]; >> + CLEANUP_FREE char *data = NULL; > > Even though you have the CLEANUP here ... Scope is too wide. I need to sink the declaration... > >> struct nbd_export_name_option_reply handshake_finish; >> const char *optname; >> uint64_t exportsize; >> @@ -281,6 +281,11 @@ negotiate_handshake_newstyle_options (struct connection *conn) >> nbdkit_error ("client option data too long (%" PRIu32 ")", optlen); >> return -1; >...

.../* Receive newstyle options. */ static int @@ -255,7 +255,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) uint64_t version; uint32_t option; uint32_t optlen; - char data[MAX_OPTION_LENGTH+1]; + CLEANUP_FREE char *data = NULL; struct nbd_export_name_option_reply handshake_finish; const char *optname; uint64_t exportsize; @@ -281,6 +281,11 @@ negotiate_handshake_newstyle_options (struct connection *conn) nbdkit_error ("client option data too long (%" PRIu32 ")", optlen); return -1; } + data = malloc (optlen + 1); /* Allowing a...

This time around, the numbers are indeed looking better than in v1; and I like the interface better. Eric Blake (2): server: Prefer send() over write() server: Group related transmission send()s server/internal.h | 7 +++- server/connections.c | 51 +++++++++++++++++++++++++--- server/crypto.c | 11 ++++--

...e (conn->exportname); + conn->exportname = malloc (optlen+1); + if (conn->exportname == NULL) { + nbdkit_error ("malloc: %m"); + return -1; + } + memcpy (conn->exportname, data, optlen+1); /* We have to finish the handshake by sending handshake_finish. */ if (finish_newstyle_options (conn, &exportsize) == -1) @@ -413,19 +419,19 @@ negotiate_handshake_newstyle_options (struct connection *conn) continue; } - /* As with NBD_OPT_EXPORT_NAME we print the export name and then - * ignore it. + /* A...