search for: send_structured_reply_error

...GIC); reply.handle = handle; - reply.error = htobe32 (nbd_errno (error)); + reply.error = htobe32 (nbd_errno (error, false)); r = conn->send (conn, &reply, sizeof reply); if (r == -1) { @@ -573,7 +575,8 @@ send_structured_reply_block_status (struct connection *conn, static int send_structured_reply_error (struct connection *conn, - uint64_t handle, uint16_t cmd, uint32_t error) + uint64_t handle, uint16_t cmd, uint16_t flags, + uint32_t error) { ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); struc...

....error = htobe32 (nbd_errno (error)); > + reply.error = htobe32 (nbd_errno (error, false)); > > r = conn->send (conn, &reply, sizeof reply); > if (r == -1) { > @@ -573,7 +575,8 @@ send_structured_reply_block_status (struct connection *conn, > > static int > send_structured_reply_error (struct connection *conn, > - uint64_t handle, uint16_t cmd, uint32_t error) > + uint64_t handle, uint16_t cmd, uint16_t flags, > + uint32_t error) > { > ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn-...

...onn->cork) { + r = conn->cork (conn, false); + if (r == -1) { + nbdkit_error ("write uncork: %s: %m", name_of_nbd_cmd (cmd)); + return connection_set_status (conn, -1); + } + } + return 1; /* command processed ok */ } @@ -609,6 +648,11 @@ send_structured_reply_error (struct connection *conn, struct structured_reply_error error_data; int r; + if (conn->cork) { + r = conn->cork (conn, true); + assert (r == 0); /* For now, only uncorking can fail */ + } + reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); reply.handle = handle; repl...

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

...amp; 3); + + r = conn->send (conn, &bd, sizeof bd); + if (r == -1) { + nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); + return connection_set_status (conn, -1); + } + } + + return 1; /* command processed ok */ +} + static int send_structured_reply_error (struct connection *conn, uint64_t handle, uint16_t cmd, uint32_t error) @@ -402,6 +500,7 @@ protocol_recv_request_send_reply (struct connection *conn) uint32_t magic, count, error = 0; uint64_t offset; CLEANUP_FREE char *buf = NULL; + CLEANUP_EXTENTS_FREE st...

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

..._flags); + + r = conn->send (conn, &bd, sizeof bd); + if (r == -1) { + nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); + return connection_set_status (conn, -1); + } + } + + return 1; /* command processed ok */ +} + static int send_structured_reply_error (struct connection *conn, uint64_t handle, uint16_t cmd, uint32_t error) @@ -402,6 +587,9 @@ protocol_recv_request_send_reply (struct connection *conn) uint32_t magic, count, error = 0; uint64_t offset; CLEANUP_FREE char *buf = NULL; + CLEANUP_EXTENTS_FREE st...

...red_reply_block_status (conn, request.handle, > + cmd, flags, > + count, offset, > + extents); > + } > else > return send_structured_reply_error (conn, request.handle, cmd, error); Ah, so you are already sending a structured error, even though the protocol didn't require it (which is the qemu bug I just sent a patch for today). Technically, the protocol requires a structured error for NBD_CMD_READ, but permits a simple error for NBD_CM...

...)); + return set_status (conn, -1); + } + + r = conn->send (conn, buf, count); + if (r == -1) { + nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); + return set_status (conn, -1); + } + + return 1; /* command processed ok */ +} + +static int +send_structured_reply_error (struct connection *conn, + uint64_t handle, uint16_t cmd, uint32_t error) +{ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); + struct structured_reply reply; + struct structured_reply_error error_data; + int r; + + reply.magic = htobe32 (NBD_STRUCTURED...

Slightly RFC, as I need more time to investigate why Unix sockets appeared to degrade with this patch. But as TCP sockets (over loopback to localhost) and TLS sessions (regardless of underlying Unix or TCP) both showed improvements, this looks like a worthwhile series. Eric Blake (2): server: Add support for corking server: Cork around grouped transmission send()s server/internal.h | 3

...cks[i], 0); + r = conn->send (conn, &blocks[i], sizeof blocks[i], + 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) {...

...if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } } @@ -579,10 +579,10 @@ send_structured_reply_block_status (struct connection *conn, } static int -send_structured_reply_error (struct connection *conn, - uint64_t handle, uint16_t cmd, uint16_t flags, +send_structured_reply_error (uint64_t handle, uint16_t cmd, uint16_t flags, uint32_t error) { + struct connection *conn = GET_CONN; ACQUIRE_LOCK_FOR_CURRENT_SCO...

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.

...)); - return set_status (conn, -1); - } - - r = conn->send (conn, buf, count); - if (r == -1) { - nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); - return set_status (conn, -1); - } - - return 1; /* command processed ok */ -} - -static int -send_structured_reply_error (struct connection *conn, - uint64_t handle, uint16_t cmd, uint32_t error) -{ - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); - struct structured_reply reply; - struct structured_reply_error error_data; - int r; - - reply.magic = htobe32 (NBD_STRUCTURED...

...ply.length = htobe32 (sizeof context_id + - nr_blocks * sizeof (struct block_descriptor)); + nr_blocks * sizeof (struct nbd_block_descriptor)); r = conn->send (conn, &reply, sizeof reply, SEND_MORE); if (r == -1) { @@ -583,8 +584,8 @@ send_structured_reply_error (struct connection *conn, uint32_t error) { ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); - struct structured_reply reply; - struct structured_reply_error error_data; + struct nbd_structured_reply reply; + struct nbd_structured_reply_error error_da...

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.

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.

...connection *conn, reply.magic = htobe32 (NBD_SIMPLE_REPLY_MAGIC); reply.handle = handle; - reply.error = htobe32 (nbd_errno (error, false)); + reply.error = htobe32 (nbd_errno (error, flags)); r = conn->send (conn, &reply, sizeof reply, f); if (r == -1) { @@ -640,7 +667,7 @@ send_structured_reply_error (struct connection *conn, } /* Send the error. */ - error_data.error = htobe32 (nbd_errno (error, flags & NBD_CMD_FLAG_DF)); + error_data.error = htobe32 (nbd_errno (error, flags)); error_data.len = htobe16 (0); r = conn->send (conn, &error_data, sizeof error_data, 0);...

Not sure what version we're up to, but this reimplements extents using the new simpler structure described in this thread: https://www.redhat.com/archives/libguestfs/2019-March/msg00077.html I also fixed most of the things that Eric pointed out in the previous review, although I need to go back over his replies and check I've got everything. This needs a bit more testing. However the

I'm not sure exactly which version we're up to, but let's say it's version 4. I'm a lot happier with this version: - all filters have been reviewed and changed where I think that's necessary - can_extents is properly defined and implemented now - NBD protocol is followed - I believe it addresses all previous review points where possible The "only" thing