June 2019 - Libguestfs - libguestfs List Archives

[libnbd PATCH v2 0/5] nbd_pread_structured
by Eric Blake 25 Jun '19

25 Jun '19

Since v1: - rebase to applied patches - split out support for Int in callbacks - sort of test that callbacks work in OCaml (see comment in patch 5) - rename API to nbd_pread_structured - expose error as explicit parameter to callback Eric Blake (5): generator: Allow Int in callbacks states: Wire in a read callback states: Add nbd_pread_structured API states: Add tests for nbd_pread_structured states: Add DF flag support for pread .gitignore | 1 + generator/generator | 140 +++++++++++++++++++-- generator/states-reply-simple.c | 15 ++- generator/states-reply-structured.c | 44 ++++++- interop/Makefile.am | 11 +- interop/structured-read.c | 183 ++++++++++++++++++++++++++++ interop/structured-read.sh | 57 +++++++++ lib/flags.c | 11 ++ lib/internal.h | 4 +- lib/nbd-protocol.h | 1 + lib/protocol.c | 1 + lib/rw.c | 42 +++++++ python/t/405-pread-structured.py | 43 +++++++ tests/oldstyle.c | 78 +++++++++++- 14 files changed, 611 insertions(+), 20 deletions(-) create mode 100644 interop/structured-read.c create mode 100755 interop/structured-read.sh create mode 100644 python/t/405-pread-structured.py -- 2.20.1

2 11

[libnbd PATCH] docs: Improve nbd_aio_get_direction documentation
by Eric Blake 25 Jun '19

25 Jun '19

Mention things to remember for avoiding the deadlock of polling for a POLLIN from a server that has no replies to send. Perhaps we could split the READY state into two - one for when there are no commands in flight (and get_direction returns 0 to state that polling is pointless, although a multi-threaded reader can still poll for POLLIN), and the other when there ARE commands in flight. Such a separation would also make it easier to know when nbd_aio_disconnect is not going to strand an in-flight command's response. But I'm not sure it is worth the effort. --- I'm pushing this one now, but sending the email because of the question it raises on whether we want the user to be able to distinguish a difference between any vs. no in-flight commands. generator/generator | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/generator/generator b/generator/generator index a289741..1d4db23 100755 --- a/generator/generator +++ b/generator/generator @@ -1719,6 +1719,18 @@ We are expected next to read from the server. If using L<poll(2)> you would set C<events = POLLIN>. If C<revents> returns C<POLLIN> you would then call C<nbd_aio_notify_read>. +Note that once libnbd reaches C<nbd_aio_is_ready>, this direction is +returned even before a command is issued via C<nbd_aio_pwrite> and +friends. In a single-threaded use of libnbd, it is not worth polling +until after issuing a command, as otherwise the server will never wake +up the poll. In a multi-threaded scenario, you can have one thread +begin a polling loop prior to any commands, but any other thread that +issues a command will need a way to kick the polling thread out of +poll in case issuing the command changes the needed polling +direction. Possible ways to do this include polling for activity on a +pipe-to-self, or using L<pthread_kill(3)> to send a signal that is +masked except during L<ppoll(2)>. + =item C<LIBNBD_AIO_DIRECTION_WRITE> = 2 We are expected next to write to the server. If using L<poll(2)> @@ -1727,13 +1739,12 @@ you would then call C<nbd_aio_notify_write>. =item C<LIBNBD_AIO_DIRECTION_BOTH> = 3 -We are expected next to either read or write to the server. -If using L<poll(2)> you would set C<events = POLLIN|POLLOUT>. -If one of C<POLLIN> or C<POLLOUT> is returned, then see above. -However note that you shouldn't call C<nbd_aio_notify_read> -and C<nbd_aio_notify_write> because calling the first one will -change the state of the connection, possibly invalidating the second -notification. +We are expected next to either read or write to the server. If using +L<poll(2)> you would set C<events = POLLIN|POLLOUT>. If only one of +C<POLLIN> or C<POLLOUT> is returned, then see above. However, if both +are returned, it is better to call only C<nbd_aio_notify_read>, as +processing the server's reply may change the state of the connection +and invalidate the need to write more commands. =back"; }; -- 2.20.1

