5 p.m.
It is finally time to introduce our first negotiating option command.
With this change, we can now enter NEWSTYLE.START more than once; as
such, it needs to know whether it is the first entry (proceed with
gflags/cflags, TLS, and structured reply) or a later entry (all
nbd_opt_* will cause an IssueCommand event to kick the state machine
out of NEGOTIATING, at which point we want to jump to the sub-state
appropriate for that option). This is done by setting h->current_opt
prior to entering NEWSTYLE.
We also need a couple of new states to implement NBD_OPT_ABORT; this
was not deemed worth a separate sub-group and new
states-newstyle-opt-abort.c file, so I just inlined it in NEWSTYLE
instead. One nice benefit of these new states: when NBD_OPT_GO fails
and we are not in opt mode, or when we require TLS but the server does
not support it, we can now hang up gracefully instead of abruptly
disconnecting from a newstyle-fixed server. This is observable when
using qemu-nbd, in that we now silence the former noise from qemu:
$ truncate --size=512 /tmp/f
$ ./run nbdsh -c '
import errno
try:
h.connect_systemd_socket_activation(["qemu-nbd", "-f",
"raw", "-x", "b",
"/tmp/f"])
except nbd.Error as e:
assert e.errnum == errno.ENOENT
'
qemu-nbd: option negotiation failed: Failed to read opts magic: Unexpected end-of-file
before all bytes were read
The aio_opt_abort counterpart does not need a completion closure, for
the same reason that aio_disconnect does not have one - the whole
point of this command is a clean shutdown, so you aren't going to rely
on a server reply, and there are no parameters that need cleanup.
Yes, it's a bit awkward that with just this patch, enabling opt_mode
means you HAVE to kill the connection without a data phase; that will
be rectified in the next patch adding nbd_opt_go. But breaking the
patches up makes review easier.
---
lib/internal.h | 1 +
generator/API.ml | 35 ++++++-
generator/state_machine.ml | 26 +++++
generator/states-magic.c | 6 +-
generator/states-newstyle-opt-go.c | 2 +-
generator/states-newstyle-opt-starttls.c | 4 +-
.../states-newstyle-opt-structured-reply.c | 7 +-
generator/states-newstyle.c | 59 +++++++++++-
lib/opt.c | 34 +++++++
tests/Makefile.am | 7 ++
tests/errors.c | 11 ++-
tests/newstyle-limited.c | 26 +++++
tests/opt-abort.c | 95 +++++++++++++++++++
.gitignore | 1 +
14 files changed, 302 insertions(+), 12 deletions(-)
create mode 100644 tests/opt-abort.c
diff --git a/lib/internal.h b/lib/internal.h
index 5f495fb..03baacd 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -101,6 +101,7 @@ struct nbd_handle {
/* Option negotiation mode. */
bool opt_mode;
+ uint8_t current_opt;
/* List exports mode. */
bool list_exports;
diff --git a/generator/API.ml b/generator/API.ml
index 3dd94f6..cbc1c33 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -699,10 +699,11 @@ 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
-export name before trying again. You may also use nbd_opt_abort
+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"];
+ see_also = [Link "get_opt_mode"; Link "aio_is_negotiating";
+ Link "opt_abort"];
};
"get_opt_mode", {
@@ -715,6 +716,19 @@ Return true if option negotiation mode was enabled on this
handle.";
see_also = [Link "set_opt_mode"];
};
+ "opt_abort", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and close the connection";
+ longdesc = "\
+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"];
+ };
+
"set_list_exports", {
default_call with
args = [Bool "list"]; ret = RErr;
@@ -1883,6 +1897,21 @@ and completed the NBD handshake by calling
L<nbd_aio_is_ready(3)>,
on the connection.";
};
+ "aio_opt_abort", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and close the connection";
+ longdesc = "\
+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.
+
+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_abort"];
+ };
+
"aio_pread", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
@@ -2504,6 +2533,8 @@ let first_version = [
"set_opt_mode", (1, 4);
"get_opt_mode", (1, 4);
"aio_is_negotiating", (1, 4);
+ "opt_abort", (1, 4);
+ "aio_opt_abort", (1, 4);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/generator/state_machine.ml b/generator/state_machine.ml
index 8525520..2d28c2e 100644
--- a/generator/state_machine.ml
+++ b/generator/state_machine.ml
@@ -278,6 +278,8 @@ and newstyle_state_machine = [
(* Options. These state groups are always entered unconditionally,
* in this order. The START state in each group will check if the
* state needs to run and skip to the next state in the list if not.
+ * When opt_mode is set, control is returned to the user in state
+ * NEGOTIATING after OPT_STRUCTURED_REPLY or any failed OPT_GO.
*)
Group ("OPT_STARTTLS", newstyle_opt_starttls_state_machine);
Group ("OPT_LIST", newstyle_opt_list_state_machine);
@@ -286,6 +288,30 @@ and newstyle_state_machine = [
Group ("OPT_GO", newstyle_opt_go_state_machine);
Group ("OPT_EXPORT_NAME", newstyle_opt_export_name_state_machine);
+ (* When NBD_OPT_GO fails, or when opt_mode is enabled, option parsing
+ * can be cleanly ended without moving through the %READY state.
+ *)
+ State {
+ default_state with
+ name = "PREPARE_OPT_ABORT";
+ comment = "Prepare to send NBD_OPT_ABORT";
+ external_events = [];
+ };
+
+ State {
+ default_state with
+ name = "SEND_OPT_ABORT";
+ comment = "Send NBD_OPT_ABORT to end negotiation";
+ external_events = [ NotifyWrite, "" ];
+ };
+
+ State {
+ default_state with
+ name = "SEND_OPTION_SHUTDOWN";
+ comment = "Sending write shutdown notification to the remote server";
+ external_events = [ NotifyWrite, "" ];
+ };
+
(* When option parsing has successfully finished negotiation
* it will jump to this state for final steps before moving to
* the %READY state.
diff --git a/generator/states-magic.c b/generator/states-magic.c
index 944728d..644bf73 100644
--- a/generator/states-magic.c
+++ b/generator/states-magic.c
@@ -1,5 +1,5 @@
/* nbd client library in userspace: state machine
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -42,8 +42,10 @@ STATE_MACHINE {
}
version = be64toh (h->sbuf.new_handshake.version);
- if (version == NBD_NEW_VERSION)
+ if (version == NBD_NEW_VERSION) {
+ assert (h->current_opt == 0);
SET_NEXT_STATE (%.NEWSTYLE.START);
+ }
else if (version == NBD_OLD_VERSION)
SET_NEXT_STATE (%.OLDSTYLE.START);
else {
diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
index d696cae..1a59ae7 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -240,7 +240,7 @@ STATE_MACHINE {
set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32,
reply);
}
- SET_NEXT_STATE (%.DEAD);
+ SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
break;
case NBD_REP_ACK:
SET_NEXT_STATE (%^FINISHED);
diff --git a/generator/states-newstyle-opt-starttls.c
b/generator/states-newstyle-opt-starttls.c
index 7162c7a..d107b74 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -1,5 +1,5 @@
/* nbd client library in userspace: state machine
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -93,7 +93,7 @@ STATE_MACHINE {
* then we can continue unencrypted.
*/
if (h->tls == LIBNBD_TLS_REQUIRE) {
- SET_NEXT_STATE (%.DEAD);
+ SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
set_error (ENOTSUP, "handshake: server refused TLS, "
"but handle TLS setting is 'require' (2)");
return 0;
diff --git a/generator/states-newstyle-opt-structured-reply.c
b/generator/states-newstyle-opt-structured-reply.c
index 58a76db..71a4952 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -1,5 +1,5 @@
/* nbd client library in userspace: state machine
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -84,7 +84,10 @@ STATE_MACHINE {
}
/* Next option. */
- SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
return 0;
} /* END STATE MACHINE */
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 8da4617..6613f9f 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -112,6 +112,26 @@ handle_reply_error (struct nbd_handle *h)
STATE_MACHINE {
NEWSTYLE.START:
+ if (h->opt_mode) {
+ /* NEWSTYLE can be entered multiple times, from MAGIC.CHECK_MAGIC and
+ * during various nbd_opt_* calls during NEGOTIATION. Each previous
+ * state has informed us what we still need to do.
+ */
+ switch (h->current_opt) {
+ case NBD_OPT_ABORT:
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (ENOTSUP, "handshake: server is not using fixed newstyle");
+ return 0;
+ }
+ SET_NEXT_STATE (%PREPARE_OPT_ABORT);
+ return 0;
+ case 0:
+ break;
+ default:
+ abort ();
+ }
+ }
h->rbuf = &h->sbuf;
h->rlen = sizeof h->sbuf.gflags;
SET_NEXT_STATE (%RECV_GFLAGS);
@@ -153,13 +173,48 @@ STATE_MACHINE {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
/* Start sending options. */
- if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
- SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
+ }
else
SET_NEXT_STATE (%OPT_STARTTLS.START);
}
return 0;
+ NEWSTYLE.PREPARE_OPT_ABORT:
+ assert ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) != 0);
+ h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
+ h->sbuf.option.option = htobe32 (NBD_OPT_ABORT);
+ h->sbuf.option.optlen = htobe32 (0);
+ h->wbuf = &h->sbuf;
+ h->wlen = sizeof h->sbuf.option;
+ SET_NEXT_STATE (%SEND_OPT_ABORT);
+ return 0;
+
+ NEWSTYLE.SEND_OPT_ABORT:
+ switch (send_from_wbuf (h)) {
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
+ case 0:
+ SET_NEXT_STATE (%SEND_OPTION_SHUTDOWN);
+ }
+ return 0;
+
+ NEWSTYLE.SEND_OPTION_SHUTDOWN:
+ /* We don't care if the server replies to NBD_OPT_ABORT. However,
+ * unless we are in opt mode, we want to preserve the error message
+ * from a failed OPT_GO by moving to DEAD instead.
+ */
+ if (h->sock->ops->shut_writes (h, h->sock)) {
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.CLOSED);
+ else
+ SET_NEXT_STATE (%.DEAD);
+ }
+ return 0;
+
NEWSTYLE.FINISHED:
SET_NEXT_STATE (%.READY);
return 0;
diff --git a/lib/opt.c b/lib/opt.c
index 306a2e9..6243553 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -39,3 +39,37 @@ nbd_unlocked_get_opt_mode (struct nbd_handle *h)
{
return h->opt_mode;
}
+
+static int
+wait_for_option (struct nbd_handle *h)
+{
+ while (nbd_internal_is_state_connecting (get_next_state (h))) {
+ if (nbd_unlocked_poll (h, -1) == -1)
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Issue NBD_OPT_ABORT and wait for the state change. */
+int
+nbd_unlocked_opt_abort (struct nbd_handle *h)
+{
+ int r = nbd_unlocked_aio_opt_abort (h);
+
+ if (r == -1)
+ return r;
+
+ return wait_for_option (h);
+}
+
+/* Issue NBD_OPT_ABORT without waiting. */
+int
+nbd_unlocked_aio_opt_abort (struct nbd_handle *h)
+{
+ h->current_opt = NBD_OPT_ABORT;
+
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 46c819a..76b370a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -163,6 +163,7 @@ check_PROGRAMS += \
can-not-cache-flag \
oldstyle \
newstyle-limited \
+ opt-abort \
connect-unix \
connect-tcp \
aio-parallel \
@@ -198,6 +199,7 @@ TESTS += \
can-not-cache-flag \
oldstyle \
newstyle-limited \
+ opt-abort \
connect-unix \
connect-tcp \
aio-parallel.sh \
@@ -373,6 +375,11 @@ newstyle_limited_CPPFLAGS = -I$(top_srcdir)/include
newstyle_limited_CFLAGS = $(WARNINGS_CFLAGS)
newstyle_limited_LDADD = $(top_builddir)/lib/libnbd.la
+opt_abort_SOURCES = opt-abort.c
+opt_abort_CPPFLAGS = -I$(top_srcdir)/include
+opt_abort_CFLAGS = $(WARNINGS_CFLAGS)
+opt_abort_LDADD = $(top_builddir)/lib/libnbd.la
+
connect_unix_SOURCES = connect-unix.c
connect_unix_CPPFLAGS = -I$(top_srcdir)/include
connect_unix_CFLAGS = $(WARNINGS_CFLAGS)
diff --git a/tests/errors.c b/tests/errors.c
index cde0f48..1ae6555 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -178,6 +178,15 @@ main (int argc, char *argv[])
}
check (EINVAL, "nbd_connect_command: ");
+ /* nbd_opt_abort can only be called during negotiation. */
+ if (nbd_opt_abort (nbd) != -1) {
+ fprintf (stderr, "%s: test failed: "
+ "nbd_opt_abort did not reject attempt in wrong state\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ check (EINVAL, "nbd_opt_abort: ");
+
/* Try to notify that writes are ready when we aren't blocked on POLLOUT */
if (nbd_aio_notify_write (nbd) != -1) {
fprintf (stderr, "%s: test failed: "
diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c
index 424da19..cdda474 100644
--- a/tests/newstyle-limited.c
+++ b/tests/newstyle-limited.c
@@ -121,6 +121,32 @@ main (int argc, char *argv[])
}
nbd_close (nbd);
+ /* Next try. Requesting opt_mode works, but opt_go is the only
+ * option that can succeed (via NBD_OPT_EXPORT_NAME); opt_abort is
+ * special-cased but moves to DEAD rather than CLOSED.
+ */
+ nbd = nbd_create ();
+ if (nbd == NULL ||
+ nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_connect_command (nbd, args) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (!nbd_aio_is_negotiating (nbd)) {
+ fprintf (stderr, "unexpected state after negotiating\n");
+ exit (EXIT_FAILURE);
+ }
+ /* XXX Test nbd_opt_list when it is implemented */
+ if (nbd_opt_abort (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (!nbd_aio_is_dead (nbd)) {
+ fprintf (stderr, "unexpected state after opt_abort\n");
+ exit (EXIT_FAILURE);
+ }
+ nbd_close (nbd);
+
/* And another try: an incorrect export name kills the connection,
* rather than allowing a second try.
*/
diff --git a/tests/opt-abort.c b/tests/opt-abort.c
new file mode 100644
index 0000000..3acc1fb
--- /dev/null
+++ b/tests/opt-abort.c
@@ -0,0 +1,95 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2020 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Test behavior of nbd_opt_abort. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <string.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int64_t r;
+ const char *s;
+ char *args[] = { "nbdkit", "-s", "--exit-with-parent",
"-v",
+ "memory", "size=512", NULL };
+
+ /* Get into negotiating state. */
+ nbd = nbd_create ();
+ if (nbd == NULL ||
+ nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_connect_command (nbd, args) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ /* Protocol should be "newstyle-fixed", with structured replies already
+ * negotiated.
+ */
+ if (nbd_aio_is_negotiating (nbd) != true) {
+ fprintf (stderr, "unexpected state after connection\n");
+ exit (EXIT_FAILURE);
+ }
+ s = nbd_get_protocol (nbd);
+ if (strcmp (s, "newstyle-fixed") != 0) {
+ fprintf (stderr,
+ "incorrect protocol \"%s\", expected
\"newstyle-fixed\"\n", s);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_get_structured_replies_negotiated (nbd)) != 1) {
+ fprintf (stderr,
+ "incorrect structured replies %" PRId64 ", expected
1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Quitting negotiation should be graceful. */
+ if (nbd_opt_abort (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_aio_is_closed (nbd) != true) {
+ fprintf (stderr, "unexpected state after abort\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* As negotiation never finished, we have no size. */
+ if ((r = nbd_get_size (nbd)) != -1) {
+ fprintf (stderr, "%s: test failed: incorrect size, "
+ "actual %" PRIi64 ", expected -1",
+ argv[0], r);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_get_errno ()) != EINVAL) {
+ fprintf (stderr, "%s: test failed: unexpected errno, "
+ "actual %" PRIi64 ", expected %d EINVAL",
+ argv[0], r, EINVAL);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index 44b4655..6fdb362 100644
--- a/.gitignore
+++ b/.gitignore
@@ -172,6 +172,7 @@ Makefile.in
/tests/meta-base-allocation
/tests/newstyle-limited
/tests/oldstyle
+/tests/opt-abort
/tests/pki/
/tests/read-only-flag
/tests/read-write-flag
--
2.28.0