8:28 a.m.
Most of the states were calling SET_NEXT_STATE(%.DEAD) then using
return -1 on error, to reflect the fact that they had also called
set_error() and wanted the caller to notice the failure.
Unfortunately, the state machine engine refuses to run the entry code
of the next state when the current state returned -1, which meant the
DEAD state entry code never runs.
A concrete example of the problems this creates: qemu-nbd defaults to
allowing only one connection at a time (newer nbdkit with the
noparallel filter can do the same, althoug it's not as picky on export
names). The DEAD state claims to close() the fd as soon as we detect
a problem, where an example problem is requesting an export name not
present on the server (NBD_OPT_GO fails, so we move to the DEAD state
since we can't retry nbd_set_export_name without a new handle). But
since the libnbd code gave up by returning -1, the DEAD state cleanup
never runs, and we end up leaving the connection fd open instead,
blocking out a second connection until we use nbd_close() to free all
resources.
Worse, we haven't yet wired up nbd_close into the python bindings;
until that happens, there is no way in nbdsh to wipe out an existing
failed connection and replace it with another attempt where we set the
correct export name, short of completely exiting nbdsh:
$ qemu-nbd -k /tmp/a -f raw -x a file
$ ./run nbdsh
nbd> h.connect_unix('/tmp/a')
Traceback (most recent call last):
File "/usr/lib64/python3.7/code.py", line 90, in runcode
exec(code, self.locals)
File "<console>", line 1, in <module>
File "/home/eblake/libnbd/python/nbd.py", line 340, in connect_unix
return libnbdmod.connect_unix (self._o, unixsocket)
nbd.Error: nbd_connect_unix: handshake: server has no export named '': No such
file or directory (ENOENT)
nbd> h = nbd.NBD()
nbd> h.set_export_name('a')
nbd> h.connect_unix('/tmp/a')
... HANGS ...
This patch does not address the missing Python binding, but does fix
the cleanup problem by ensuring that any transition to DEAD closes the
client end of the fd right away, rather than waiting until the final
nbd_close(), while still returning an error to the caller. This is
done by mandating that other states never move to DEAD without first
setting an error but returning 0, then letting DEAD's code return -1.
---
generator/generator | 30 +++++++---
generator/states-connect.c | 24 ++++----
generator/states-issue-command.c | 4 +-
generator/states-magic.c | 6 +-
generator/states-newstyle-opt-export-name.c | 8 +--
generator/states-newstyle-opt-go.c | 20 +++----
.../states-newstyle-opt-set-meta-context.c | 24 ++++----
generator/states-newstyle-opt-starttls.c | 18 +++---
.../states-newstyle-opt-structured-reply.c | 10 ++--
generator/states-newstyle.c | 6 +-
generator/states-oldstyle.c | 4 +-
generator/states-reply-simple.c | 2 +-
generator/states-reply-structured.c | 58 +++++++++----------
generator/states-reply.c | 8 +--
generator/states.c | 4 +-
15 files changed, 121 insertions(+), 105 deletions(-)
diff --git a/generator/generator b/generator/generator
index 5cc3e80..45a030f 100755
--- a/generator/generator
+++ b/generator/generator
@@ -47,15 +47,26 @@ if not (Sys.file_exists "lib/handle.c") then
* Each handle starts in the top level START state.
*
* When you enter a state, the associated C code for that state
- * runs. If the C code calls SET_NEXT_STATE then the connection
- * enters the next state without blocking. If the C code does _not_
- * call SET_NEXT_STATE before returning then the state machine
- * blocks and will not be re-entered until an external event
- * happens (see below).
+ * runs. If the C code calls SET_NEXT_STATE and returns 0 then
+ * the connection enters the next state without blocking. If the
+ * C code calls SET_NEXT_STATE_AND_BLOCK and returns 0 then the
+ * connection blocks, but will resume with the code for the next
+ * state on the next external event. If the C code does _not_
+ * call either macro but returns 0, the state machine is blocked
+ * and will not be re-entered until an external event happens
+ * (see below), where the same C code will be executed again on
+ * re-entry. If the C code calls returns -1 after using
+ * set_error(), then the state machine blocks and the caller
+ * should report failure; the next external event will resume the
+ * state machine according to whether SET_NEXT_STATE was used.
*
* There are various end states such as CLOSED and DEAD. These
- * are not special, it's just that they have no way to move to
- * another state.
+ * are not special in relation to the above state transition rules,
+ * it's just that they have no way to move to another state. However,
+ * the DEAD state expects that set_error() was used in the previous
+ * state, and will return -1 itself after performing cleanup actions;
+ * the earlier state that wants to transition to DEAD should return 0
+ * rather than -1, so as not to bypass this cleanup.
*
* An external event is something like the file descriptor being
* ready to read or write, or the main program calling a function
@@ -2708,7 +2719,10 @@ let generate_lib_states_c () =
pr " abort (); /* Should never happen, but keeps GCC happy. */\n";
pr " }\n";
pr "\n";
- pr " if (r == -1) return -1;\n";
+ pr " if (r == -1) {\n";
+ pr " assert (nbd_get_error () != NULL);\n";
+ pr " return -1;\n";
+ pr " }\n";
pr " } while (!blocked);\n";
pr " return 0;\n";
pr "}\n";
diff --git a/generator/states-connect.c b/generator/states-connect.c
index c6a4ae7..9e2e1d4 100644
--- a/generator/states-connect.c
+++ b/generator/states-connect.c
@@ -55,12 +55,12 @@ disable_nagle (int sock)
if (fd == -1) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "socket");
- return -1;
+ return 0;
}
h->sock = nbd_internal_socket_create (fd);
if (!h->sock) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
disable_nagle (fd);
@@ -70,7 +70,7 @@ disable_nagle (int sock)
if (errno != EINPROGRESS) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "connect");
- return -1;
+ return 0;
}
}
return 0;
@@ -83,7 +83,7 @@ disable_nagle (int sock)
SOL_SOCKET, SO_ERROR, &status, &len) == -1) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "getsockopt: SO_ERROR");
- return -1;
+ return 0;
}
/* This checks the status of the original connect call. */
if (status == 0) {
@@ -93,7 +93,7 @@ disable_nagle (int sock)
else {
SET_NEXT_STATE (%.DEAD);
set_error (status, "connect");
- return -1;
+ return 0;
}
CONNECT_UNIX.START:
@@ -107,7 +107,7 @@ disable_nagle (int sock)
if (namelen > sizeof sun.sun_path) {
set_error (ENAMETOOLONG, "socket name too long: %s", h->unixsocket);
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
memcpy (sun.sun_path, h->unixsocket, namelen);
len = sizeof sun;
@@ -175,7 +175,7 @@ disable_nagle (int sock)
h->sock = nbd_internal_socket_create (fd);
if (!h->sock) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
disable_nagle (fd);
@@ -198,7 +198,7 @@ disable_nagle (int sock)
SOL_SOCKET, SO_ERROR, &status, &len) == -1) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "getsockopt: SO_ERROR");
- return -1;
+ return 0;
}
/* This checks the status of the original connect call. */
if (status == 0)
@@ -228,7 +228,7 @@ disable_nagle (int sock)
if (socketpair (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, sv) == -1) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "socketpair");
- return -1;
+ return 0;
}
pid = fork ();
@@ -237,7 +237,7 @@ disable_nagle (int sock)
set_error (errno, "fork");
close (sv[0]);
close (sv[1]);
- return -1;
+ return 0;
}
if (pid == 0) { /* child - run command */
close (0);
@@ -268,14 +268,14 @@ disable_nagle (int sock)
fcntl (sv[0], F_SETFL, flags|O_NONBLOCK) == -1) {
SET_NEXT_STATE (%.DEAD);
close (sv[0]);
- return -1;
+ return 0;
}
h->sock = nbd_internal_socket_create (sv[0]);
if (!h->sock) {
SET_NEXT_STATE (%.DEAD);
close (sv[0]);
- return -1;
+ return 0;
}
/* The sockets are connected already, we can jump directly to
diff --git a/generator/states-issue-command.c b/generator/states-issue-command.c
index 35f3c79..8890e1c 100644
--- a/generator/states-issue-command.c
+++ b/generator/states-issue-command.c
@@ -54,7 +54,7 @@
ISSUE_COMMAND.SEND_REQUEST:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%PREPARE_WRITE_PAYLOAD);
}
return 0;
@@ -85,7 +85,7 @@
ISSUE_COMMAND.SEND_WRITE_PAYLOAD:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%FINISH);
}
return 0;
diff --git a/generator/states-magic.c b/generator/states-magic.c
index 93c92fc..de8d235 100644
--- a/generator/states-magic.c
+++ b/generator/states-magic.c
@@ -27,7 +27,7 @@
MAGIC.RECV_MAGIC:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK_MAGIC);
}
return 0;
@@ -38,7 +38,7 @@
if (strncmp (h->sbuf.new_handshake.nbdmagic, "NBDMAGIC", 8) != 0) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "handshake: server did not send expected NBD magic");
- return -1;
+ return 0;
}
version = be64toh (h->sbuf.new_handshake.version);
@@ -49,7 +49,7 @@
else {
SET_NEXT_STATE (%.DEAD);
set_error (0, "handshake: server is not either an oldstyle or fixed newstyle NBD
server");
- return -1;
+ return 0;
}
return 0;
diff --git a/generator/states-newstyle-opt-export-name.c
b/generator/states-newstyle-opt-export-name.c
index 968cea8..ec73136 100644
--- a/generator/states-newstyle-opt-export-name.c
+++ b/generator/states-newstyle-opt-export-name.c
@@ -31,7 +31,7 @@
NEWSTYLE.OPT_EXPORT_NAME.SEND:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->wbuf = h->export_name;
h->wlen = strlen (h->export_name);
@@ -41,7 +41,7 @@
NEWSTYLE.OPT_EXPORT_NAME.SEND_EXPORT:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->rbuf = &h->sbuf;
h->rlen = sizeof h->sbuf.export_name_reply;
@@ -53,7 +53,7 @@
NEWSTYLE.OPT_EXPORT_NAME.RECV_REPLY:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK_REPLY);
}
return 0;
@@ -66,7 +66,7 @@
eflags = be16toh (h->sbuf.export_name_reply.eflags);
if (nbd_internal_set_size_and_flags (h, exportsize, eflags) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
SET_NEXT_STATE (%.READY);
return 0;
diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
index e245c75..49875a5 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -34,7 +34,7 @@
const uint32_t exportnamelen = strlen (h->export_name);
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->sbuf.len = htobe32 (exportnamelen);
h->wbuf = &h->sbuf;
@@ -46,7 +46,7 @@
NEWSTYLE.OPT_GO.SEND_EXPORTNAMELEN:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->wbuf = h->export_name;
h->wlen = strlen (h->export_name);
@@ -57,7 +57,7 @@
NEWSTYLE.OPT_GO.SEND_EXPORT:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->sbuf.nrinfos = 0;
h->wbuf = &h->sbuf;
@@ -68,7 +68,7 @@
NEWSTYLE.OPT_GO.SEND_NRINFOS:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->rbuf = &h->sbuf;
h->rlen = sizeof h->sbuf.or.option_reply;
@@ -78,11 +78,11 @@
NEWSTYLE.OPT_GO.RECV_REPLY:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
if (prepare_for_reply_payload (h, NBD_OPT_GO) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
}
@@ -90,7 +90,7 @@
NEWSTYLE.OPT_GO.RECV_REPLY_PAYLOAD:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK_REPLY);
}
return 0;
@@ -121,13 +121,13 @@
if (len != sizeof h->sbuf.or.payload.export) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "handshake: incorrect NBD_INFO_EXPORT option reply
length");
- return -1;
+ return 0;
}
exportsize = be64toh (h->sbuf.or.payload.export.exportsize);
eflags = be16toh (h->sbuf.or.payload.export.eflags);
if (nbd_internal_set_size_and_flags (h, exportsize, eflags) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
break;
default:
@@ -177,7 +177,7 @@
}
}
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
} /* END STATE MACHINE */
diff --git a/generator/states-newstyle-opt-set-meta-context.c
b/generator/states-newstyle-opt-set-meta-context.c
index a00a411..7904fe7 100644
--- a/generator/states-newstyle-opt-set-meta-context.c
+++ b/generator/states-newstyle-opt-set-meta-context.c
@@ -53,7 +53,7 @@
NEWSTYLE.OPT_SET_META_CONTEXT.SEND:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->sbuf.len = htobe32 (strlen (h->export_name));
h->wbuf = &h->sbuf.len;
@@ -65,7 +65,7 @@
NEWSTYLE.OPT_SET_META_CONTEXT.SEND_EXPORTNAMELEN:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->wbuf = h->export_name;
h->wlen = strlen (h->export_name);
@@ -76,7 +76,7 @@
NEWSTYLE.OPT_SET_META_CONTEXT.SEND_EXPORTNAME:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->sbuf.nrqueries =
htobe32 (nbd_internal_string_list_length (h->request_meta_contexts));
@@ -89,7 +89,7 @@
NEWSTYLE.OPT_SET_META_CONTEXT.SEND_NRQUERIES:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->querynum = 0;
SET_NEXT_STATE (%PREPARE_NEXT_QUERY);
@@ -115,7 +115,7 @@
const char *query = h->request_meta_contexts[h->querynum];
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->wbuf = query;
h->wlen = strlen (query);
@@ -125,7 +125,7 @@
NEWSTYLE.OPT_SET_META_CONTEXT.SEND_QUERY:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->querynum++;
SET_NEXT_STATE (%PREPARE_NEXT_QUERY);
@@ -140,11 +140,11 @@
NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
if (prepare_for_reply_payload (h, NBD_OPT_SET_META_CONTEXT) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
}
@@ -152,7 +152,7 @@
NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY_PAYLOAD:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK_REPLY);
}
return 0;
@@ -178,7 +178,7 @@
if (meta_context == NULL) {
set_error (errno, "malloc");
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
meta_context->context_id =
be32toh (h->sbuf.or.payload.context.context.context_id);
@@ -189,7 +189,7 @@
set_error (errno, "strdup");
SET_NEXT_STATE (%.DEAD);
free (meta_context);
- return -1;
+ return 0;
}
debug (h, "negotiated %s with context ID %" PRIu32,
meta_context->name, meta_context->context_id);
@@ -202,7 +202,7 @@
/* Anything else is an error, ignore it */
if (handle_reply_error (h) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
debug (h, "handshake: unexpected error from "
diff --git a/generator/states-newstyle-opt-starttls.c
b/generator/states-newstyle-opt-starttls.c
index 61f254f..0a18db0 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -36,7 +36,7 @@
NEWSTYLE.OPT_STARTTLS.SEND:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->rbuf = &h->sbuf;
h->rlen = sizeof (h->sbuf.or.option_reply);
@@ -46,11 +46,11 @@
NEWSTYLE.OPT_STARTTLS.RECV_REPLY:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
if (prepare_for_reply_payload (h, NBD_OPT_STARTTLS) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
}
@@ -58,7 +58,7 @@
NEWSTYLE.OPT_STARTTLS.RECV_REPLY_PAYLOAD:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK_REPLY);
}
return 0;
@@ -73,7 +73,7 @@
new_sock = nbd_internal_crypto_create_session (h, h->sock);
if (new_sock == NULL) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
h->sock = new_sock;
if (nbd_internal_crypto_is_reading (h))
@@ -85,7 +85,7 @@
default:
if (handle_reply_error (h) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
/* Server refused to upgrade to TLS. If h->tls is not require (2)
@@ -95,7 +95,7 @@
SET_NEXT_STATE (%.DEAD);
set_error (ENOTSUP, "handshake: server refused TLS, "
"but handle TLS setting is require (2)");
- return -1;
+ return 0;
}
debug (h,
@@ -112,7 +112,7 @@
r = nbd_internal_crypto_handshake (h);
if (r == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
if (r == 0) {
/* Finished handshake. */
@@ -135,7 +135,7 @@
r = nbd_internal_crypto_handshake (h);
if (r == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
if (r == 0) {
/* Finished handshake. */
diff --git a/generator/states-newstyle-opt-structured-reply.c
b/generator/states-newstyle-opt-structured-reply.c
index 65d5958..d932248 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -30,7 +30,7 @@
NEWSTYLE.OPT_STRUCTURED_REPLY.SEND:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
h->rbuf = &h->sbuf;
h->rlen = sizeof h->sbuf.or.option_reply;
@@ -40,11 +40,11 @@
NEWSTYLE.OPT_STRUCTURED_REPLY.RECV_REPLY:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
if (prepare_for_reply_payload (h, NBD_OPT_STRUCTURED_REPLY) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
}
@@ -52,7 +52,7 @@
NEWSTYLE.OPT_STRUCTURED_REPLY.RECV_REPLY_PAYLOAD:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK_REPLY);
}
return 0;
@@ -69,7 +69,7 @@
default:
if (handle_reply_error (h) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
debug (h, "structured replies are not supported by this server");
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 912ecb5..c8f817e 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -119,7 +119,7 @@ handle_reply_error (struct nbd_handle *h)
NEWSTYLE.RECV_GFLAGS:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK_GFLAGS);
}
return 0;
@@ -133,7 +133,7 @@ handle_reply_error (struct nbd_handle *h)
SET_NEXT_STATE (%.DEAD);
set_error (ENOTSUP, "handshake: server is not fixed newstyle, "
"but handle TLS setting is require (2)");
- return -1;
+ return 0;
}
cflags = h->gflags & (NBD_FLAG_FIXED_NEWSTYLE|NBD_FLAG_NO_ZEROES);
@@ -145,7 +145,7 @@ handle_reply_error (struct nbd_handle *h)
NEWSTYLE.SEND_CFLAGS:
switch (send_from_wbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
/* Start sending options. */
if ((h->gflags & NBD_FLAG_FIXED_NEWSTYLE) == 0)
diff --git a/generator/states-oldstyle.c b/generator/states-oldstyle.c
index b5618af..668931b 100644
--- a/generator/states-oldstyle.c
+++ b/generator/states-oldstyle.c
@@ -32,7 +32,7 @@
OLDSTYLE.RECV_REMAINING:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0: SET_NEXT_STATE (%CHECK);
}
return 0;
@@ -51,7 +51,7 @@
if (nbd_internal_set_size_and_flags (h, exportsize, eflags) == -1) {
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
SET_NEXT_STATE (%.READY);
diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c
index cab72d6..23b6b5f 100644
--- a/generator/states-reply-simple.c
+++ b/generator/states-reply-simple.c
@@ -52,7 +52,7 @@
struct command_in_flight *cmd = h->reply_cmd;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
index 91c6215..9a8677d 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -37,7 +37,7 @@
REPLY.STRUCTURED_REPLY.RECV_REMAINING:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
@@ -70,14 +70,14 @@
if (length > MAX_REQUEST_SIZE + sizeof h->sbuf.sr.payload.offset_data) {
set_error (0, "invalid server reply length");
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
if (NBD_REPLY_TYPE_IS_ERR (type)) {
if (length < sizeof h->sbuf.sr.payload.error.error) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "too short length in structured reply error");
- return -1;
+ return 0;
}
h->rbuf = &h->sbuf.sr.payload.error.error;
h->rlen = sizeof h->sbuf.sr.payload.error.error;
@@ -88,12 +88,12 @@
if (length != 0) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "invalid length in NBD_REPLY_TYPE_NONE");
- return -1;
+ return 0;
}
if (!(flags & NBD_REPLY_FLAG_DONE)) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "NBD_REPLY_FLAG_DONE must be set in NBD_REPLY_TYPE_NONE");
- return -1;
+ return 0;
}
SET_NEXT_STATE (%FINISH);
return 0;
@@ -109,12 +109,12 @@
"cmd->type=%" PRIu16 ", "
"this is likely to be a bug in the server",
cmd->type);
- return -1;
+ return 0;
}
if (length < sizeof h->sbuf.sr.payload.offset_data) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "too short length in NBD_REPLY_TYPE_OFFSET_DATA");
- return -1;
+ return 0;
}
h->rbuf = &h->sbuf.sr.payload.offset_data;
h->rlen = sizeof h->sbuf.sr.payload.offset_data;
@@ -128,12 +128,12 @@
"cmd->type=%" PRIu16 ", "
"this is likely to be a bug in the server",
cmd->type);
- return -1;
+ return 0;
}
if (length != sizeof h->sbuf.sr.payload.offset_hole) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "invalid length in NBD_REPLY_TYPE_OFFSET_HOLE");
- return -1;
+ return 0;
}
h->rbuf = &h->sbuf.sr.payload.offset_hole;
h->rlen = sizeof h->sbuf.sr.payload.offset_hole;
@@ -147,18 +147,18 @@
"cmd->type=%" PRIu16 ", "
"this is likely to be a bug in the server",
cmd->type);
- return -1;
+ return 0;
}
/* XXX We should be able to skip the bad reply in these two cases. */
if (length < 12 || ((length-4) & 7) != 0) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "invalid length in NBD_REPLY_TYPE_BLOCK_STATUS");
- return -1;
+ return 0;
}
if (cmd->cb.fn.extent == NULL) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "not expecting NBD_REPLY_TYPE_BLOCK_STATUS here");
- return -1;
+ return 0;
}
/* We read the context ID followed by all the entries into a
* single array and deal with it at the end.
@@ -168,7 +168,7 @@
if (h->bs_entries == NULL) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "malloc");
- return -1;
+ return 0;
}
h->rbuf = h->bs_entries;
h->rlen = length;
@@ -178,14 +178,14 @@
else {
SET_NEXT_STATE (%.DEAD);
set_error (0, "unknown structured reply type (%" PRIu16 ")",
type);
- return -1;
+ return 0;
}
REPLY.STRUCTURED_REPLY.RECV_ERROR:
uint32_t length, msglen;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
@@ -197,7 +197,7 @@
msglen > sizeof h->sbuf.sr.payload.error.msg) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "error message length too large");
- return -1;
+ return 0;
}
h->rbuf = h->sbuf.sr.payload.error.msg;
@@ -211,7 +211,7 @@
uint16_t type;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
@@ -235,14 +235,14 @@
if (length != 0) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "error payload length too large");
- return -1;
+ return 0;
}
break;
case NBD_REPLY_TYPE_ERROR_OFFSET:
if (length != sizeof h->sbuf.sr.payload.error.offset) {
SET_NEXT_STATE (%.DEAD);
set_error (0, "invalid error payload length");
- return -1;
+ return 0;
}
h->rbuf = &h->sbuf.sr.payload.error.offset;
break;
@@ -258,7 +258,7 @@
uint16_t type;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
@@ -289,7 +289,7 @@
"cmd->count=%" PRIu32 ", "
"this is likely to be a bug in the server",
offset, cmd->offset, cmd->count);
- return -1;
+ return 0;
}
if (cmd->type == NBD_CMD_READ && cmd->cb.fn.read) {
int scratch = error;
@@ -319,7 +319,7 @@
uint32_t length;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
@@ -343,7 +343,7 @@
"offset=%" PRIu64 ", cmd->offset=%" PRIu64
", "
"this is likely to be a bug in the server",
offset, cmd->offset);
- return -1;
+ return 0;
}
/* Now this is the byte offset in the read buffer. */
offset -= cmd->offset;
@@ -355,7 +355,7 @@
"cmd->count=%" PRIu32 ", "
"this is likely to be a bug in the server",
offset, length, cmd->count);
- return -1;
+ return 0;
}
/* Set up to receive the data directly to the user buffer. */
@@ -371,7 +371,7 @@
uint32_t length;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
@@ -401,7 +401,7 @@
uint32_t length;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
@@ -422,7 +422,7 @@
"offset=%" PRIu64 ", cmd->offset=%" PRIu64
", "
"this is likely to be a bug in the server",
offset, cmd->offset);
- return -1;
+ return 0;
}
/* Now this is the byte offset in the read buffer. */
offset -= cmd->offset;
@@ -434,7 +434,7 @@
"cmd->count=%" PRIu32 ", "
"this is likely to be a bug in the server",
offset, length, cmd->count);
- return -1;
+ return 0;
}
/* The spec states that 0-length requests are unspecified, but
@@ -464,7 +464,7 @@
struct meta_context *meta_context;
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1:
save_reply_state (h);
SET_NEXT_STATE (%.READY);
diff --git a/generator/states-reply.c b/generator/states-reply.c
index 6fb0a7a..88274bc 100644
--- a/generator/states-reply.c
+++ b/generator/states-reply.c
@@ -85,7 +85,7 @@ save_reply_state (struct nbd_handle *h)
/* sock->ops->recv called set_error already. */
SET_NEXT_STATE (%.DEAD);
- return -1;
+ return 0;
}
if (r == 0) {
SET_NEXT_STATE (%.CLOSED);
@@ -99,7 +99,7 @@ save_reply_state (struct nbd_handle *h)
REPLY.RECV_REPLY:
switch (recv_into_rbuf (h)) {
- case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 1: SET_NEXT_STATE (%.READY); return 0;
case 0: SET_NEXT_STATE (%CHECK_SIMPLE_OR_STRUCTURED_REPLY);
}
@@ -120,7 +120,7 @@ save_reply_state (struct nbd_handle *h)
else {
SET_NEXT_STATE (%.DEAD); /* We've probably lost synchronization. */
set_error (0, "invalid reply magic");
- return -1;
+ return 0;
}
/* NB: This works for both simple and structured replies because the
@@ -141,7 +141,7 @@ save_reply_state (struct nbd_handle *h)
SET_NEXT_STATE (%.DEAD);
set_error (0, "no matching handle found for server reply, "
"this is probably a bug in the server");
- return -1;
+ return 0;
}
h->reply_cmd = cmd;
return 0;
diff --git a/generator/states.c b/generator/states.c
index b0dab83..deea73c 100644
--- a/generator/states.c
+++ b/generator/states.c
@@ -125,11 +125,13 @@ send_from_wbuf (struct nbd_handle *h)
return 0;
DEAD:
+ /* The caller should have used set_error() before reaching here */
+ assert (nbd_get_error ());
if (h->sock) {
h->sock->ops->close (h->sock);
h->sock = NULL;
}
- return 0;
+ return -1;
CLOSED:
if (h->sock) {
--
2.20.1