2 2

[libnbd PATCH 0/8] Add nbd_pread_callback
by Eric Blake 21 Jun '19

21 Jun '19

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 an error at the same offset, that it does not send overlapping data, and that it does not report success unless all of the range is covered). [1] https://www.redhat.com/archives/libguestfs/2019-May/msg00209.html Eric Blake (8): states: Add state for structured reply completion states: Consolidate search for current reply's command pread: Reject server SR read response with no data chunks states: Prepare for read callback states: Wire in a read callback states: Add nbd_pread_callback API states: Add tests for nbd_pread_callback states: Add DF flag support for pread .gitignore | 1 + generator/generator | 140 +++++++++++++++++++-- generator/states-reply-simple.c | 31 +++-- generator/states-reply-structured.c | 147 +++++++++++----------- generator/states-reply.c | 31 ++++- interop/Makefile.am | 11 +- interop/structured-read.c | 182 ++++++++++++++++++++++++++++ interop/structured-read.sh | 57 +++++++++ lib/aio.c | 2 + lib/flags.c | 11 ++ lib/internal.h | 22 +++- lib/nbd-protocol.h | 1 + lib/protocol.c | 1 + lib/rw.c | 51 +++++++- python/t/405-pread-callback.py | 42 +++++++ tests/oldstyle.c | 74 ++++++++++- 16 files changed, 686 insertions(+), 118 deletions(-) create mode 100644 interop/structured-read.c create mode 100755 interop/structured-read.sh create mode 100644 python/t/405-pread-callback.py -- 2.20.1

2 21

[PATCH libnbd v3] lib: Atomically update h->state when leaving the locked region.
by Richard W.M. Jones 20 Jun '19

20 Jun '19

v1 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00055 v2 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00067 v3: - Fix atomicly -> atomically in commit message. - Fix a comment. - Fix TOCTTOU: There is now an inline function generated called <name>_is_permitted_state, and this is called twice, first outside the lock for the quick rejection check, and again inside the lock in case the state changed. I think an improvement would be to _drop_ the first check (outside the lock). This is an error check, so not performance critical since we are definitely going to acquire the lock on the hot path. Rich.

2 4

[nbdkit PATCH] extents: Cap maximum reply length
by Eric Blake 19 Jun '19

19 Jun '19

A devious plugin can cause an 8-fold increase in the reply size in relation to the request size, by alternating status every byte of the request. In the worst case, this can cause the client to reject our response and kill the connection because the response is too large. What's more, it consumes a lot of memory to track that many extents. Let's put an upper bound on the maximum number of extents we are willing to return (1M extents is an 8M reply chunk). Pre-patch, this can be used to demonstrate the problem: $ nbdkit -f sh - <<\EOF #!/bin/bash size=$((9*1024*1024)) case $1 in get_size) echo $size ;; pread) dd iflag=skip_bytes,count_bytes skip=$4 count=$3 if=/dev/zero || exit 1 ;; can_extents) ;; extents) # Unrealistic in real life, but works to provoke the bug. For a full 9M # query, this produces ~100M for nbdkit to parse, and in turn tries to # produce a 72M reply chunk if we don't cap extents. for ((i=$4;i<$4+$3;i++)); do echo $i 1 $((i&1)) done ;; *) exit 2 ;; esac EOF $ ~/libnbd/run sh/nbdsh ... nbd> h.connect_tcp("localhost","10809") nbd> def f(data,metacontext,offset,e): ... print ("entries:%d" % len(e)) ... nbd> h.block_status(9*1024*1024,0,0,f) Traceback (most recent call last): File "/usr/lib64/python3.7/code.py", line 90, in runcode exec(code, self.locals) File "<console>", line 1, in <module> File "/home/eblake/libnbd/python/nbd.py", line 577, in block_status return libnbdmod.block_status (self._o, count, offset, data, extent, flags) RuntimeError: nbd_block_status: invalid server reply length Signed-off-by: Eric Blake <eblake(a)redhat.com> --- server/extents.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/server/extents.c b/server/extents.c index c4224916..0ebd77b1 100644 --- a/server/extents.c +++ b/server/extents.c @@ -45,6 +45,12 @@ #include "internal.h" +/* Cap nr_extents to avoid sending over-large replies to the client, + * and to avoid a plugin with frequent alternations consuming too much + * memory. + */ +#define MAX_EXTENTS (1 * 1024 * 1024) + struct nbdkit_extents { struct nbdkit_extent *extents; size_t nr_extents, allocated; @@ -163,8 +169,8 @@ nbdkit_add_extent (struct nbdkit_extents *exts, if (length == 0) return 0; - /* Ignore extents beyond the end of the range. */ - if (offset >= exts->end) + /* Ignore extents beyond the end of the range, or if list is full. */ + if (offset >= exts->end || exts->nr_extents == MAX_EXTENTS) return 0; /* Shorten extents that overlap the end of the range. */ -- 2.20.1

