search for: offset_hol

...ta; > - h->rlen = sizeof h->sbuf.sr.payload.offset_data; > + h->rbuf = &h->sbuf.reply.payload.offset_data; > + h->rlen = sizeof h->sbuf.reply.payload.offset_data; > SET_NEXT_STATE (%RECV_OFFSET_DATA); > break; > > case NBD_REPLY_TYPE_OFFSET_HOLE: > if (cmd->type != NBD_CMD_READ || > - length != sizeof h->sbuf.sr.payload.offset_hole) > + length != sizeof h->sbuf.reply.payload.offset_hole) > goto resync; > - h->rbuf = &h->sbuf.sr.payload.offset_hole; > - h->rlen = sizeo...

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

...return -1; - case 0: - flags = be16toh (h->sbuf.sr.structured_reply.flags); - - if (flags & NBD_REPLY_FLAG_DONE) - SET_NEXT_STATE (%^FINISH_COMMAND); - else - SET_NEXT_STATE (%.READY); + case 0: SET_NEXT_STATE (%FINISH); } return 0; REPLY.STRUCTURED_REPLY.RECV_OFFSET_HOLE: struct command_in_flight *cmd; uint64_t handle; - uint16_t flags; uint64_t offset; uint32_t length; @@ -359,7 +345,6 @@ case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: handle = be64toh (h->sbuf.sr.structured_reply.handle); - flags = be16toh (h->sbuf.sr.struc...

..."); > + free (buf); > + return nbd_mark_dead (h); > + } > + memcpy (&offset, buf, sizeof offset); > + offset = be64toh (offset); > + len = rep.structured.length - sizeof offset; > + break; leaks buf > + case NBD_REPLY_TYPE_OFFSET_HOLE: > + if (rep.structured.length != sizeof offset + sizeof len) { > + nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); > + free (buf); > + return nbd_mark_dead (h); > + } > + memcpy (&offset, buf, sizeof offset); > +...

On Mon, Jun 17, 2019 at 07:07:53PM -0500, Eric Blake wrote: > The NBD spec requires that a server doing structured reads must not > succeed unless it covers the entire buffer with reply chunks. In the > general case, this requires a lot of bookkeeping to check whether > offsets were non-overlapping and sufficient, and we'd rather defer > such checking to an optional callback

...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) { @@ -336,23 +310,16 @@ return 0; REPLY.STRUCTURED_REPLY.RECV_OFFSET_HOLE: - struct command_in_flight *cmd; - uint64_t handle; + struct command_in_flight *cmd = h->reply_cmd; uint64_t offset; uint32_t length; switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: - handle = be64toh (h->sbuf.sr.structured_reply.hand...

...quot;RECV_OFFSET_DATA_DATA"; > comment = "Receive a structured reply offset-data block of data"; > - external_events = [ NotifyRead, "" ]; > + external_events = []; > }; > > 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"; > comm...

..._error ("structured reply OFFSET_DATA too small"); + free (buf); + return nbd_mark_dead (h); + } + memcpy (&offset, buf, sizeof offset); + offset = be64toh (offset); + len = rep.structured.length - sizeof offset; + break; + case NBD_REPLY_TYPE_OFFSET_HOLE: + if (rep.structured.length != sizeof offset + sizeof len) { + nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); + free (buf); + return nbd_mark_dead (h); + } + memcpy (&offset, buf, sizeof offset); + offset = be64toh (offset); +...

...{ default_state with name = "RECV_OFFSET_DATA_DATA"; comment = "Receive a structured reply offset-data block of data"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; 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-...

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

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

Patch 1 played with an early draft of Rich's Fedora 30 libnbd package: https://bugzilla.redhat.com/show_bug.cgi?id=1713767#c17 Note that comment 21 provides a newer package 0.1.1-1 with a different API; and that libnbd has more unreleased API changes in the pipeline (whether that will be called 0.2 or 0.1.2); so we'll have to tweak things based on what is actually available in distros.

Updated based on other changes that have happened in the meantime: - rely more on cleanup.h (throughout) - split structured read for easier review (patch 2 and 3 were combined in v1) - rely on nbdkit not leaking a server's partial answer (patch 3) - add tests (patch 5) - other bug fixes I found while testing it - drop EOVERFLOW patch for now; it will be separate once upstream NBD protocol

libnbd-0.1.2-1 is now available in Fedora 29/30 updates-testing, although it was not compiled against libxml2 so it lacks uri support (I ended up testing patch 4 with a self-built libnbd). Diffs since v1 - rebase to master, bump from libnbd 0.1 to 0.1.2, add URI support, better timing results Still not done - patch 5 needs associated tests Eric Blake (5): nbd: Check for libnbd nbd:

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

...nbdkit_error ("structured reply OFFSET_DATA too small"); - return nbdplug_mark_dead (h); - } - memcpy (&offset, payload, sizeof offset); - offset = be64toh (offset); - len = rep.structured.length - sizeof offset; - break; - case NBD_REPLY_TYPE_OFFSET_HOLE: - if (rep.structured.length != sizeof offset + sizeof len) { - nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); - return nbdplug_mark_dead (h); - } - memcpy (&offset, payload, sizeof offset); - offset = be64toh (offset); - memcpy (...

...nbdkit_error ("structured reply OFFSET_DATA too small"); - return nbdplug_mark_dead (h); - } - memcpy (&offset, payload, sizeof offset); - offset = be64toh (offset); - len = rep.structured.length - sizeof offset; - break; - case NBD_REPLY_TYPE_OFFSET_HOLE: - if (rep.structured.length != sizeof offset + sizeof len) { - nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); - return nbdplug_mark_dead (h); - } - memcpy (&offset, payload, sizeof offset); - offset = be64toh (offset); - memcpy (...

...{ - nbdkit_error ("structured reply OFFSET_DATA too small"); - return nbd_mark_dead (h); - } - memcpy (&offset, payload, sizeof offset); - offset = be64toh (offset); - len = rep.structured.length - sizeof offset; - break; - case NBD_REPLY_TYPE_OFFSET_HOLE: - if (rep.structured.length != sizeof offset + sizeof len) { - nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); - return nbd_mark_dead (h); - } - memcpy (&offset, payload, sizeof offset); - offset = be64toh (offset); - memcpy (&amp...

libnbd-0.1.4-1 is now available in Fedora 29/30 updates testing. Diffs since v2 - rebase to master, bump from libnbd 0.1.2 to 0.1.3+, add tests to TLS usage which flushed out the need to turn relative pathnames into absolute, doc tweaks Now that the testsuite covers TLS and libnbd has been fixed to provide the things I found lacking when developing v2, I'm leaning towards pushing this on

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