search for: nbd_opt_abort

At least 'nbd-client -l localhost' is a good demonstration of a client that legitimately sends NBD_OPT_ABORT - it exists only to enumerate the exports advertised by the server, rather than intending to connect to any of those exports. As such, we should downgrade the message for a client telling us it is going away to the same status as our debug message for NBD_CMD_DISC, given that the two have the same...

On Mon, Jun 12, 2023 at 07:27:52PM +0100, Richard W.M. Jones wrote: > This change avoids the following harmless but annoying warning on the > normal exit path: > > libnbd: debug: nbd1: nbd_opt_abort: enter: > libnbd: debug: nbd1: nbd_opt_abort: leave: error="nbd_opt_abort: invalid state: READY: the handle must be negotiating: Invalid argument" > --- > info/main.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake <eblake at redhat....

...entry (all nbd_opt_* will cause an IssueCommand event to kick the state machine out of NEGOTIATING, at which point we want to jump to the sub-state appropriate for that option). This is done by setting h->current_opt prior to entering NEWSTYLE. We also need a couple of new states to implement NBD_OPT_ABORT; this was not deemed worth a separate sub-group and new states-newstyle-opt-abort.c file, so I just inlined it in NEWSTYLE instead. One nice benefit of these new states: when NBD_OPT_GO fails and we are not in opt mode, or when we require TLS but the server does not support it, we can now hang up...

...ption negotiation mode. */ > bool opt_mode; > + uint8_t current_opt; Be nice to add a comment here about what current_opt can contain, which would also explain why it has this somewhat unexpected type. Something like: + uint8_t current_opt; /* 0 or NBD_OPT_* */ (Can it only contain NBD_OPT_ABORT or are other options added in later patches?) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many lan...

These patches aren't related to each other, but both are quite simple. The second one requires particular attention - it's my experience that printing out the state transitions in debug mode has never helped me to diagnose a bug, but it has made the debug logs huge and hard to follow. However that might just be me! Has it helped anyone else? Also I'm open to the concept of debug

...needed to allow the user to take control over the rest of option negotiating. This patch adds several new API: nbd_set_opt_mode() - called during Created to enable the new mode nbd_get_opt_mode() - query whether opt mode is enabled nbd_opt_go() - used in Negotiating state to attempt to use export nbd_opt_abort() - used in Negotiating state to skip Connected state nbd_aio_is_negotiating() - used to detect Negotiating state Older clients that do not use nbd_set_opt_mode see no difference: the new state is never reached, all configuration (export name, requesting meta contexts) has to be done in Created st...

...eady to commit while I continue working on the latter half. Eric Blake (13): api: Add nbd_set_full_info and friends info: Improve info without --list api: Add nbd_is_state_negotiating for new state api: Permit several existing API in Negotiating state api: Add nbd_set_opt_mode api: Add nbd_opt_abort and nbd_aio_opt_abort api: Add nbd_opt_go and nbd_aio_opt_go examples: Update list-exports to demonstrate nbd_opt_go info: Simplify by using nbd_opt_go api: Add nbd_opt_list api: Add nbd_aio_opt_list wip: api: Give aio_opt_go a completion callback wip: api: add nbd_opt_info, nbd_aio_o...

