search for: nbd_opt_export_name

There's no substantial difference over v1, I simply fixed a few whitespace issues, moved one struct around and tidied up the comments. Rich.

..._context_state_machine); Group ("OPT_GO", newstyle_opt_go_state_machine); + Group ("OPT_EXPORT_NAME", newstyle_opt_export_name_state_machine); ] (* Fixed newstyle NBD_OPT_STARTTLS option. *) @@ -565,6 +566,44 @@ and newstyle_opt_go_state_machine = [ }; ] +(* Newstyle NBD_OPT_EXPORT_NAME option. *) +and newstyle_opt_export_name_state_machine = [ + State { + default_state with + name = "START"; + comment = "Try to send newstyle NBD_OPT_EXPORT_NAME to end handshake"; + external_events = []; + }; + + State { + default_state with + name = "...

Actually I was wrong, there *is* one substantive change over v1, which is this: > + /* The client is buggy. The last option must be NBD_OPT_GO or > + * NBD_OPT_EXPORT_NAME. > + */ > + else { > + nbdkit_error ("client options list didn't finish with NBD_OPT_GO " > + "or NBD_OPT_EXPORT_NAME"); > + return -1; > + } > + > return 0; Current behaviour in nbdkit is that if the last requested option is a...

...te_handshake_newstyle_options (struct connection *conn) } /* Apart from printing it, ignore the export name. */ data[optlen] = '\0'; - debug ("newstyle negotiation: client requested export '%s' (ignored)", + debug ("newstyle negotiation: NBD_OPT_EXPORT_NAME: " + "client requested export '%s' (ignored)", data); break; @@ -701,6 +754,101 @@ _negotiate_handshake_newstyle_options (struct connection *conn) } break; + case NBD_OPT_GO: + if (conn->recv (conn, data, optlen) =...

...ERROR_REPLY[reply], message)) ovirt_imageio_common.nbd.Error: The option sent by the client is unknown by this server implementation [message=]) According to the NBD protocol: For backwards compatibility, clients SHOULD be prepared to also handle NBD_REP_ERR_UNSUP by falling back to using NBD_OPT_EXPORT_NAME. Should I fall back to sending NBD_OPT_EXPORT_NAME, or nbkit needs to implement NBD_OPT_GO? The client code is here: https://gerrit.ovirt.org/c/93384/ Nir

This is only lightly tested (against just qemu NBD client), and the code might be structured a little better as the _negotiate_handshake_newstyle_options function has now grown to be huge. Anyway works for me. Rich.

The NBD spec was recently patched (nbd.git commit 7827f3ae and friends) to require NBD_OPT_GO for baseline interoperability, with the aim of fewer servers and clients falling back to NBD_OPT_EXPORT_NAME. And since nbdkit as server recently started supporting NBD_OPT_GO (commit f7dd9799), our nbd client as plugin should take advantage of it. This patch is a prerequisite to teaching the nbd plugin about structured replies, which in turn will allow an implementation of .extents. Signed-off-by: Eric...

...anual edits to qemu-nbd source code to provoke various other compliant (if uncommon) server behaviors. Eric Blake (4): starttls: Skip error payload if falling back to unencrypted states: Reject payload to NBD_REP_ACK meta-context: Skip error payload if server lacks meta_context states: Add NBD_OPT_EXPORT_NAME handling generator/Makefile.am | 1 + generator/generator | 53 ++++++++++++++ generator/states-newstyle-opt-export-name.c | 73 +++++++++++++++++++ generator/states-newstyle-opt-go.c | 13 +++- .../states-newstyle-opt-set-meta-conte...

On 08/06/2018 07:03 AM, Richard W.M. Jones wrote: > Actually I was wrong, there *is* one substantive change over v1, which > is this: > >> + /* The client is buggy. The last option must be NBD_OPT_GO or >> + * NBD_OPT_EXPORT_NAME. >> + */ >> + else { >> + nbdkit_error ("client options list didn't finish with NBD_OPT_GO " >> + "or NBD_OPT_EXPORT_NAME"); >> + return -1; >> + } As I pointed out in the other mail, this else clause is unreachable....

