5 p.m.
Time to make option negotiation actually useful, by now allowing the
user to request an export name before (re-)trying NBD_OPT_GO. This
also means that we have to reset state before each attempt at
NBD_OPT_GO (any meta contexts from the previous try must not be
inherited into the next try).
For now, I chose not to have a completion closure on nbd_aio_opt_go;
you can detect success or failure based on the resulting state, and
being able to set *err would be awkward (you can't magically get back
to negotiation state if the server has moved on to data phase). But
we have until 1.4 to decide if this decision needs to be revisited.
---
lib/internal.h | 1 +
generator/API.ml | 55 ++++++++++++++++---
generator/states-newstyle-opt-go.c | 6 +-
.../states-newstyle-opt-set-meta-context.c | 1 +
generator/states-newstyle.c | 6 ++
lib/flags.c | 28 +++++++++-
lib/handle.c | 7 +--
lib/opt.c | 26 +++++++++
tests/newstyle-limited.c | 9 ++-
9 files changed, 122 insertions(+), 17 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 03baacd..3672538 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -405,6 +405,7 @@ extern void nbd_internal_set_last_error (int errnum, char *error);
} while (0)
/* flags.c */
+extern void nbd_internal_reset_size_and_flags (struct nbd_handle *h);
extern int nbd_internal_set_size_and_flags (struct nbd_handle *h,
uint64_t exportsize,
uint16_t eflags);
diff --git a/generator/API.ml b/generator/API.ml
index cbc1c33..21485ea 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -285,9 +285,15 @@ export names. The NBD protocol limits export names to
4096 bytes, but servers may not support the full length.
The encoding of export names is always UTF-8.
+When option mode is not in use, the export name must be set
+before beginning a connection. However, when L<nbd_set_opt_mode(3)>
+has enabled option mode, it is possible to change the export
+name prior to L<nbd_opt_go(3)>.
+
This call may be skipped if using L<nbd_connect_uri(3)> to connect
to a URI that includes an export name.";
- see_also = [Link "get_export_name"; Link "connect_uri"];
+ see_also = [Link "get_export_name"; Link "connect_uri";
+ Link "set_opt_mode"; Link "opt_go"];
};
"get_export_name", {
@@ -697,13 +703,13 @@ When option mode is enabled, you have fine-grained control over
which
options are negotiated, compared to the default of the server
negotiating everything on your behalf using settings made before
starting the connection. To leave the mode and proceed on to the
-ready state, you must use nbd_opt_go successfully; a failed
-C<nbd_opt_go> returns to the negotiating state to allow a change of
+ready state, you must use L<nbd_opt_go(3)> successfully; a failed
+L<nbd_opt_go(3)> returns to the negotiating state to allow a change of
export name before trying again. You may also use L<nbd_opt_abort(3)>
to end the connection without finishing negotiation.";
example = Some "examples/list-exports.c";
see_also = [Link "get_opt_mode"; Link "aio_is_negotiating";
- Link "opt_abort"];
+ Link "opt_abort"; Link "opt_go"];
};
"get_opt_mode", {
@@ -716,6 +722,24 @@ Return true if option negotiation mode was enabled on this
handle.";
see_also = [Link "set_opt_mode"];
};
+ "opt_go", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and move on to using an export";
+ longdesc = "\
+Request that the server finish negotiation and move on to serving the
+export previously specified by L<nbd_set_export_name(3)>. This can only
+be used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+If this fails, the server may still be in negotiation, where it is
+possible to attempt another option such as a different export name;
+although older servers will instead have killed the connection.";
+ example = Some "examples/list-exports.c";
+ see_also = [Link "set_opt_mode"; Link "aio_opt_go"; Link
"opt_abort";
+ Link "set_export_name"];
+ };
+
"opt_abort", {
default_call with
args = []; ret = RErr;
@@ -726,7 +750,7 @@ Request that the server finish negotiation, gracefully if possible,
then
close the connection. This can only be used if L<nbd_set_opt_mode(3)>
enabled option mode.";
example = Some "examples/list-exports.c";
- see_also = [Link "set_opt_mode"; Link "aio_opt_abort"];
+ see_also = [Link "set_opt_mode"; Link "aio_opt_abort"; Link
"opt_go"];
};
"set_list_exports", {
@@ -938,8 +962,8 @@ L<nbd_set_export_name(3)> and L<nbd_set_tls(3)> and other
calls as needed, followed by L<nbd_connect_tcp(3)> or
L<nbd_connect_unix(3)>. However, it is possible to override the
export name portion of a URI by using L<nbd_set_opt_mode(3)> to
-enable option mode, then using L<nbd_set_export_name(3)> as part
-of later negotiation.
+enable option mode, then using L<nbd_set_export_name(3)> and
+L<nbd_opt_go(3)> as part of subsequent negotiation.
This call returns when the connection has been made.
@@ -1897,6 +1921,21 @@ and completed the NBD handshake by calling
L<nbd_aio_is_ready(3)>,
on the connection.";
};
+ "aio_opt_go", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and move on to using an export";
+ longdesc = "\
+Request that the server finish negotiation and move on to serving the
+export previously specified by L<nbd_set_export_name(3)>. This can only
+be used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+To determine when the request completes, wait for
+L<nbd_aio_is_connecting(3)> to return false.";
+ see_also = [Link "set_opt_mode"; Link "opt_go"];
+ };
+
"aio_opt_abort", {
default_call with
args = []; ret = RErr;
@@ -2533,7 +2572,9 @@ let first_version = [
"set_opt_mode", (1, 4);
"get_opt_mode", (1, 4);
"aio_is_negotiating", (1, 4);
+ "opt_go", (1, 4);
"opt_abort", (1, 4);
+ "aio_opt_go", (1, 4);
"aio_opt_abort", (1, 4);
(* These calls are proposed for a future version of libnbd, but
diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
index 1a59ae7..0568695 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -240,7 +240,11 @@ STATE_MACHINE {
set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32,
reply);
}
- SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
+ nbd_internal_reset_size_and_flags (h);
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
break;
case NBD_REP_ACK:
SET_NEXT_STATE (%^FINISHED);
diff --git a/generator/states-newstyle-opt-set-meta-context.c
b/generator/states-newstyle-opt-set-meta-context.c
index 77fd022..2dc8db8 100644
--- a/generator/states-newstyle-opt-set-meta-context.c
+++ b/generator/states-newstyle-opt-set-meta-context.c
@@ -28,6 +28,7 @@ STATE_MACHINE {
* contexts.
*/
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
+ nbd_internal_reset_size_and_flags (h);
if (!h->structured_replies ||
h->request_meta_contexts == NULL ||
nbd_internal_string_list_length (h->request_meta_contexts) == 0) {
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 6613f9f..d771097 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -118,6 +118,12 @@ STATE_MACHINE {
* state has informed us what we still need to do.
*/
switch (h->current_opt) {
+ case NBD_OPT_GO:
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
+ SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
+ else
+ SET_NEXT_STATE (%OPT_SET_META_CONTEXT.START);
+ return 0;
case NBD_OPT_ABORT:
if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
SET_NEXT_STATE (%.DEAD);
diff --git a/lib/flags.c b/lib/flags.c
index 223b77d..09eac72 100644
--- a/lib/flags.c
+++ b/lib/flags.c
@@ -27,6 +27,31 @@
#include "internal.h"
+/* Reset connection data. Called after swapping export name, after
+ * failed OPT_GO/OPT_INFO, or when starting a fresh OPT_SET_META_CONTEXT.
+ */
+void
+nbd_internal_reset_size_and_flags (struct nbd_handle *h)
+{
+ struct meta_context *m, *m_next;
+
+ h->exportsize = 0;
+ h->eflags = 0;
+ for (m = h->meta_contexts; m != NULL; m = m_next) {
+ m_next = m->next;
+ free (m->name);
+ free (m);
+ }
+ h->meta_contexts = NULL;
+ h->block_minimum = 0;
+ h->block_preferred = 0;
+ h->block_maximum = 0;
+ free (h->canonical_name);
+ h->canonical_name = NULL;
+ free (h->description);
+ h->description = NULL;
+}
+
/* Set the export size and eflags on the handle, validating them.
* This is called from the state machine when either the newstyle or
* oldstyle negotiation reaches the point that these are available.
@@ -194,7 +219,8 @@ nbd_unlocked_get_size (struct nbd_handle *h)
return h->exportsize;
}
-int64_t nbd_unlocked_get_block_size (struct nbd_handle *h, int type)
+int64_t
+nbd_unlocked_get_block_size (struct nbd_handle *h, int type)
{
switch (type) {
case LIBNBD_SIZE_MINIMUM:
diff --git a/lib/handle.c b/lib/handle.c
index 28c30dd..a51b923 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -112,7 +112,6 @@ nbd_create (void)
void
nbd_close (struct nbd_handle *h)
{
- struct meta_context *m, *m_next;
size_t i;
nbd_internal_set_error_context ("nbd_close");
@@ -126,11 +125,7 @@ nbd_close (struct nbd_handle *h)
nbd_unlocked_clear_debug_callback (h);
free (h->bs_entries);
- for (m = h->meta_contexts; m != NULL; m = m_next) {
- m_next = m->next;
- free (m->name);
- free (m);
- }
+ nbd_internal_reset_size_and_flags (h);
for (i = 0; i < h->nr_exports; ++i) {
free (h->exports[i].name);
free (h->exports[i].description);
diff --git a/lib/opt.c b/lib/opt.c
index 6243553..6ad7f1a 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -51,6 +51,21 @@ wait_for_option (struct nbd_handle *h)
return 0;
}
+/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) and wait for the reply. */
+int
+nbd_unlocked_opt_go (struct nbd_handle *h)
+{
+ int r = nbd_unlocked_aio_opt_go (h);
+
+ if (r == -1)
+ return r;
+
+ r = wait_for_option (h);
+ if (r == 0 && nbd_internal_is_state_negotiating (get_next_state (h)))
+ return -1; /* NBD_OPT_GO failed, but can be attempted again */
+ return r;
+}
+
/* Issue NBD_OPT_ABORT and wait for the state change. */
int
nbd_unlocked_opt_abort (struct nbd_handle *h)
@@ -63,6 +78,17 @@ nbd_unlocked_opt_abort (struct nbd_handle *h)
return wait_for_option (h);
}
+/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */
+int
+nbd_unlocked_aio_opt_go (struct nbd_handle *h)
+{
+ h->current_opt = NBD_OPT_GO;
+
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
+
/* Issue NBD_OPT_ABORT without waiting. */
int
nbd_unlocked_aio_opt_abort (struct nbd_handle *h)
diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c
index cdda474..aa6f87b 100644
--- a/tests/newstyle-limited.c
+++ b/tests/newstyle-limited.c
@@ -152,11 +152,16 @@ main (int argc, char *argv[])
*/
nbd = nbd_create ();
if (nbd == NULL ||
- nbd_set_export_name (nbd, "b") == -1 ||
- nbd_connect_command (nbd, args) == 1) {
+ nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_connect_command (nbd, args) == -1 ||
+ nbd_set_export_name (nbd, "b") == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
+ if (nbd_opt_go (nbd) != -1) {
+ fprintf (stderr, "%s\n", "expected failure");
+ exit (EXIT_FAILURE);
+ }
if (!nbd_aio_is_dead (nbd)) {
fprintf (stderr, "unexpected state after failed export\n");
exit (EXIT_FAILURE);
--
2.28.0