search for: recv_into_rbuf

...: 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);...

...et returns EAGAIN. --- generator/states.c | 91 ++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 48 deletions(-) diff --git a/generator/states.c b/generator/states.c index 145e8c1..cde934a 100644 --- a/generator/states.c +++ b/generator/states.c @@ -46,43 +46,40 @@ recv_into_rbuf (struct nbd_handle *h) void *rbuf; size_t rlen; - if (h->rlen == 0) - return 0; /* move to next state */ + while (h->rlen > 0) { + /* As a special case h->rbuf is allowed to be NULL, meaning + * throw away the data. + */ + if (h->rbuf) { +...

...al_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/gen...

...events = [ NotifyRead, "" ]; + 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 00659d...

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

...git a/generator/states-reply-structured.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_er...

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

...generator/states-newstyle-opt-go.c 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-...

...: https://gitlab.com/nbdkit/libnbd/-/blob/20dceca48623835c3e8a195ef4135cce5e33efba/generator/states-magic.c#L28 For example: $ nbdkit -s null NBDMAGICIHAVEOPT <- magic string sent by the server But in this case let's say the server is slow to send the magic string. The call to recv_into_rbuf -> h->sock->ops->recv -> recv returns EWOULDBLOCK (recv_into_rbuf returns 1). Then we stay in the same state (MAGIC.RECV_MAGIC), and return to the main program. At this point if the main program calls nbd_aio_get_direction it would return AIO_DIRECTION_READ. We are expecting to re...

...NBD_CMD_READ on the largest @@ -224,8 +212,7 @@ return 0; REPLY.STRUCTURED_REPLY.RECV_ERROR_TAIL: - struct command_in_flight *cmd; - uint64_t handle; + struct command_in_flight *cmd = h->reply_cmd; uint32_t error; uint64_t offset; uint16_t type; @@ -233,15 +220,9 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: - handle = be64toh (h->sbuf.sr.structured_reply.handle); error = be32toh (h->sbuf.sr.payload.error.error.error); type = be16toh (h->sbuf.sr.structured_reply.type); - /* Find the command amongst the commands...

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

...T_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; @@ -234,7 +233,6 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: - flags = be16toh (h->sbuf.sr.structured_reply.flags); handle = be64toh (h->sbuf.sr.structured_reply.handle); error = be32toh (h->sbuf.sr.payload.error.error.error); type = be16toh (h->sbuf.sr.structured...

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

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

On 7/6/2023 7:06 PM, Nir Soffer wrote: > > - After calling for example aio_notify_read(3), can I know that the > next reading from the file descriptor would block? > > No, you have to call again aio_get_direction() and poll again until > the event happens. > Well, what I mean is: After calling aio_notify_read, if aio_get_direction returns AIO_DIRECTION_READ or

...states-reply-simple.c b/generator/states-reply-simple.c index 935f6d2..ddc91ce 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -49,9 +49,22 @@ return 0; REPLY.SIMPLE_REPLY.RECV_READ_PAYLOAD: + struct command_in_flight *cmd = h->reply_cmd; + switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; - case 0: SET_NEXT_STATE (%^FINISH_COMMAND); + case 0: + /* guaranteed by START */ + assert (cmd); + if (cmd->cb.fn.read) { + assert (cmd->error == 0); + errno = 0; + if (cmd->cb.fn.read (cmd->cb.opaque, c...

...FOR_REPLY: + NEWSTYLE.OPT_META_CONTEXT.PREPARE_FOR_REPLY: h->rbuf = &h->sbuf.or.option_reply; h->rlen = sizeof h->sbuf.or.option_reply; SET_NEXT_STATE (%RECV_REPLY); return 0; - NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY: + NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY: switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: @@ -151,14 +151,14 @@ STATE_MACHINE { } return 0; - NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY_PAYLOAD: + NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY_PAYLOAD: switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return...

...states-reply-simple.c b/generator/states-reply-simple.c index 935f6d2..87622a0 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -49,9 +49,22 @@ return 0; REPLY.SIMPLE_REPLY.RECV_READ_PAYLOAD: + struct command_in_flight *cmd = h->reply_cmd; + switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; - case 0: SET_NEXT_STATE (%^FINISH_COMMAND); + case 0: + /* guaranteed by START */ + assert (cmd); + if (cmd->cb.fn.read) { + assert (cmd->error == 0); + errno = 0; + if (cmd->cb.fn.read (cmd->cb.opaque, c...

I've mentioned this topic before (in fact, the idea of adding NBD_CMD_FLAG_DF was first mentioned at [1]), but finally finished enough of an implementation to feel confident in posting it. I'd still like to add something under examples/ that uses the new API to implement strict checking of a server's structured replies read implementation (ensure that a server never sends data after

This wasn't exactly how I imagined it - I thought we'd change the generator so that ‘return -1’ wouldn't stop the state machine, but would save an error indication, keep running the machine until it blocks, then return an error. However this is fine, so ACK. If I was going to improve this patch in some way (and didn't implement the idea above) then: I'd add a new macro for