search for: structured_replies

This was v3 patch 2/22, reworked to address the confusion about how a structured reply header is read in two pieces before getting to the payload portion. I'm still working on rebasing the rest of my v3 series (patches 1, 3-22) from other comments given, but this seemed independent enough that it's worth posting now rather than holding it up for the rest of the series. Eric Blake (4):

Rather than repeating the logic for checking flags at multiple sites, stick it in a new state. --- generator/generator | 7 +++++ generator/states-reply-structured.c | 43 ++++++++++------------------- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/generator/generator b/generator/generator index a289741..4c81859 100755 --- a/generator/generator +++

...SET_NEXT_STATE (%.READY); - set_error (0, "no matching handle found for server reply, " - "this is probably a bug in the server"); - return -1; - } + assert (cmd); + assert (cmd->handle == handle); if (cmd->type == NBD_CMD_READ && h->structured_replies) { set_error (0, "server sent unexpected simple reply for read"); diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 2125e41..9bb165b 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -43,7 +43,7 @@...

On Wed, Sep 11, 2019 at 08:48:28AM -0500, Eric Blake wrote: > On 9/11/19 5:40 AM, Richard W.M. Jones wrote: > > Very much a work in progress as there are still many tests using > > qemu-io which are candidates for conversion. > > > > You'll notice at the end of test-full.sh that the new test has some > > duplicated code which looks as if it ought to be

Very much a work in progress as there are still many tests using qemu-io which are candidates for conversion. You'll notice at the end of test-full.sh that the new test has some duplicated code which looks as if it ought to be refactored into a Python function. When I tried to do that, I got loads of strange Python problems which may indicate bugs in nbdsh itself or problems with my

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

...gt; - error = be32toh (h->sbuf.simple_reply.error); > + error = be32toh (h->sbuf.reply.hdr.simple.error); > > if (cmd == NULL) { > /* Unexpected reply. If error was set or we have structured > @@ -39,7 +39,7 @@ REPLY.SIMPLE_REPLY.START: > if (error || h->structured_replies) > SET_NEXT_STATE (%^FINISH_COMMAND); > else { > - uint64_t cookie = be64toh (h->sbuf.simple_reply.cookie); > + uint64_t cookie = be64toh (h->sbuf.reply.hdr.simple.cookie); > SET_NEXT_STATE (%.DEAD); > set_error (EPROTO, >...

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

When we introduced valid_flags, there was an incentive to do as few callbacks as possible, favoring cb(VALID|FREE) calls over the sequence cb(VALID);cb(FREE). To make it work, we set .callback=NULL after an early free, so that the later check during retirement didn't free again. But now that our .free callback is distinct from our other callbacks, there is no longer an advantage to bundling

...in nbdkit_extents in patch 1, this may be picked up automatically) > @@ -498,15 +605,23 @@ protocol_recv_request_send_reply (struct connection *conn) > } > > /* Currently we prefer to send simple replies for everything except > - * where we have to (ie. NBD_CMD_READ when structured_replies have > - * been negotiated). However this prevents us from sending > - * human-readable error messages to the client, so we should > - * reconsider this in future. > + * where we have to (ie. NBD_CMD_READ and NBD_CMD_BLOCK_STATUS when > + * structured_replies have been neg...

On Wed, Aug 14, 2019 at 05:38:31PM -0500, Eric Blake wrote: > When we introduced valid_flags, there was an incentive to do as few > callbacks as possible, favoring cb(VALID|FREE) calls over the sequence > cb(VALID);cb(FREE). To make it work, we set .callback=NULL after an > early free, so that the later check during retirement didn't free > again. > > But now that our

On Wed, Jun 19, 2019 at 01:18:01PM -0500, Eric Blake wrote: > Oddly enough, I am not getting any measurable performance difference > with this patch applied and using examples/threaded-reads-and-writes > coupled with nbdkit. My explanation is that in the common case, once > a server has something to send, it is going to send the entire reply > as fast as it can, rather than sending

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must

When processing a server reply within the REPLY subgroup, we will often hit a situation where recv() requires us to block until the next NotifyRead. But since NotifyRead is the only permitted external action while in this group, we are effectively blocking CmdIssue and NotifyWrite events from happening until the server finishes the in-progress reply, even though those events have no strict

...++++++++++++++++++++++--- 4 files changed, 229 insertions(+), 15 deletions(-) diff --git a/server/internal.h b/server/internal.h index ae51804..58d5794 100644 --- a/server/internal.h +++ b/server/internal.h @@ -183,6 +183,7 @@ struct connection { bool can_multi_conn; bool using_tls; bool structured_replies; + bool meta_context_base_allocation; int sockin, sockout; connection_recv_function recv; @@ -219,6 +220,12 @@ extern int protocol_handshake_newstyle (struct connection *conn) extern int protocol_recv_request_send_reply (struct connection *conn) __attribute__((__nonnull__ (1))); +/*...

This is about the simplest implementation of NBD Structured Replies (SR) that we can do right now. It accepts NBD_OPT_STRUCTURED_REPLIES when negotiated by the client, but only sends back the simplest possible SR when required to by NBD_CMD_READ. The rest of the time it will send back simple replies as before. We do not modify the plugin API so plugins are unable to send complex SRs. Also we do not handle human-readable error mes...

...+++++++++++++++++++++++-- 4 files changed, 335 insertions(+), 16 deletions(-) diff --git a/server/internal.h b/server/internal.h index 825dd3e..03d6119 100644 --- a/server/internal.h +++ b/server/internal.h @@ -183,6 +183,7 @@ struct connection { bool can_multi_conn; bool using_tls; bool structured_replies; + bool meta_context_base_allocation; int sockin, sockout; connection_recv_function recv; @@ -219,6 +220,12 @@ extern int protocol_handshake_newstyle (struct connection *conn) extern int protocol_recv_request_send_reply (struct connection *conn) __attribute__((__nonnull__ (1))); +/*...

When the server sets NBD_REPLY_FLAG_DONE on a data or block status chunk, we can use that fact to pass (VALID|FREE) and avoid a separate callback later just for FREE. --- As I've been promising in other threads... generator/states-reply-structured.c | 33 +++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/generator/states-reply-structured.c

...r *name_of_nbd_rep (int); #define NBD_REP_ACK 1 diff --git a/server/connections.c b/server/connections.c index aeb27f8..7e32f00 100644 --- a/server/connections.c +++ b/server/connections.c @@ -926,6 +926,90 @@ _negotiate_handshake_newstyle_options (struct connection *conn) conn->structured_replies = true; break; + case NBD_OPT_LIST_META_CONTEXT: + case NBD_OPT_SET_META_CONTEXT: + { + uint32_t opt_index; + uint32_t exportnamelen; + uint32_t nr_queries; + uint32_t querylen; + const char *what; + + optname = name_of_nbd_opt (option);...

Diff against libnbd’s copy of this file, and change this one until it matches. --- common/protocol/nbd-protocol.h | 76 +++++++++++++++++----------- server/protocol-handshake-newstyle.c | 26 +++++----- server/protocol-handshake-oldstyle.c | 4 +- server/protocol.c | 25 ++++----- tests/test-layers.c | 14 ++--- 5 files changed, 81 insertions(+), 64