search for: send_structured_reply_block_status

...conn->send (conn, &offset_data, sizeof offset_data, 0); + r = conn->send (conn, &offset_data, sizeof offset_data, SEND_MORE); if (r == -1) { nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); return connection_set_status (conn, -1); @@ -573,7 +574,7 @@ send_structured_reply_block_status (struct connection *conn, reply.length = htobe32 (sizeof context_id + nr_blocks * sizeof (struct block_descriptor)); - r = conn->send (conn, &reply, sizeof reply, 0); + r = conn->send (conn, &reply, sizeof reply, SEND_MORE); if (r == -1) { nbdk...

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

...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 */ } @@ -566,6 +592,11 @@ send_structured_reply_block_status (struct connection *conn, if (blocks == NULL) return connection_set_status (conn, -1); + 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 = h...

...| 7 ++ > server/protocol.h | 15 +++ > server/protocol-handshake-newstyle.c | 81 ++++++++++++++- > server/protocol.c | 141 ++++++++++++++++++++++++--- > 4 files changed, 229 insertions(+), 15 deletions(-) > > +static int > +send_structured_reply_block_status (struct connection *conn, > + uint64_t handle, > + uint16_t cmd, uint16_t flags, > + uint32_t count, uint64_t offset, > + struct nbdkit_extents *ex...

...llowed? */ | - if (cmd == NBD_CMD_BLOCK_STATUS) { | + if (cmd == NBD_CMD_BLOCK_STATUS && 0) { | if (!conn->structured_replies) { | nbdkit_error ("invalid request: " | "%s: structured replies was not negotiated", | @@ -536,7 +536,7 @@ send_structured_reply_block_status (uint64_t cookie, | size_t i; | int r; | | - assert (conn->meta_context_base_allocation); | + // assert (conn->meta_context_base_allocation); | assert (cmd == NBD_CMD_BLOCK_STATUS); | | blocks = extents_to_block_descriptors (extents, flags, count, offset, plus this sequence:...

...> > Here's how to reproduce it. ... Thanks, that helps. > with the small patch attached to this email on top. > diff --git a/server/protocol.c b/server/protocol.c > index ac6eb2c..d406dc8 100644 > --- a/server/protocol.c > +++ b/server/protocol.c > @@ -503,6 +503,9 @@ send_structured_reply_block_status (struct connection *conn, > if (blocks == NULL) > return connection_set_status (conn, -1); > > + nr_blocks = 32; > + blocks[0].length = htobe32 (4096); > + > reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); > reply.handle = handle; > reply.flags =...

On 3/23/19 7:58 AM, Richard W.M. Jones wrote: >> A good point here is what happens with the block-status branch[1]. >> Let's see: >> >> $ ./nbdkit memory size=64M --run '/home/rjones/d/qemu/qemu-img convert $nbd /var/tmp/out' >> qemu-img: Payload too large >> nbdkit: memory.1: error: write reply: NBD_CMD_BLOCK_STATUS: Broken pipe >> >>

...kend, conn, count, offset, f, + extents, &err) == -1) + return err; + break; + default: abort (); } @@ -359,6 +399,64 @@ send_structured_reply_read (struct connection *conn, return 1; /* command processed ok */ } +static int +send_structured_reply_block_status (struct connection *conn, + uint64_t handle, + uint16_t cmd, uint16_t flags, + uint32_t count, uint64_t offset, + struct nbdkit_extents *extents) +{ + ACQUIRE...

...re. */ - blocks = calloc (req_one ? 1 : nr_extents, sizeof (struct block_descriptor)); + blocks = calloc (req_one ? 1 : nr_extents, + sizeof (struct nbd_block_descriptor)); if (blocks == NULL) { nbdkit_error ("calloc: %m"); return NULL; @@ -528,8 +529,8 @@ send_structured_reply_block_status (struct connection *conn, struct nbdkit_extents *extents) { ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); - struct structured_reply reply; - CLEANUP_FREE struct block_descriptor *blocks = NULL; + struct nbd_structured_reply reply; + CLEANUP_...

...ly (struct connection *conn, reply.magic = htobe32 (NBD_SIMPLE_REPLY_MAGIC); 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) {...

...offset, (uint64_t) count) == -1) + return errno; + + return 0; +} + static int skip_over_write_buffer (int sock, size_t count) { @@ -359,6 +490,60 @@ send_structured_reply_read (struct connection *conn, return 1; /* command processed ok */ } +static int +send_structured_reply_block_status (struct connection *conn, + uint64_t handle, + uint16_t cmd, uint16_t flags, + uint32_t count, uint64_t offset, + const struct block_descriptor *blocks, +...

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

...= NBD_CMD_BLOCK_STATUS) { > | + if (cmd == NBD_CMD_BLOCK_STATUS && 0) { > | if (!conn->structured_replies) { > | nbdkit_error ("invalid request: " > | "%s: structured replies was not negotiated", > | @@ -536,7 +536,7 @@ send_structured_reply_block_status (uint64_t cookie, > | size_t i; > | int r; > | > | - assert (conn->meta_context_base_allocation); > | + // assert (conn->meta_context_base_allocation); > | assert (cmd == NBD_CMD_BLOCK_STATUS); > | > | blocks = extents_to_block_descriptors (extents, flag...

...data: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } return 1; /* command processed ok */ @@ -522,12 +522,12 @@ extents_to_block_descriptors (struct nbdkit_extents *extents, } static int -send_structured_reply_block_status (struct connection *conn, - uint64_t handle, +send_structured_reply_block_status (uint64_t handle, uint16_t cmd, uint16_t flags, uint32_t count, uint64_t offset,...

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.

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.

...ply.magic = htobe32 (NBD_SIMPLE_REPLY_MAGIC); > 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, > +...

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

We should have only one NBD protocol file. Let's make nbdkit's version the canonical one, and use it in libnbd. Rich.

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