...-git a/doc/proto.md b/doc/proto.md > index 8f08583..53c334a 100644 > --- a/doc/proto.md > +++ b/doc/proto.md > @@ -745,8 +745,8 @@ text unless the client insists on TLS. > > During transmission phase, several operations are constrained by the > export size sent by the final `NBD_OPT_EXPORT_NAME` or `NBD_OPT_GO`, > -as well as by three block size constraints defined here (minimum, > -preferred, and maximum). > +as well as by three block size constraints defined here (minimum > +block, preferred block, and maximum payload). > > If a client can honour server block size co...

...age=]) Correct, nbdkit has not yet implemented this extension. (It's on my list of things to add, someday...) > > According to the NBD protocol: > > For backwards compatibility, clients SHOULD be prepared to also handle > NBD_REP_ERR_UNSUP by falling back to using NBD_OPT_EXPORT_NAME. > > Should I fall back to sending NBD_OPT_EXPORT_NAME, or nbkit needs > to implement NBD_OPT_GO? Both. You need to implement the fallback to NBD_OPT_EXPORT_NAME regardless of whether nbdkit is fixed, because there are other servers that are also not fixed, and you want to interoperate...

...> > index 8f08583..53c334a 100644 > > --- a/doc/proto.md > > +++ b/doc/proto.md > > @@ -745,8 +745,8 @@ text unless the client insists on TLS. > > > > During transmission phase, several operations are constrained by the > > export size sent by the final `NBD_OPT_EXPORT_NAME` or `NBD_OPT_GO`, > > -as well as by three block size constraints defined here (minimum, > > -preferred, and maximum). > > +as well as by three block size constraints defined here (minimum > > +block, preferred block, and maximum payload). > > > > If a client c...

On Tue, Sep 24, 2019 at 05:24:34PM -0500, Eric Blake wrote: > On 9/24/19 4:07 PM, Richard W.M. Jones wrote: > > /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. > > * Modern clients use NBD_OPT_GO instead of this. > > @@ -167,7 +167,7 @@ struct nbd_new_handshake_finish { > > uint64_t exportsize; > > uint16_t eflags; /* per-export flags */ > > char zeroes[124]; /* must be sent as zero bytes */ &...

This is similar to the recent --no-sr option - it's a change that is unlikely to ever be used except by someone testing whether a client is compliant to the protocol, but in that niche case, it can be quite handy (it's a lot nicer to be able to purposefully cripple a server from the command line than from a one-off compile, when testing if a client's fallback for a spec-compliant but

...hether the client will request structured replies, we can also expose whether the client will understand various handshake flags from the NBD protocol. Of course, we default to supporting all flags that we understand and which are advertised by the server. But clearing FIXED_NEWSTYLE lets us test NBD_OPT_EXPORT_NAME handling, and clearing NO_ZEROES lets us test whether the zero padding in response to NBD_OPT_EXPORT_NAME is correct. --- I'm still considering the addition of tests/* against nbd, or interop/* against qemu, to ensure that we have interoperability between various degraded connection modes. But...

...NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */ struct nbd_block_descriptor { uint32_t length; /* length of block */ uint32_t status_flags; /* block type (hole etc) */ -} __attribute__((packed)); +} NBD_ATTRIBUTE_PACKED; /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO instead of this. @@ -167,7 +167,7 @@ struct nbd_new_handshake_finish { uint64_t exportsize; uint16_t eflags; /* per-export flags */ char zeroes[124]; /* must be sent as zero bytes */ -} __attribute__((packed)); +} NBD_ATTRIBUTE_PACKED...

Experimental and only very lightly tested so far. Rich.

On 08/06/2018 09:31 AM, Nir Soffer wrote: > Eric, can you point us to the part of the spec allowing ignoring the export > name sent by the client? Nothing in the NBD spec requires the server to reject unknown export names. So nbdkit never rejects export names (which means it behaves as if all names work, regardless of whether or not it was the name it was configured with, since it

Since the v1 series (0/4, at [1]), I've applied patches 1 and 2, rewritten patch 3 [Forbid NUL in export and context names] into patch 4 here, patch 4 there turned into patch 6 here, and everything else here is new. [1]https://www.redhat.com/archives/libguestfs/2019-September/msg00180.html I don't know if there is a handy reusable function for checking whether a string contains valid

If a plugin's .open or .get_size or .can_write fails, right now that is fatal to the connection. When nbdkit was first implemented, this made sense (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_...