search for: send_structured_reply_read

...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 */ } @@ -433,6 +446,11 @@ send_structured_reply_read (struct connection *conn, assert (cmd == NBD_CMD_READ); + 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; reply.flags = htobe16 (NB...

...ies have been negotiated). However this prevents > + * us from sending human-readable error messages to the client, so > + * we should reconsider this in future. > */ > - if (conn->structured_replies && cmd == NBD_CMD_READ) { > - if (!error) > - return send_structured_reply_read (conn, request.handle, cmd, > - buf, count, offset); > + if (conn->structured_replies && > + (cmd == NBD_CMD_READ || cmd == NBD_CMD_BLOCK_STATUS)) { > + if (!error) { > + if (cmd == NBD_CMD_READ) > + return se...

...NBD_CMD_BLOCK_STATUS: + if (flags & NBD_CMD_FLAG_REQ_ONE) + f |= NBDKIT_FLAG_REQ_ONE; + if (backend->extents (backend, 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, +...

...ents_map, + copy_extents, &data, + foreach_flags, + 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, +...

...errno (error, false)); - r = conn->send (conn, &reply, sizeof reply, 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 (&...

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

..._CMD_READ && !error) { + 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_read (struct connection *conn, + uint64_t handle, uint16_t cmd, + const char *buf, uint32_t count, uint64_t offset) +{ + /* Once we are really using structured replies and sending data back + * in chunks, we'll be able to grab the write lock f...

...le_reply (struct connection *conn, uint32_t error) { ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); - struct simple_reply reply; + struct nbd_simple_reply reply; int r; int f = (cmd == NBD_CMD_READ && !error) ? SEND_MORE : 0; @@ -404,8 +404,8 @@ send_structured_reply_read (struct connection *conn, * 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; + struct nbd_structured_reply reply; + struct nbd_structu...

...reply for cookie %#" PRIx64 ", status %s", > rep.handle, name_of_nbd_error(be32toh (rep.error))); Teaching the nbd plugin to negotiate structured replies as the client is obviously a separate patch, I'm fine if you leave that to me :) > +static int > +send_structured_reply_read (struct connection *conn, > + uint64_t handle, uint16_t cmd, > + const char *buf, uint32_t count, uint64_t offset) > +{ > + /* Once we are really using structured replies and sending data back > + * in chunks, we'll be able...

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

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

..._CMD_READ && !error) { - 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_read (struct connection *conn, - uint64_t handle, uint16_t cmd, - const char *buf, uint32_t count, uint64_t offset) -{ - /* Once we are really using structured replies and sending data back - * in chunks, we'll be able to grab the write lock f...

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.

...uf, count, 0); if (r == -1) { nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } } @@ -394,10 +394,10 @@ send_simple_reply (struct connection *conn, } static int -send_structured_reply_read (struct connection *conn, - uint64_t handle, uint16_t cmd, +send_structured_reply_read (uint64_t handle, uint16_t cmd, const char *buf, uint32_t count, uint64_t offset) { + struct connection *conn = GET_CONN; /* Once we are really using s...

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.

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

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

This has already been pushed upstream. I am simply posting these here so we have a reference in the mailing list in case we find bugs later (as I'm sure we will - it's a complex patch series). Great thanks to Eric Blake for tireless review on this one. It also seems to have identified a few minor bugs in qemu along the way. Rich.