search for: set_next_state

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...

...tured.c b/generator/states-reply-structured.c index 6337dad..2125e41 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -103,7 +103,7 @@ set_error (0, "NBD_REPLY_FLAG_DONE must be set in NBD_REPLY_TYPE_NONE"); return -1; } - SET_NEXT_STATE (%^FINISH_COMMAND); + SET_NEXT_STATE (%FINISH); return 0; } else if (type == NBD_REPLY_TYPE_OFFSET_DATA) { @@ -225,7 +225,6 @@ REPLY.STRUCTURED_REPLY.RECV_ERROR_TAIL: struct command_in_flight *cmd; - uint16_t flags; uint64_t handle; uint32_t error; uint64_t offset; @@...

Some of these affect attempts to connect to older qemu-nbd versions, some of them were triggered by manual edits to qemu-nbd source code to provoke various other compliant (if uncommon) server behaviors. Eric Blake (4): starttls: Skip error payload if falling back to unencrypted states: Reject payload to NBD_REP_ACK meta-context: Skip error payload if server lacks meta_context states: Add

...index 2ee739f..3458f09 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -77,21 +77,13 @@ return 0; NEWSTYLE.OPT_GO.RECV_REPLY: - uint32_t len; - const size_t maxpayload = sizeof h->sbuf.or.payload; - switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: - /* Read the following payload if it is short enough to fit in the - * static buffer. If it's too long, skip it. - */ - len = be32toh (h->sbuf.or.option_reply.replylen); - if (len <= maxpayload) - h->rbuf = &h->sbuf.or.payl...

I'm still playing with ideas on how to split rstate from wstate (so that we can send a request without waiting for POLLIN to complete a pending reply), but this is some preliminary refactoring I found useful. I also fixed a couple of bugs while in the area (already pushed). There's a question of whether we want nbd_handle to be nearly 5k, or if we should instead keep it small and add one

...State machine for negotiating NBD_OPT_SET_META_CONTEXT. */ +/* State machine for negotiating NBD_OPT_SET/LIST_META_CONTEXT. */ STATE_MACHINE { - NEWSTYLE.OPT_SET_META_CONTEXT.START: + NEWSTYLE.OPT_META_CONTEXT.START: size_t i, nr_queries; uint32_t len; @@ -52,7 +52,7 @@ STATE_MACHINE { SET_NEXT_STATE (%SEND); return 0; - NEWSTYLE.OPT_SET_META_CONTEXT.SEND: + NEWSTYLE.OPT_META_CONTEXT.SEND: switch (send_from_wbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: @@ -64,7 +64,7 @@ STATE_MACHINE { } return 0; - NEWSTYLE.OPT_SET_META_CONTEXT.SEND_EXPORTNAMELEN: + NEWSTY...

I still need to wire in the use of *_notify functions into nbdkit to prove whether it makes the code any faster or easier to maintain, but at least the added example shows one good use case for the new API. Eric Blake (6): api: Add nbd_aio_in_flight generator: Allow DEAD state actions to run generator: Allow Int64 in callbacks states: Prepare for aio notify callback api: Add new

...; ]; + external_events = []; }; State { diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index ddc91ce..3b63d07 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -53,6 +53,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: /* guaranteed by START */ assert (cmd); diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 00659de..594525e 100644 --- a/generator/...

...State { > diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c > index ddc91ce..3b63d07 100644 > --- a/generator/states-reply-simple.c > +++ b/generator/states-reply-simple.c > @@ -53,6 +53,10 @@ > > switch (recv_into_rbuf (h)) { > case -1: SET_NEXT_STATE (%.DEAD); return -1; > + case 1: > + save_reply_state (h); > + SET_NEXT_STATE (%.READY); > + return 0; > case 0: > /* guaranteed by START */ > assert (cmd); > diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c > i...

....OPT_EXPORT_NAME.START: + conn->sbuf.option.version = htobe64 (NBD_NEW_VERSION); + conn->sbuf.option.option = htobe32 (NBD_OPT_EXPORT_NAME); + conn->sbuf.option.optlen = strlen (h->export_name); + conn->wbuf = &conn->sbuf; + conn->wlen = sizeof conn->sbuf.option; + SET_NEXT_STATE (%SEND); + return 0; + + NEWSTYLE.OPT_EXPORT_NAME.SEND: + switch (send_from_wbuf (conn)) { + case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 0: + conn->wbuf = h->export_name; + conn->wlen = strlen (h->export_name); + SET_NEXT_STATE (%SEND_EXPORT); + } + return 0; + +...

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must

...tured.c b/generator/states-reply-structured.c index 5791360..6d58742 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -149,21 +149,69 @@ } REPLY.STRUCTURED_REPLY.RECV_ERROR: + uint32_t length, msglen; + switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: + length = be32toh (h->sbuf.sr.structured_reply.length); + msglen = be16toh (h->sbuf.sr.payload.error.len); + if (msglen > length - sizeof h->sbuf.sr.payload.error) { + SET_NEXT_STATE (%.DEAD); + set_error (0, "error message length...

...conn->cmds_in_flight = cmd; conn->sbuf.request.magic = htobe32 (NBD_REQUEST_MAGIC); conn->sbuf.request.flags = htobe16 (cmd->flags); @@ -40,29 +37,43 @@ return 0; ISSUE_COMMAND.SEND_REQUEST: - struct command_in_flight *cmd; - switch (send_from_wbuf (conn)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; - case 0: - assert (conn->cmds_in_flight != NULL); - cmd = conn->cmds_in_flight; - assert (cmd->handle == be64toh (conn->sbuf.request.handle)); - if (cmd->type == NBD_CMD_WRITE) { - conn->wbuf = cmd->data; - conn->wlen = cmd->co...

...tion, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* State machine for sending NBD_OPT_LIST to list exports. + * + * This is skipped unless list exports mode was enabled on the handle. + */ + +STATE_MACHINE { + NEWSTYLE.OPT_LIST.START: + if (!h->list_exports) { + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); + return 0; + } + + h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); + h->sbuf.option.option = htobe32 (NBD_OPT_LIST); + h->sbuf.option.optlen = 0; + h->wbuf = &h->sbuf; + h->wlen = sizeof (h->sbuf.option); + SET_NEXT_STATE (%SEN...

This was v3 patch 2/22, reworked to address the confusion about how a structured reply header is read in two pieces before getting to the payload portion. I'm still working on rebasing the rest of my v3 series (patches 1, 3-22) from other comments given, but this seemed independent enough that it's worth posting now rather than holding it up for the rest of the series. Eric Blake (4):

...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-newst...

Proposal for new APIs to list exports. The general shape of the API can probably best be seen from the examples/list-exports.c example. Rich.

I'm posting this now, as I'm at the end of a workday and I got things working for manual experimentation. Still to do: - write interop tests for qemu-nbd and nbdkit (including my proposed patch addition of qemu-nbd -A to show qemu:allocation-depth) - figure out if we can make 'nbdinfo --map' use the new API to automatically select all contexts advertised by the server Eric Blake

...a/generator/states-issue-command.c +++ b/generator/states-issue-command.c @@ -42,6 +42,8 @@ h->request.count = htobe32 ((uint32_t) cmd->count); h->wbuf = &h->request; h->wlen = sizeof (h->request); + if (cmd->type == NBD_CMD_WRITE) + h->wflags = MSG_MORE; SET_NEXT_STATE (%SEND_REQUEST); return 0; diff --git a/generator/states-newstyle-opt-export-name.c b/generator/states-newstyle-opt-export-name.c index 774c93c..968cea8 100644 --- a/generator/states-newstyle-opt-export-name.c +++ b/generator/states-newstyle-opt-export-name.c @@ -25,6 +25,7 @@ h->sbuf.op...

...> if (r == -1) { > @@ -122,22 +125,22 @@ REPLY.CHECK_REPLY_MAGIC: > uint32_t magic; > uint64_t cookie; > > - magic = be32toh (h->sbuf.simple_reply.magic); > + magic = be32toh (h->sbuf.reply.hdr.magic); > if (magic == NBD_SIMPLE_REPLY_MAGIC) { > SET_NEXT_STATE (%SIMPLE_REPLY.START); > } > else if (magic == NBD_STRUCTURED_REPLY_MAGIC) { > - /* We've only read the bytes that fill simple_reply. The > - * structured_reply is longer, so prepare to read the remaining > + /* We've only read the bytes that fill hdr.simple....