search for: nbd_opt_go

There's no substantial difference over v1, I simply fixed a few whitespace issues, moved one struct around and tidied up the comments. 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...

...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) == -1) { + nbdkit_error ("read: %m"); + return -1; + } + + if (optlen < 6) { /* 32 bit export length + 16 bit nr info */ + debug ("newstyle negotiation: NBD_OPT_GO option length < 6"); + +...

..._LIST and NBD_OPT_INFO to give the user as much detail as possible about an export without actually connecting to it. For that to display more than the export name when nbdkit is the server, we need to implement NBD_OPT_INFO. Thankfully, the NBD spec intentionally made the command very similar to NBD_OPT_GO, to the point that all we really have to do is tweak our debug messages :) Signed-off-by: Eric Blake <eblake@redhat.com> --- This turned out to be so trivial that I'm pushing it right away. docs/nbdkit-protocol.pod | 7 +++++++ src/protocol.h | 1 + src/connections.c...

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

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

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

...> > 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 with all such servers. But nbdkit should (eventually) learn how to support NBD_OPT_GO. > > The cl...

...; I does not make sense to allow access if the server was configured with one export, and the user ask for another export. This is likely a bug in user code or the code running nbdkit. If users do not want to specify the export, they run nbdkit with empty export name, and use empty export name in NBD_OPT_GO. If an export was specified it should be handled in a strict way. Here is one use case that will be easily avoided by being strict about export name: 1. application starts nbdkit on a port 2. application reports port to user 3. application time out, terminating nbdkit 4. applcation starts nbdkit...

...all the time). > The spec mention this about NBD_OPT_EXPORT_NAME: If the chosen export does not exist or requirements for the chosen export are not met (e.g., the client did not initiate TLS for an export where the server requires it), the server MUST terminate the session. and for NBD_OPT_GO, we have NBD_REP_ERR_UNKNOWN: The chosen export does not exist on this server. In this case, the server SHOULD NOT send NBD_REP_INFO replies. And the spec also says: If the server is unwilling to allow the export, it MUST terminate the session Why would a server want to allow an exp...

Most known NBD clients do not bother with NBD_OPT_INFO (except for clients like 'qemu-nbd --list' that don't ever intend to connect), but go straight to NBD_OPT_GO. However, it's not too hard to hack up qemu to add in an extra client step (whether info on the same name, or more interestingly, info on a different name), as a patch against qemu commit 6f214b30445: | diff --git i/nbd/client.c w/nbd/client.c | index f6733962b49b..425292ac5ea9 100644 | --- i...

...*/ + nbd_set_opt_mode (nbd2, true); - if (nbd_connect_uri (nbd2, new_uri) == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } + if (nbd_connect_uri (nbd2, uri) == -1 || + nbd_set_export_name (nbd2, name) == -1 || + nbd_opt_go (nbd2) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } - /* List the metadata of this export. */ - desc = nbd_get_list_export_description (nbd1, i); - list_one_export (nbd2, desc, i == 0, i + 1 == count); + /* List the meta...

...low access if the server was configured > with one export, and the user ask for another export. This is likely a bug > in user code or the code running nbdkit. > > If users do not want to specify the export, they run nbdkit with empty > export > name, and use empty export name in NBD_OPT_GO. If an export was > specified it should be handled in a strict way. > > Here is one use case that will be easily avoided by being strict about > export name: > > 1. application starts nbdkit on a port > 2. application reports port to user > 3. application time out, termina...

On Sat, Aug 4, 2018 at 4:04 PM Richard W.M. Jones <rjones@redhat.com> wrote: > 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. > Works for my python nbd client: $ rm -f /tmp/nbd.sock && src/nbdkit -f

On Sun, Aug 05, 2018 at 12:11:04AM +0300, Nir Soffer wrote: > But we have a bug - server configure to allow access to "export", > but allow access to the default empty "" export. nbdkit ignores the export name you pass in and always has done: https://github.com/libguestfs/nbdkit/blob/a2c4e88af503310892ebce6da3ac478beb47888a/src/connections.c#L626 The -e option on the

I'm easily able to provoke NBD_REP_ERR_TLS_REQD (use nbd_set_tls(0) to talk to a server that requires encryption) and NBD_REP_ERR_UNKNOWN (forget to use nbd_set_export_name for qemu-nbd); it's nice to display a useful error for these rather than "unknown reply from NBD_OPT_GO: 0x80000005" or similar. Other errors are less common, but as long as we're decoding things, it doesn't hurt to decode everything in the protocol. --- I'm pushing this one now (I mentioned it on IRC earlier today). generator/states-newstyle-opt-go.c | 32 +++++++++++++++++++++++...

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

...-extended-headers.c diff --git a/generator/API.ml b/generator/API.ml index 36033817..d1849710 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -953,23 +953,24 @@ "set_request_meta_context", { (all C<nbd_connect_*> calls when L<nbd_set_opt_mode(3)> is false, or L<nbd_opt_go(3)> and L<nbd_opt_info(3)> when option mode is enabled) will also try to issue NBD_OPT_SET_META_CONTEXT when -the server supports structured replies and any contexts were -registered by L<nbd_add_meta_context(3)>. The default setting -is true; however the extra step of negotiating...

On 08/06/2018 02:54 AM, Richard W.M. Jones wrote: > On Sun, Aug 05, 2018 at 12:11:04AM +0300, Nir Soffer wrote: >> But we have a bug - server configure to allow access to "export", >> but allow access to the default empty "" export. > > nbdkit ignores the export name you pass in and always has done: > >