search for: recv_bs_entri

...le changed, 58 insertions(+), 20 deletions(-) > > diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c > index 17bb5149..735f9456 100644 > --- a/generator/states-reply-chunk.c > +++ b/generator/states-reply-chunk.c > @@ -461,6 +461,11 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES: > struct command *cmd = h->reply_cmd; > size_t i; > uint32_t context_id; > + int error; > + const char *name; > + uint32_t orig_len, len, flags; > + uint64_t total, cap; > + bool stop; > > switch (recv_into_rbuf (h)) { > case -1: SET_NEXT_...

...mix non-default settings (using nbd_set_strict to disable STRICT_COMMANDS) to send a block status without having negotiated it, coupled with a non-compliant server that responds with status anyways, we can then hit the assertion failure where h->meta_valid is not set during the REPLY.CHUNK_REPLY.RECV_BS_ENTRIES state. Demonstration of the assertion failure can be done with a quick patch to nbdkit (here, on top of v1.35.6): | diff --git i/server/protocol.c w/server/protocol.c | index cb530e65..6b115d99 100644 | --- i/server/protocol.c | +++ w/server/protocol.c | @@ -190,7 +190,7 @@ validate_request (ui...

...> (using nbd_set_strict to disable STRICT_COMMANDS) to send a block > status without having negotiated it, coupled with a non-compliant > server that responds with status anyways, we can then hit the > assertion failure where h->meta_valid is not set during the > REPLY.CHUNK_REPLY.RECV_BS_ENTRIES state. > > Demonstration of the assertion failure can be done with a quick patch > to nbdkit (here, on top of v1.35.6): > > | diff --git i/server/protocol.c w/server/protocol.c > | index cb530e65..6b115d99 100644 > | --- i/server/protocol.c > | +++ w/server/protocol.c &g...

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

...load.offset_hole.length); @@ -404,17 +389,13 @@ memset (cmd->data + offset, 0, length); - if (flags & NBD_REPLY_FLAG_DONE) - SET_NEXT_STATE (%^FINISH_COMMAND); - else - SET_NEXT_STATE (%.READY); + SET_NEXT_STATE(%FINISH); } return 0; REPLY.STRUCTURED_REPLY.RECV_BS_ENTRIES: struct command_in_flight *cmd; uint64_t handle; - uint16_t flags; uint32_t length; size_t i; uint32_t context_id; @@ -424,7 +405,6 @@ case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: handle = be64toh (h->sbuf.sr.structured_reply.handle); - flags = be16toh (h-...

...mmands in flight. */ - for (cmd = h->cmds_in_flight; cmd != NULL; cmd = cmd->next) { - if (cmd->handle == handle) - break; - } assert (cmd); /* guaranteed by CHECK */ if (cmd->type != NBD_CMD_READ) { @@ -394,8 +361,7 @@ return 0; REPLY.STRUCTURED_REPLY.RECV_BS_ENTRIES: - struct command_in_flight *cmd; - uint64_t handle; + struct command_in_flight *cmd = h->reply_cmd; uint32_t length; size_t i; uint32_t context_id; @@ -404,16 +370,9 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: - handle = be64to...

...gt; name = "RECV_OFFSET_HOLE"; > comment = "Receive a structured reply offset-hole header"; > - external_events = [ NotifyRead, "" ]; > + external_events = []; > }; > > State { > default_state with > name = "RECV_BS_ENTRIES"; > comment = "Receive a structured reply block-status payload"; > - external_events = [ NotifyRead, "" ]; > + external_events = []; > }; > > State { > diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c &g...

...; }; State { default_state with name = "RECV_OFFSET_HOLE"; comment = "Receive a structured reply offset-hole header"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_BS_ENTRIES"; comment = "Receive a structured reply block-status payload"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index ddc91ce..3b63d07 100644 --- a/g...

...h (h->sbuf.sr.payload.offset_hole.length); > + offset = be64toh (h->sbuf.reply.payload.offset_hole.offset); > + length = be32toh (h->sbuf.reply.payload.offset_hole.length); > > assert (cmd); /* guaranteed by CHECK */ > > @@ -416,7 +416,7 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES: > SET_NEXT_STATE (%.READY); > return 0; > case 0: > - length = be32toh (h->sbuf.sr.structured_reply.length); > + length = be32toh (h->sbuf.reply.hdr.structured.length); > > assert (cmd); /* guaranteed by CHECK */ > assert (cmd->type...

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