search for: offset_data

...*/ > + struct { > + uint32_t magic; > + uint32_t pad_; > + uint64_t cookie; > + }; > + } hdr; > union { > uint64_t align_; /* Start sr.payload on an 8-byte alignment */ > struct nbd_structured_reply_offset_data offset_data; > @@ -250,7 +260,7 @@ struct nbd_handle { > uint64_t offset; /* Only used for NBD_REPLY_TYPE_ERROR_OFFSET */ > } NBD_ATTRIBUTE_PACKED error; > } payload; > - } sr; > + } reply; > uint16_t gflags; > uint32_t cflags; >...

...IC. */ + uint16_t flags; /* NBD_REPLY_FLAG_* */ + uint16_t type; /* NBD_REPLY_TYPE_* */ + uint64_t handle; /* Opaque handle. */ + uint32_t length; /* Length of payload which follows. */ +} __attribute__((packed)); + +struct structured_reply_offset_data { + uint64_t offset; /* offset */ + /* Followed by data. */ +} __attribute__((packed)); + +struct structured_reply_error { + uint32_t error; /* NBD_E* error number */ + uint16_t len; /* Length of human readable error. */ + /* Followed by human readabl...

...->send (conn, &reply, sizeof reply, f); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); return connection_set_status (conn, -1); @@ -439,7 +440,7 @@ send_structured_reply_read (struct connection *conn, reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA); reply.length = htobe32 (count + sizeof offset_data); - r = conn->send (conn, &reply, sizeof reply, 0); + r = conn->send (conn, &reply, sizeof reply, SEND_MORE); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); return connection...

This time around, the numbers are indeed looking better than in v1; and I like the interface better. Eric Blake (2): server: Prefer send() over write() server: Group related transmission send()s server/internal.h | 7 +++- server/connections.c | 51 +++++++++++++++++++++++++--- server/crypto.c | 11 ++++--

...d.length > 64 * 1024 * 1024) { > + nbdkit_error ("structured reply length is suspiciously large: %" PRId32, > + rep.structured.length); > + return nbd_mark_dead (h); > + } > + if (rep.structured.length) { > + /* Special case for OFFSET_DATA in order to read tail of chunk > + directly into final buffer later on */ > + len = (rep.structured.type == NBD_REPLY_TYPE_OFFSET_DATA && > + rep.structured.length > sizeof offset) ? sizeof offset : > + rep.structured.length; > + buf =...

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

...se we always honor it (but that's a larger diffstat, and thus at odds with "minimal implementation"). Either way works. > +/* Structured reply types. */ > +extern const char *name_of_nbd_reply_type (int); > +#define NBD_REPLY_TYPE_NONE 0 > +#define NBD_REPLY_TYPE_OFFSET_DATA 1 > +#define NBD_REPLY_TYPE_OFFSET_HOLE 2 > +#define NBD_REPLY_TYPE_BLOCK_STATUS 3 > +#define NBD_REPLY_TYPE_ERROR 32769 > +#define NBD_REPLY_TYPE_ERROR_OFFSET 32770 Worth writing these later ones in hex or via a helper macro that does ((1 << 15) | value)? Or would that...

..._FLAG_DF interacts with NBD_CMD_BLOCK_STATUS. What does it mean for extents which are by their nature fragmented? > > +/* Structured reply types. */ > > +extern const char *name_of_nbd_reply_type (int); > > +#define NBD_REPLY_TYPE_NONE 0 > > +#define NBD_REPLY_TYPE_OFFSET_DATA 1 > > +#define NBD_REPLY_TYPE_OFFSET_HOLE 2 > > +#define NBD_REPLY_TYPE_BLOCK_STATUS 3 > > +#define NBD_REPLY_TYPE_ERROR 32769 > > +#define NBD_REPLY_TYPE_ERROR_OFFSET 32770 > > Worth writing these later ones in hex or via a helper macro that does > ((1 &l...

...rab the write lock for each chunk, - * allowing other threads to interleave replies. As we're not doing - * that yet we acquire the lock for the whole function. - */ - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); - struct structured_reply reply; - struct structured_reply_offset_data offset_data; - int r; - - assert (cmd == NBD_CMD_READ); - - reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); - reply.handle = handle; - reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); - reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA); - reply.length = htobe32 (count + sizeof offset_data);...

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

...RECV_REMAINING: switch (recv_into_rbuf (h)) { - case -1: SET_NEXT_STATE (%.DEAD); return -1; + case -1: SET_NEXT_STATE (%.DEAD); return 0; case 1: save_reply_state (h); SET_NEXT_STATE (%.READY); @@ -70,14 +70,14 @@ if (length > MAX_REQUEST_SIZE + sizeof h->sbuf.sr.payload.offset_data) { set_error (0, "invalid server reply length"); SET_NEXT_STATE (%.DEAD); - return -1; + return 0; } if (NBD_REPLY_TYPE_IS_ERR (type)) { if (length < sizeof h->sbuf.sr.payload.error.error) { SET_NEXT_STATE (%.DEAD); set_error (0, "too s...

...th > name = "RECV_ERROR_TAIL"; > comment = "Receive a structured reply error tail"; > - external_events = [ NotifyRead, "" ]; > + external_events = []; > }; > > State { > default_state with > name = "RECV_OFFSET_DATA"; > comment = "Receive a structured reply offset-data header"; > - external_events = [ NotifyRead, "" ]; > + external_events = []; > }; > > State { > default_state with > name = "RECV_OFFSET_DATA_DATA"; >...

...ength); + if (rep.structured.length > 64 * 1024 * 1024) { + nbdkit_error ("structured reply length is suspiciously large: %" PRId32, + rep.structured.length); + return nbd_mark_dead (h); + } + if (rep.structured.length) { + /* Special case for OFFSET_DATA in order to read tail of chunk + directly into final buffer later on */ + len = (rep.structured.type == NBD_REPLY_TYPE_OFFSET_DATA && + rep.structured.length > sizeof offset) ? sizeof offset : + rep.structured.length; + buf = malloc (len); + if (...

...= []; }; State { default_state with name = "RECV_ERROR_TAIL"; comment = "Receive a structured reply error tail"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_OFFSET_DATA"; comment = "Receive a structured reply offset-data header"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_OFFSET_DATA_DATA"; comment = "Receive a structured reply o...

...LY_MAGIC. */ uint16_t flags; /* NBD_REPLY_FLAG_* */ uint16_t type; /* NBD_REPLY_TYPE_* */ @@ -176,15 +189,20 @@ struct structured_reply { uint32_t length; /* Length of payload which follows. */ } __attribute__((packed)); -struct structured_reply_offset_data { +struct nbd_structured_reply_offset_data { uint64_t offset; /* offset */ /* Followed by data. */ } __attribute__((packed)); -struct structured_reply_error { +struct nbd_structured_reply_offset_hole { + uint64_t offset; + uint32_t length; /* Length of hole. */...

...The most likely culprit is a server + * that replies with block status with way too many extents, but any + * oversized reply is going to take long enough to resync that it is + * not worth keeping the connection alive. + */ + if (length > MAX_REQUEST_SIZE + sizeof h->sbuf.sr.payload.offset_data) { + set_error (0, "invalid server reply length"); + SET_NEXT_STATE (%.DEAD); + return -1; + } + if (NBD_REPLY_TYPE_IS_ERR (type)) { if (length < sizeof h->sbuf.sr.payload.error) { SET_NEXT_STATE (%.DEAD); -- 2.20.1

...At any rate, the idea makes sense as a lighter-weight way for libnbd to always inform the callback about the last invocation. For nbd_aio_pread_structured and nbd_aio_block_status, the chunk/extent callback can either match server state (if the server replies replies with NBD_REPLY_FLAG_DONE on an OFFSET_DATA/BLOCK_STATUS reply, we could set VALID|FREE; if the server defers NBD_REPLY_FLAG_DONE to an NBD_REPLY_TYPE_NONE packet then the two flags will definitely not be set at the same time), or we could always defer the FREE flag until the overall command is ready to retire; for nbd_aio_FOO_callback, it m...

On Mon, Jun 17, 2019 at 07:07:55PM -0500, Eric Blake wrote: > + errno = nbd_internal_errno_of_nbd_error (error); > + set_error (errno, "server reported read failure at offset 0x%" PRIx64, > + offset); > + if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset - cmd->offset), > + 0, offset,

...- cmd->offset), + 0, offset, LIBNBD_READ_ERROR) == -1) + if (cmd->error == 0 && errno) + cmd->error = errno; + } } /* Preserve first error encountered */ @@ -304,9 +318,27 @@ return 0; REPLY.STRUCTURED_REPLY.RECV_OFFSET_DATA_DATA: + 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: SET_NEXT_STATE (%FINISH); + case 0: + length = be32toh (h->sbuf.sr.structured_reply.length); + o...

...n.chunk.callback) { + if (cmd->type == NBD_CMD_READ && + CALLBACK_IS_NOT_NULL (cmd->cb.fn.chunk)) { int scratch = error; /* Different from successful reads: inform the callback about the @@ -385,7 +386,7 @@ offset = be64toh (h->sbuf.sr.payload.offset_data.offset); assert (cmd); /* guaranteed by CHECK */ - if (cmd->cb.fn.chunk.callback) { + if (CALLBACK_IS_NOT_NULL (cmd->cb.fn.chunk)) { int error = cmd->error; if (CALL_CALLBACK (cmd->cb.fn.chunk, cmd->data + (offset - cmd->offset), @@ -446,7 +447,7 @@...