search for: nbd_reply_type_error

...; +#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..2f494cd 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -345,7 +345,7 @@ nbd_request (struct handle *...

...ructured 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 mess up the generated > protocol-to-lookup magic? Could do it either way really. The sed magic uses t...

...payload.error.len); + type = be16toh (h->sbuf.sr.structured_reply.type); + + length -= sizeof h->sbuf.sr.payload.error - msglen; + + /* Special case two specific errors; ignore the tail for all others */ + h->rbuf = NULL; + h->rlen = length; + switch (type) { + case NBD_REPLY_TYPE_ERROR: + if (length != 0) { + SET_NEXT_STATE (%.DEAD); + set_error (0, "error payload length too large"); + return -1; + } + break; + case NBD_REPLY_TYPE_ERROR_OFFSET: + if (length != sizeof h->sbuf.offset) { + SET_NEXT_STATE (%.DEAD); +...

...optional callback function. But a callback > function will only be reached per chunk, while we still want to fail > the overall read if the callback function was never called because the > server erroneously replied with NBD_REPLY_TYPE_NONE with no other > chunks instead of an expected NBD_REPLY_TYPE_ERROR*. For this specific > error case, the bookkeeping is much simpler - we merely track if we've > seen at least one data chunk. I guess the implicit assumption here is that count > 0, which IIRC the NBD protocol demands. It seems like we don't actually check this in nbd_internal_com...

...such checking to an optional callback function. But a callback function will only be reached per chunk, while we still want to fail the overall read if the callback function was never called because the server erroneously replied with NBD_REPLY_TYPE_NONE with no other chunks instead of an expected NBD_REPLY_TYPE_ERROR*. For this specific error case, the bookkeeping is much simpler - we merely track if we've seen at least one data chunk. --- generator/states-reply-simple.c | 1 + generator/states-reply-structured.c | 2 ++ lib/aio.c | 2 ++ lib/internal.h |...

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

...ither 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 mess up the generated protocol-to-lookup magic? > +++ b/plugins/nbd/nbd.c > @@ -345,7 +345,7 @@ nbd_request (struct han...

...i == nr_blocks - 1 ? 0 : SEND_MORE); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); return connection_set_status (conn, -1); @@ -615,7 +617,7 @@ send_structured_reply_error (struct connection *conn, reply.type = htobe16 (NBD_REPLY_TYPE_ERROR); reply.length = htobe32 (0 /* no human readable error */ + sizeof error_data); - r = conn->send (conn, &reply, sizeof reply, 0); + r = conn->send (conn, &reply, sizeof reply, SEND_MORE); if (r == -1) { nbdkit_error ("write error reply: %m"); return connec...

...T_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); #define NBD_CMD_FLUSH 3 #define NBD_CMD_TRIM 4 #define NBD_CMD_WRITE_ZEROES 6 +#define NBD_CMD_BLOCK_STATUS 7...

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

...IRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); - struct structured_reply reply; - struct structured_reply_error error_data; - int r; - - reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); - reply.handle = handle; - reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); - reply.type = htobe16 (NBD_REPLY_TYPE_ERROR); - reply.length = htobe32 (0 /* no human readable error */ + sizeof error_data); - - r = conn->send (conn, &reply, sizeof reply); - if (r == -1) { - nbdkit_error ("write error reply: %m"); - return set_status (conn, -1); - } - - /* Send the error. */ - error_data.error...

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.

...ether the client's hinted effect length was + more or less than 32 bits; but the server MUST use exactly one of + the two chunk types per negotiated metacontext ID. + All error chunk types have bit 15 set, and begin with the same *error*, *message length*, and optional *message* fields as `NBD_REPLY_TYPE_ERROR`. If nonzero, *message length* indicates @@ -1857,8 +2079,9 @@ remaining structured fields at the end. and the client MAY NOT make any assumptions about partial success. This type SHOULD NOT be used more than once in a structured reply. Valid as a reply to any request. Note that - *mes...

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

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

...error) { + struct connection *conn = GET_CONN; ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); struct nbd_structured_reply reply; struct nbd_structured_reply_error error_data; @@ -594,19 +594,19 @@ send_structured_reply_error (struct connection *conn, reply.type = htobe16 (NBD_REPLY_TYPE_ERROR); reply.length = htobe32 (0 /* no human readable error */ + sizeof error_data); - r = conn->send (conn, &reply, sizeof reply, SEND_MORE); + r = conn->send (&reply, sizeof reply, SEND_MORE); if (r == -1) { nbdkit_error ("write error reply: %m"); - return con...

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

I want to post this but mainly for discussion and early review. It's not safe for these patches to all go upstream yet (because not all filters have been checked/adjusted), but if any patches were to go upstream then probably 1 & 2 only are safe. File, VDDK, memory and data plugins all work, although I have only done minimal testing on them. The current tests, such as they are, all

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.