3 2

[nbdkit PATCH] Experiment: nbd: Use ppoll() instead of pipe-to-self
by Eric Blake 18 Jun '19

18 Jun '19

It is necessary to kick the state machine any time another thread blocks due to write() encountering a full buffer, to get the reader thread to learn that it must now poll on POLLOUT rather than just POLLIN. POSIX only provides two ways to do this: either poll() on a second fd (our pipe-to-self trick), or interrupt the polling with EINTR due to delivery of a signal. So, I played with the latter approach by running the reader thread with SIGUSR1 blocked except during the poll, where other threads change their kick action from writing into the pipe-to-self to instead calling pthread_kill() to force EINTR. Unfortunately, using only poll() for this is racy; to fix the race, we either have to resort to POSIX pselect() (which is horribly klunky and does not scale well to large fd values if we have lots of parallel clients) or use the non-POSIX ppoll() (these days, BSD and Linux systems all have it; if someone complains about failure to compile, we can #ifdef the code and fall back to pipe-to-self on the systems lacking ppoll()). Also unfortunately, the timing does not favor this approach. Repeating the setup for commit e897ed70, I see the following changes: Pre-patch, the runs averaged 17.232s, 9.76E+09 bits/s Post-patch, the runs averaged 17.993s, 9.34E+09 bits/s I'm posting this in case something changes in the future (for example, I envision that libnbd might add a callback function for performing a kick such that we signal the reader thread ONLY when we actually blocked on write(), rather than after every single request sent to the server), but will not be applying it for now. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- plugins/nbd/nbd.c | 86 +++++++++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 941e4cb7..02b8119a 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -45,6 +45,7 @@ #include <pthread.h> #include <semaphore.h> #include <poll.h> +#include <signal.h> #include <libnbd.h> @@ -67,7 +68,6 @@ struct handle { /* These fields are read-only once initialized */ struct nbd_handle *nbd; int fd; /* Cache of nbd_aio_get_fd */ - int fds[2]; /* Pipe for kicking the reader thread */ bool readonly; pthread_t reader; @@ -105,6 +105,15 @@ static char *tls_psk; static struct handle *nbdplug_open_handle (int readonly); static void nbdplug_close_handle (struct handle *h); +/* Original signal mask, with SIGUSR1 unblocked */ +static sigset_t origmask; + +/* No-op signal handler for interrupting ppoll on SIGUSR1 */ +static void +nbdplug_sigusr1 (int sig) +{ +} + static void nbdplug_unload (void) { @@ -195,10 +204,30 @@ nbdplug_config (const char *key, const char *value) return 0; } -/* Check the user passed exactly one socket description. */ +/* Finalize the configuration. */ static int nbdplug_config_complete (void) { + struct sigaction act = { + .sa_handler = nbdplug_sigusr1, + .sa_flags = SA_RESTART, + }; + + /* Register a no-op SIGUSR1 handler for use with ppoll */ + if ((errno = pthread_sigmask (SIG_SETMASK, NULL, &origmask))) { + nbdkit_error ("pthread_sigmask: %m"); + return -1; + } + if (sigismember (&origmask, SIGUSR1)) { + nbdkit_error ("SIGUSR1 should not be blocked"); + return -1; + } + if (sigaction (SIGUSR1, &act, NULL) == -1) { + nbdkit_error ("sigaction: %m"); + return -1; + } + + /* Check the user passed exactly one socket description. */ if (sockname) { struct sockaddr_un sock; @@ -306,37 +335,27 @@ nbdplug_reader (void *handle) int r; while (!nbd_aio_is_dead (h->nbd) && !nbd_aio_is_closed (h->nbd)) { - struct pollfd fds[2] = { - [0].fd = h->fd, - [1].fd = h->fds[0], - [1].events = POLLIN, - }; + struct pollfd fd; struct transaction *trans, **prev; int dir; - char c; + fd.fd = h->fd; dir = nbd_aio_get_direction (h->nbd); nbdkit_debug ("polling, dir=%d", dir); if (dir & LIBNBD_AIO_DIRECTION_READ) - fds[0].events |= POLLIN; + fd.events |= POLLIN; if (dir & LIBNBD_AIO_DIRECTION_WRITE) - fds[0].events |= POLLOUT; - if (poll (fds, 2, -1) == -1) { - nbdkit_error ("poll: %m"); + fd.events |= POLLOUT; + if (ppoll (&fd, 1, NULL, &origmask) == -1 && errno != EINTR) { + nbdkit_error ("ppoll: %m"); break; } - if (dir & LIBNBD_AIO_DIRECTION_READ && fds[0].revents & POLLIN) + if (dir & LIBNBD_AIO_DIRECTION_READ && fd.revents & POLLIN) nbd_aio_notify_read (h->nbd); - else if (dir & LIBNBD_AIO_DIRECTION_WRITE && fds[0].revents & POLLOUT) + else if (dir & LIBNBD_AIO_DIRECTION_WRITE && fd.revents & POLLOUT) nbd_aio_notify_write (h->nbd); - /* Check if we were kicked because a command was started */ - if (fds[1].revents & POLLIN && read (h->fds[0], &c, 1) != 1) { - nbdkit_error ("failed to read pipe: %m"); - break; - } - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&h->trans_lock); trans = h->trans; prev = &h->trans; @@ -400,7 +419,6 @@ static struct transaction * nbdplug_register (struct handle *h, int64_t cookie) { struct transaction *trans; - char c = 0; if (cookie == -1) { nbdkit_error ("command failed: %s", nbd_get_error ()); @@ -417,8 +435,8 @@ nbdplug_register (struct handle *h, int64_t cookie) /* While locked, kick the reader thread and add our transaction */ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&h->trans_lock); - if (write (h->fds[1], &c, 1) != 1) { - nbdkit_error ("write to pipe: %m"); + if ((errno = pthread_kill (h->reader, SIGUSR1))) { + nbdkit_error ("pthread_kill: %m"); free (trans); return NULL; } @@ -466,17 +484,13 @@ nbdplug_open_handle (int readonly) struct handle *h; int r; unsigned long retries = retry; + sigset_t blocked; h = calloc (1, sizeof *h); if (h == NULL) { nbdkit_error ("malloc: %m"); return NULL; } - if (pipe (h->fds)) { - nbdkit_error ("pipe: %m"); - free (h); - return NULL; - } retry: h->fd = -1; @@ -520,22 +534,30 @@ nbdplug_open_handle (int readonly) if (readonly) h->readonly = true; - /* Spawn a dedicated reader thread */ + /* Spawn a dedicated reader thread with SIGUSR1 blocked */ if ((errno = pthread_mutex_init (&h->trans_lock, NULL))) { nbdkit_error ("failed to initialize transaction mutex: %m"); goto err; } + if (sigemptyset (&blocked) == -1 || + sigaddset (&blocked, SIGUSR1) == -1) + assert (false); + if ((errno = pthread_sigmask (SIG_BLOCK, &blocked, NULL))) { + nbdkit_error ("failed to set signal mask: %m"); + pthread_mutex_destroy (&h->trans_lock); + goto err; + } if ((errno = pthread_create (&h->reader, NULL, nbdplug_reader, h))) { nbdkit_error ("failed to initialize reader thread: %m"); pthread_mutex_destroy (&h->trans_lock); goto err; } + if (pthread_sigmask (SIG_SETMASK, &origmask, NULL)) + assert (false); return h; err: - close (h->fds[0]); - close (h->fds[1]); nbdkit_error ("failure while creating nbd handle: %s", nbd_get_error ()); if (h->nbd) nbd_close (h->nbd); @@ -560,8 +582,6 @@ nbdplug_close_handle (struct handle *h) nbdkit_debug ("failed to clean up handle: %s", nbd_get_error ()); if ((errno = pthread_join (h->reader, NULL))) nbdkit_debug ("failed to join reader thread: %m"); - close (h->fds[0]); - close (h->fds[1]); nbd_close (h->nbd); pthread_mutex_destroy (&h->trans_lock); free (h); -- 2.20.1

1 0

[libnbd PATCH] build: Fix OCaml build on Fedora 29
by Eric Blake 18 Jun '19

18 Jun '19

Once ocamlfind is installed, 'make' failed for me with: ocamlfind ocamlc -g -annot -safe-string -warn-error CDEFLMPSUVYZX+52-3 -ccopt '-gdwarf' -package unix -c NBD.ml -o NBD.cmo File "NBD.ml", line 1: Error: Could not find the .cmi file for interface NBD.mli. make[2]: *** [Makefile:823: NBD.cmo] Error 2 I'm not positive that this is the perfect fix, but it at least got the build working for me, and copies from idioms in nbdkit/plugins/ocaml/Makefile.am. --- ocaml/Makefile.am | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am index 0d876bb..557151c 100644 --- a/ocaml/Makefile.am +++ b/ocaml/Makefile.am @@ -32,6 +32,11 @@ if HAVE_OCAML OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)' OCAMLPACKAGES = -package unix +NBD.cmi: NBD.mli + $(OCAMLC) -c $< -o $@ + +NBD.cmo: NBD.cmi + noinst_DATA = mlnbd.cma META if HAVE_OCAMLOPT noinst_DATA += mlnbd.cmxa -- 2.20.1

2 2

[libnbd PATCH 0/7] state machine refactoring
by Eric Blake 17 Jun '19

17 Jun '19

I'm still playing with ideas on how to split rstate from wstate (so that we can send a request without waiting for POLLIN to complete a pending reply), but this is some preliminary refactoring I found useful. I also fixed a couple of bugs while in the area (already pushed). There's a question of whether we want nbd_handle to be nearly 5k, or if we should instead keep it small and add one more indirection where sbuf is malloc'd; I'm not sure that it matters much (outside of libnbd, nbd_handle is an opaque type always referenced as a pointer). Eric Blake (7): states: Log structured error messages, if any states: Refactor SET_META_CONTEXT reply parsing states: Allow large SET_CONTEXT_NAME replies states: Rewrite NBD_REP_INFO parsing states: Factor out NBD_REP payload prep states: Give up on oversized reply length states: Capture NBD_REP_ERR message generator/generator | 19 ++-- generator/states-newstyle-opt-go.c | 71 ++++++------- .../states-newstyle-opt-set-meta-context.c | 99 +++++++------------ generator/states-newstyle-opt-starttls.c | 39 ++------ .../states-newstyle-opt-structured-reply.c | 34 +++---- generator/states-newstyle.c | 90 +++++++++++++++++ generator/states-reply-structured.c | 23 ++++- lib/internal.h | 4 +- 8 files changed, 207 insertions(+), 172 deletions(-) -- 2.20.1

2 11

[libnbd PATCH] states: Validate error message size
by Eric Blake 14 Jun '19

14 Jun '19

If the server passes us a malformed error reply type with a message length longer than the overall structured reply, we would blindly obey that size and get out of sync with the server (perhaps even hanging on a read for data that will never come). Broken since its introduction in commit 28952eda. Fix it by parsing the tail of an error separate from the message, which also lets us add other sanity checking, and makes it easier if a future patch wants to capture instead of ignore the server's message. --- I'm looking at installing a few other length sanity checks as well: we ought to require that the server's answers to NBD_OPT and NBD_CMD don't exceed MAX_REQUEST_SIZE (other than a bit of fudge to allow payload + header when handling NBD_CMD_READ for 64M). Also, the NBD spec says strings must not exceed 4k (we need to enforce this on the export name we send to the server, and should also check any string field the server replies to us). generator/generator | 7 ++++ generator/states-reply-structured.c | 64 ++++++++++++++++++++++++++++- lib/internal.h | 1 + 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/generator/generator b/generator/generator index 3b0ca82..deb77f0 100755 --- a/generator/generator +++ b/generator/generator @@ -771,6 +771,13 @@ and structured_reply_state_machine = [ external_events = [ NotifyRead, "" ]; }; + State { + default_state with + name = "RECV_ERROR_TAIL"; + comment = "Receive a structured reply error tail"; + external_events = [ NotifyRead, "" ]; + }; + State { default_state with name = "RECV_OFFSET_DATA"; diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 5791360..6d58742 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -149,21 +149,69 @@ } REPLY.STRUCTURED_REPLY.RECV_ERROR: + uint32_t length, msglen; + switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: + length = be32toh (h->sbuf.sr.structured_reply.length); + msglen = be16toh (h->sbuf.sr.payload.error.len); + if (msglen > length - sizeof h->sbuf.sr.payload.error) { + SET_NEXT_STATE (%.DEAD); + set_error (0, "error message length too large"); + return -1; + } + /* We skip the human readable error for now. XXX */ h->rbuf = NULL; - h->rlen = be16toh (h->sbuf.sr.payload.error.len); + h->rlen = msglen; SET_NEXT_STATE (%RECV_ERROR_MESSAGE); } return 0; REPLY.STRUCTURED_REPLY.RECV_ERROR_MESSAGE: + uint32_t length, msglen; + uint16_t type; + + switch (recv_into_rbuf (h)) { + case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 0: + length = be32toh (h->sbuf.sr.structured_reply.length); + msglen = be16toh (h->sbuf.sr.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); + set_error (0, "error payload length too large"); + return -1; + } + h->rbuf = &h->sbuf.offset; + break; + } + SET_NEXT_STATE (%RECV_ERROR_TAIL); + } + return 0; + + REPLY.STRUCTURED_REPLY.RECV_ERROR_TAIL: struct command_in_flight *cmd; uint16_t flags; uint64_t handle; uint32_t error; + uint64_t offset; switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; @@ -183,6 +231,20 @@ if (cmd->error == 0) cmd->error = nbd_internal_errno_of_nbd_error (error); + /* Sanity check that any error offset is in range */ + if (error == NBD_REPLY_TYPE_ERROR_OFFSET) { + offset = be64toh (h->sbuf.offset); + if (offset < cmd->offset || offset >= cmd->offset + cmd->count) { + SET_NEXT_STATE (%.DEAD); + set_error (0, "offset of error reply is out of bounds, " + "offset=%" PRIu64 ", cmd->offset=%" PRIu64 ", " + "cmd->count=%" PRIu32 ", " + "this is likely to be a bug in the server", + offset, cmd->offset, cmd->count); + return -1; + } + } + if (flags & NBD_REPLY_FLAG_DONE) SET_NEXT_STATE (%^FINISH_COMMAND); else diff --git a/lib/internal.h b/lib/internal.h index e7be05b..7ad6219 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -143,6 +143,7 @@ struct nbd_handle { uint32_t len; uint16_t nrinfos; uint32_t nrqueries; + uint64_t offset; } sbuf; /* Issuing a command must use a buffer separate from sbuf, for the -- 2.20.1

1 1

[libnbd PATCH 0/2] More with MSG_MORE
by Eric Blake 14 Jun '19

14 Jun '19

I'm not sure if this is worth pursuing. On paper, it makes sense (if we know we have multiple commands batched to send over the wire, AND those commands are short in length, we might as well use MSG_MORE), but the measurement numbers with it applied might just be in the noise. Eric Blake (2): examples: Enhance access patterns of threaded-reads-and-writes states: Another use for MSG_MORE examples/threaded-reads-and-writes.c | 12 ++++++++---- generator/states-issue-command.c | 4 +++- 2 files changed, 11 insertions(+), 5 deletions(-) -- 2.20.1

2 3