...*argv[]) fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } - /* Disconnect from the server to move the handle into a closed - * state, in case the server serializes further connections. - * But we can ignore errors in this case. - */ - nbd_opt_abort (nbd); + if (probe_content) + /* Disconnect from the server to move the handle into a closed + * state, in case the server serializes further connections. + * But we can ignore errors in this case. + */ + nbd_opt_abort (nbd); } if (size_only) { @@ -456,9 +457,6...

...12,25 @@ handle_reply_error (struct nbd_handle *h) > > STATE_MACHINE { > NEWSTYLE.START: > + if (h->opt_mode) { > + switch (h->sbuf.option.option) { > + case NBD_OPT_GO: > + SET_NEXT_STATE (%OPT_SET_META_CONTEXT.START); > + return 0; > + case NBD_OPT_ABORT: > + h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); > + h->sbuf.option.option = htobe32 (NBD_OPT_ABORT); > + h->sbuf.option.optlen = htobe32 (0); > + h->wbuf = &h->sbuf; > + h->wlen = sizeof h->sbuf.option; > + SET_NEXT_S...

...receiving the list. */ > + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); > + return 0; > + > + default: > + if (handle_reply_error (h) == -1) { > + SET_NEXT_STATE (%.DEAD); Would this be better as %.CLOSED, or even a transition into a state where we send a clean NBD_OPT_ABORT for graceful shutdown rather than abrupt disconnect from the server? > + return 0; > + } > + set_error (ENOTSUP, "unexpected response, possibly the server does not " > + "support listing exports"); > + SET_NEXT_STATE (%.DEAD); wherea...

...d-ftl-wearleveling.py disksize=52428800 Segmentation fault nbdkit -n -f python script=ssd-ftl-wearleveling.py disksize=52428800 nbdkit: python[1]: error: invalid flush request: expecting offset and length == 0 nbdkit: python[1]: error: write reply: Broken pipe nbdkit: python[3]: error: client sent NBD_OPT_ABORT to abort the connection nbdkit: python[6]: error: ssd-ftl-wearleveling.py: callback failed: close Traceback (most recent call last): File "ssd-ftl-wearleveling.py", line 221, in close pickle.dump(lbahist, f) File "/usr/lib/python2.7/pickle.py", line 1370, in dump Segment...

Experimental and only very lightly tested so far. Rich.

...ent to inspect details about +the export without actually connecting. + =item Structured Replies I<Not supported>. diff --git a/src/protocol.h b/src/protocol.h index 792a905..088dcab 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -98,6 +98,7 @@ struct fixed_new_option_reply { #define NBD_OPT_ABORT 2 #define NBD_OPT_LIST 3 #define NBD_OPT_STARTTLS 5 +#define NBD_OPT_INFO 6 #define NBD_OPT_GO 7 #define NBD_REP_ACK 1 diff --git a/src/connections.c b/src/connections.c index 1b40e46..410a893 100644 --- a/src/connections.c +++ b/src/connections.c...

...o negotiate with a client that has early EOF at a message boundary, just as we do for one that quits mid-field, with the one difference that we treat a message boundary as a warning instead of an error because a client hanging up after an option response that it didn't like (rather than sending NBD_OPT_ABORT to inform the server that it won't be negotiating further) is a surprisingly common behavior among some existing clients. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/connections.c | 60 ++++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 21 deleti...

This replaces 13/13 of my v2 series; and now that it has pretty good testsuite coverage and demonstrable performance improvement to nbdinfo, I'm going ahead and pushing this now. We may still want to add further nbd_opt_* commands for other fine-grained tuning of negotiation, but for now, I think things have stabilized on this end, and I can return to polishing .list_exports on the nbdkit

With some crufty sed scripts we can generate functions that map from NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ"). This works on GNU sed and with FreeBSD, also with GNU sed's --posix option, so I guess the sed code is POSIX-compatible. Rich.

...e next command). Another thought - if we ever want to allow the user the ability to send custom NBD_OPT_ commands during handshake, or even to emulate 'qemu-nbd --list' where the client can make queries to see what the server supports before finally settling on whether to run NBD_OPT_GO or NBD_OPT_ABORT, we'll need to add an external event after nbd_aio_connect but before nbd_aio_is_connected for doing those additional handshake steps. It's easier to think about adding a mandatory nbd_aio_go() called in between nbd_aio_connect*() and the first nbd_aio_pread now, before we have to start wor...

...8800 > Segmentation fault > > nbdkit -n -f python script=ssd-ftl-wearleveling.py disksize=52428800 > nbdkit: python[1]: error: invalid flush request: expecting offset and > length == 0 > nbdkit: python[1]: error: write reply: Broken pipe > nbdkit: python[3]: error: client sent NBD_OPT_ABORT to abort the connection > nbdkit: python[6]: error: ssd-ftl-wearleveling.py: callback failed: close > Traceback (most recent call last): > File "ssd-ftl-wearleveling.py", line 221, in close > pickle.dump(lbahist, f) > File "/usr/lib/python2.7/pickle.py", l...

This is a subset of my v2 posting, but limited to just the NBD_OPT_LIST handling. The biggest change since v2 is the addition of added unit testing in all four language bindings (C, python, ocaml, golang). The tests require nbdkit built from git on PATH, and may not be entirely idiomatic, but I at least validated that they catch issues (for example, adding an exit statement near the end of the

...there was no way to report errors to oldstyle or NBD_OPT_EXPORT_NAME). But now that newstyle is around, it's rather abrupt to hang up on the client, and better is to return an error to NBD_OPT_GO, and let the client choose what to do (most clients will probably hang up, whether gracefully with NBD_OPT_ABORT or abruptly, rather than try other NBD_OPT_*, but _we_ shouldn't be the ones forcing their hand). For an example of what this improves, if you run: $ nbdkit -fv sh - <<\EOF case $1 in get_size) echo oops >&2; exit 1;; *) exit 2;; esac EOF Pre-patch, qemu complains about the abru...