search for: send_from_wbuf

...->rlen == 0) - return 0; /* move to next state */ - else - return 1; /* more data */ + if (h->rbuf) + h->rbuf += r; + h->rlen -= r; + } + + return 0; /* move to next state */ } static int @@ -90,21 +87,19 @@ send_from_wbuf (struct nbd_handle *h) { ssize_t r; - if (h->wlen == 0) - return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen); - if (r == -1) { - if (errno == EAGAIN || errno == EWOULDBLOCK) - return 1;...

...ets 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); r...

...XT. */ 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: + NEWSTYLE.OPT_META_CONTEXT.SEND_EXPORTNAMELEN: switch (send_from_wbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0;...

...when we move to another state. --- generator/states.c | 10 +++++++--- lib/internal.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/generator/states.c b/generator/states.c index e879a83..b0dab83 100644 --- a/generator/states.c +++ b/generator/states.c @@ -91,8 +91,8 @@ send_from_wbuf (struct nbd_handle *h) ssize_t r; if (h->wlen == 0) - return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, 0); + goto next_state; + r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, h-...

I thought this should produce a fairly dramatic performance gain. In fact I couldn't measure any performance difference at all. I think what's happening is we're actually paying an extra syscall (to discover the socket would block) and then doing the poll anyway. So I don't know if it's worth having this patch. It could be argued that it makes the code shorter (therefore

Appears to have a measurable benefit, see 3/3 for test results. Rich.

...cally when we move to another state. --- generator/states.c | 6 ++++-- lib/internal.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/generator/states.c b/generator/states.c index e879a83..36cca37 100644 --- a/generator/states.c +++ b/generator/states.c @@ -92,7 +92,7 @@ send_from_wbuf (struct nbd_handle *h) if (h->wlen == 0) return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, 0); + r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, h->wflags); if (r == -1) {...

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

...y (cmd->cb.opaque, handle, &error) == -1 && error) + cmd->error = error; + } + SET_NEXT_STATE (%.READY); return 0; diff --git a/generator/states.c b/generator/states.c index deea73c..c9c3ef7 100644 --- a/generator/states.c +++ b/generator/states.c @@ -111,6 +111,31 @@ send_from_wbuf (struct nbd_handle *h) return 0; /* move to next state */ } +/* Forcefully fail any remaining in-flight commands in list */ +void abort_commands (struct nbd_handle *h, + struct command_in_flight **list) +{ + struct command_in_flight *prev_cmd, *cmd; + +...

...t;next = conn->cmds_in_flight; - 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->da...

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

.... Previously, nbd_aio_command_completed() would never > locate such stranded commands, but adding a common point to fire the > notifier for such commands makes it also possible to move those > commands to the completion queue. > > +++ b/generator/states.c > @@ -111,6 +111,31 @@ send_from_wbuf (struct nbd_handle *h) > return 0; /* move to next state */ > } > > +/* Forcefully fail any remaining in-flight commands in list */ > +void abort_commands (struct nbd_handle *h, > + struct command_in_flight **list) > +{ > + struc...

...; + 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; + + NEWSTYLE.OPT_EXPORT_NAME.SEND_EXPORT: + switch (send_from_wbuf (conn)) { + case -...

Necessary when writing a client that wants to avoid unnecessary EINVAL errors from sending unaligned requests. At some point, we may want to add synchronous convenience API wrappers that do request splitting or read-modify-write to obey server constraints while still appearing to the library client as accepting any possible request. But such a wrapper should only be synchronous and not copied to

...the patch itself...) generator/states.c | 5 +++++ lib/crypto.c | 7 +++++++ lib/internal.h | 1 + 3 files changed, 13 insertions(+) diff --git a/generator/states.c b/generator/states.c index bce4f85..145e8c1 100644 --- a/generator/states.c +++ b/generator/states.c @@ -113,6 +113,11 @@ send_from_wbuf (struct nbd_handle *h) READY: if (h->cmds_to_issue) SET_NEXT_STATE (%ISSUE_COMMAND.START); + else { + assert (h->sock); + if (h->sock->ops->pending && h->sock->ops->pending (h->sock)) + SET_NEXT_STATE (%REPLY.START); + } return 0; DEAD...

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

...* Were we interrupted by reading a reply to an earlier command? If + * so, we can only get back here after a non-blocking jaunt through + * the REPLY engine, which means we are unlikely to be unblocked for + * writes yet; we want to advance back to the correct state but + * without trying a send_from_wbuf that will likely return 1. + */ if (h->wlen) { if (h->in_write_payload) - SET_NEXT_STATE(%SEND_WRITE_PAYLOAD); + *next_state = %SEND_WRITE_PAYLOAD; else - SET_NEXT_STATE(%SEND_REQUEST); + *next_state = %SEND_REQUEST; return 0; } -- 2.20.1

...tates.c | 2 +- lib/crypto.c | 2 +- lib/internal.h | 2 +- lib/socket.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/generator/states.c b/generator/states.c index 145e8c1..e879a83 100644 --- a/generator/states.c +++ b/generator/states.c @@ -92,7 +92,7 @@ send_from_wbuf (struct nbd_handle *h) if (h->wlen == 0) return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen); + r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, 0); if (r == -1) { if (errno == E...

...--- generator/states.c | 5 +++++ lib/rw.c | 32 +++++++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/generator/states.c b/generator/states.c index 202c305..22ce853 100644 --- a/generator/states.c +++ b/generator/states.c @@ -110,6 +110,11 @@ send_from_wbuf (struct nbd_connection *conn) /*----- End of prologue. -----*/ /* STATE MACHINE */ { + READY: + if (conn->cmds_to_issue) + SET_NEXT_STATE (%ISSUE_COMMAND.START); + return 0; + DEAD: if (conn->sock) { conn->sock->ops->close (conn->sock); diff --git a/lib/rw.c b/li...

....c | 6 ++++++ lib/internal.h | 3 +++ lib/rw.c | 24 ++++++++++++++++++++---- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/generator/states.c b/generator/states.c index 202c305..a4eecbd 100644 --- a/generator/states.c +++ b/generator/states.c @@ -110,6 +110,12 @@ send_from_wbuf (struct nbd_connection *conn) /*----- End of prologue. -----*/ /* STATE MACHINE */ { + READY: + conn->reached_ready = true; + if (conn->cmds_to_issue) + SET_NEXT_STATE (%ISSUE_COMMAND.START); + return 0; + DEAD: if (conn->sock) { conn->sock->ops->close (conn-&gt...