search for: connection_set_status

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

...e reply: %s: %m", name_of_nbd_cmd (cmd)); @@ -413,6 +418,14 @@ send_simple_reply (struct connection *conn, } } + if (conn->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...

...reply.handle = handle; reply.error = htobe32 (nbd_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, sizeo...

...num { HANDLE_OPEN = 1, /* Set if .open passed, so .close is needed */ @@ -234,29 +231,22 @@ struct connection { }; extern void handle_single_connection (int sockin, int sockout); -extern int connection_get_status (struct connection *conn) - __attribute__((__nonnull__ (1))); -extern int connection_set_status (struct connection *conn, int value) - __attribute__((__nonnull__ (1))); +extern int connection_get_status (void); +extern int connection_set_status (int value); /* protocol-handshake.c */ -extern int protocol_handshake (struct connection *conn) - __attribute__((__nonnull__ (1))); -extern int...

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.

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

...r/plugins.c | 2 +- server/protocol.c | 4 ++-- server/sockets.c | 4 ++-- 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/server/connections.c b/server/connections.c index c55d381..95d8296 100644 --- a/server/connections.c +++ b/server/connections.c @@ -97,7 +97,7 @@ connection_set_status (struct connection *conn, int value) assert (conn->status_pipe[1] >= 0); if (write (conn->status_pipe[1], &c, 1) != 1 && errno != EAGAIN) - nbdkit_debug ("failed to notify pipe-to-self: %m"); + debug ("failed to notify pipe-to-self:...

...handle (struct connection *conn, __attribute__((__nonnull__ (1 /* not 3 */))); extern void *connection_get_handle (struct connection *conn, size_t i) __attribute__((__nonnull__ (1))); +extern int connection_get_status (struct connection *conn) + __attribute__((__nonnull__ (1))); +extern int connection_set_status (struct connection *conn, int value) + __attribute__((__nonnull__ (1))); + +/* protocol-handshake.c */ +extern int protocol_handshake (struct connection *conn) + __attribute__((__nonnull__ (1))); +extern int protocol_compute_eflags (struct connection *conn, uint16_t *flags) + __attribute__((__no...

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.

...BLOCK_STATUS); + reply.length = htobe32 (sizeof context_id + + nr_extents * sizeof (struct block_descriptor)); + + r = conn->send (conn, &reply, sizeof reply); + if (r == -1) { + nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); + return connection_set_status (conn, -1); + } + + /* Send the base:allocation context ID. */ + context_id = htobe32 (base_allocation_id); + r = conn->send (conn, &context_id, sizeof context_id); + if (r == -1) { + nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); + return connection_set_st...

..._BLOCK_STATUS); + reply.length = htobe32 (sizeof context_id + + nr_blocks * sizeof (struct block_descriptor)); + + r = conn->send (conn, &reply, sizeof reply); + if (r == -1) { + nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); + return connection_set_status (conn, -1); + } + + /* Send the base:allocation context ID. */ + context_id = htobe32 (base_allocation_id); + r = conn->send (conn, &context_id, sizeof context_id); + if (r == -1) { + nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); + return connection_set_st...

...conn->status; - if (conn->nworkers) - pthread_mutex_unlock (&conn->status_lock); + if (conn->nworkers && + pthread_mutex_unlock (&conn->status_lock)) + abort (); return r; } @@ -105,12 +107,14 @@ connection_get_status (struct connection *conn) int connection_set_status (struct connection *conn, int value) { - if (conn->nworkers) - pthread_mutex_lock (&conn->status_lock); + if (conn->nworkers && + pthread_mutex_lock (&conn->status_lock)) + abort (); if (value < conn->status) conn->status = value; - if (co...

...fset); > + > + bd.length = htobe32 (e.length); > + bd.status_flags = htobe32 (e.type & 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); > + } > + } Where does the list terminate once you send the final extent that overlaps the end of the original request? (If you implement my idea of tracking a maximum offset in nbdkit_extents in patch 1, this may be picked up automatically) > @@ -498,15 +605,23 @@ prot...

...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 = htobe16 (NBD_REPLY_FLAG_DONE); > @@ -526,7 +529,7 @@ send_structured_reply_block_status (s...

...truct connection { connection_close_function close; }; +static inline struct handle * +get_handle (struct connection *conn, int i) +{ + return &conn->handles[i]; +} + extern void handle_single_connection (int sockin, int sockout); extern int connection_get_status (void); extern int connection_set_status (int value); diff --git a/server/backend.c b/server/backend.c index 616c24d8..9669ada1 100644 --- a/server/backend.c +++ b/server/backend.c @@ -154,7 +154,7 @@ int backend_open (struct backend *b, int readonly) { GET_CONN; - struct b_conn_handle *h = &conn->handles[b->i]; + struct...

This fixes the long-standing crash on close when nbdkit exits. I did try first to fix threads so we're using a proper thread pool, but that's difficult to implement. So this does the minimal change needed to fix the crash instead. There are still two segfaults that happen during running the test suite. One is deliberately caused (tests/test-captive.sh). The other appears to be an

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

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