search for: nbd_reply_type_offset_data

Rather than requiring all servers and clients to have a 32-bit limit on maximum NBD_CMD_READ/WRITE sizes, we can choose to standardize support for a 64-bit single I/O transaction now. NBD_REPLY_TYPE_OFFSET_DATA can already handle a large reply, but NBD_REPLY_TYPE_OFFSET_HOLE needs a 64-bit counterpart. By standardizing this, all clients must be prepared to support both types of hole type replies, even though most server implementations of extended replies are likely to only send one hole type. --- As t...

...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); ... > + case NBD_REPLY_TYPE_OFFSET_DATA: > + if (rep.structured.length <= sizeof offset) { > + nbdkit_error (&qu...

..._STRUCTURED_REPLY_MAGIC 0x668e33ef + +/* Structured reply flags. */ +extern const char *name_of_nbd_reply_flag (int); +#define NBD_REPLY_FLAG_DONE (1<<0) + +/* 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 /* NBD commands. */ extern const char *name_of_nbd_cmd (int); diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 674f4a4.....

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

...ertise it because 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...

...is how NBD_CMD_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...

v2 was here: https://lists.debian.org/nbd/2022/11/msg00030.html The bulk of the changes since then are: - forbid NBD_OPT_EXPORT_NAME once extended headers are negotiated (Wouter) - consistently use 'maximum payload', rather than a haphazard mix of 'maximum block payload' (Vladimir) At this point, I want to make sure we are happy with the spec before re-posting patches for

...till need tweaking. > Or we may even want to let the user register a set of callbacks, where a > different callback is invoked for different chunk types: > > set = nbd_callback_set_create(nbd, NBD_CMD_READ, opaque, default_cb, > default_error_cb); > nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_DATA, opaque, cb); > nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_HOLE, opaque, cb); > nbd_pread_callback(nbd, buf, count, offset, set, flags); This would also work, with the caveat that the Python bindings will currently crash if you try to do this. I'm trying to fix that now. >...

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

On Mon, May 27, 2019 at 09:01:01PM -0500, Eric Blake wrote: > diff --git a/lib/rw.c b/lib/rw.c > index feaf468..343c340 100644 > --- a/lib/rw.c > +++ b/lib/rw.c > @@ -234,11 +234,17 @@ int64_t > nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf, > size_t count, uint64_t offset, uint32_t flags) > { > - if (flags != 0) { > + if ((flags

...y 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 (!buf) { + nbdkit_error ("reading structured reply payload: %m"); + return nbd_mark_dead (h); + } + if (read_fu...

...tructured.c +++ b/generator/states-reply-structured.c @@ -103,7 +103,7 @@ set_error (0, "NBD_REPLY_FLAG_DONE must be set in NBD_REPLY_TYPE_NONE"); return -1; } - SET_NEXT_STATE (%^FINISH_COMMAND); + SET_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: - flag...

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

...or error. The signature may still need tweaking. Or we may even want to let the user register a set of callbacks, where a different callback is invoked for different chunk types: set = nbd_callback_set_create(nbd, NBD_CMD_READ, opaque, default_cb, default_error_cb); nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_DATA, opaque, cb); nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_HOLE, opaque, cb); nbd_pread_callback(nbd, buf, count, offset, set, flags); The idea of registering a set of callbacks to handle a particular integer command id may work well for other extensions as well; particularly if we encount...

...heir counterparts in structured replies (including the +use of `NBD_REPLY_FLAG_DONE` to end a sequence of one or more chunks +comprising the overall reply). The extended reply has an additional +field *offset* which MUST match the *offset* of the client's request +(even in reply types such as `NBD_REPLY_TYPE_OFFSET_DATA` where the +payload itself contains a different offset representing the middle of +the buffer). + +The other difference between extended replies and structured replies +is that the *length* field is 64 bits to match the layout of the +extended request header. However, note that while the request f...

...0); + r = conn->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...

...NBD_REPLY_FLAG_DONE (1<<0) #define NBD_REPLY_TYPE_ERR(val) ((1<<15) | (val)) #define NBD_REPLY_TYPE_IS_ERR(val) (!!((val) & (1<<15))) /* 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 @@ -213,7 +206,6 @@ extern const char *name_of_nbd_reply_type (int); #define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_TYPE_ERR (2) /* NBD commands. */ -extern const char *name_of_nbd_cmd (int); #define NBD_CMD_READ 0 #define NBD_CMD_WRITE...

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

.../* block type (hole etc) */ +} __attribute__((packed)); + /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO instead of this. */ @@ -187,7 +200,7 @@ 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_BLOCK_STATUS 5 #define NBD_REPLY_TYPE_ERROR ((1<<15) + 1) #define NBD_REPLY_TYPE_ERROR_OFFSET ((1<<15) + 2) @@ -199,10 +212,12 @@ extern const char *name_of_nbd_cmd (int);...

...rge: %" PRId32, - rep.structured.length); - return nbdplug_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; - payload = malloc (len); - if (!payload) { - nbdkit_error ("reading structured reply payload: %m"); - return nbdplug_mark_dead (h); - } -...