search for: nbd_rep_err_unknown

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

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

...support plugins that have a different size per connection); and that nbdkit in general is still rather rude to clients when .prepare prevents the use of a connection (perhaps nbdkit should instead be taught to proceed with the socekt, but respond to NBD_OPT_LIST with 0 items and to NBD_OPT_GO with NBD_REP_ERR_UNKNOWN or NBD_REP_ERR_SHUTDOWN to give the client a hint that this server has nothing further to offer). Signed-off-by: Eric Blake <eblake@redhat.com> --- I pushed this one, but am posting to the list because of the conversation points it brings up about what to do when .prepare fails (there's...

...> + sys.exit (0) > +# This should not happen. > +sys.exit (1) The test looks good. (Yeah, figuring out how to make it more graceful in the future might be nice, but that's not this patch's problem. I'm thinking that if a plugin's .open() fails, nbdkit could reply with NBD_REP_ERR_UNKNOWN or NBD_REP_ERR_INVALID, and then wait for the client to disconnect, rather than the current hard hangup initiated by the server) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...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 export using different name? Wha...

...-1; + if (finish_newstyle_options (conn, &exportsize) == -1) { + if (backend->finalize (backend, conn) == -1) + return -1; + backend_close (backend, conn); + if (send_newstyle_option_reply (conn, option, + NBD_REP_ERR_UNKNOWN) == -1) + return -1; + continue; + } if (send_newstyle_option_reply_info_export (conn, option, NBD_REP_INFO, @@ -497,7 +513,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) }...

...ng the string directly through printf) >> The test looks good. (Yeah, figuring out how to make it more graceful >> in the future might be nice, but that's not this patch's problem. I'm >> thinking that if a plugin's .open() fails, nbdkit could reply with >> NBD_REP_ERR_UNKNOWN or NBD_REP_ERR_INVALID, and then wait for the client >> to disconnect, rather than the current hard hangup initiated by the server) > > Abrupt disconnection isn't very good here. I guess we have never > really thought before about .open failing. I've hit it before (such as...

...# This should not happen. > > +sys.exit (1) > > The test looks good. (Yeah, figuring out how to make it more graceful > in the future might be nice, but that's not this patch's problem. I'm > thinking that if a plugin's .open() fails, nbdkit could reply with > NBD_REP_ERR_UNKNOWN or NBD_REP_ERR_INVALID, and then wait for the client > to disconnect, rather than the current hard hangup initiated by the server) Abrupt disconnection isn't very good here. I guess we have never really thought before about .open failing. Rich. -- Richard Jones, Virtualization Group, Re...

...ER_DIR("dir2/") NBD_OPT_LIST_HIER("dir1/") => SERVER("dir1/f2"), SERVER("dir1/f3") NBD_OPT_LIST_HIER("dir2/") => SERVER("dir2/f4") NBD_OPT_LIST_HIER("f1") => NBD_REP_ERR_NOTDIR NBD_OPT_LIST_HIER("nosuch") => NBD_REP_ERR_UNKNOWN True, this hierarchy is only available to new client/server pairs (if either side lacks the new code, you're back to flat listings), but it is food for thought. I'll propose something on the upstream NBD list if we think it has potential. > > I think we need to consider these a...

...TA_CONTEXT 4 +#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1) +#define NBD_REP_ERR_POLICY NBD_REP_ERR (2) +#define NBD_REP_ERR_INVALID NBD_REP_ERR (3) +#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) +#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5) +#define NBD_REP_ERR_UNKNOWN NBD_REP_ERR (6) +#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR (7) +#define NBD_REP_ERR_BLOCK_SIZE_REQD NBD_REP_ERR (8) +#define NBD_REP_ERR_TOO_BIG NBD_REP_ERR (9) #define NBD_INFO_EXPORT 0 /* NBD_INFO_EXPORT reply (follows fixed_new_option_reply). */ -struct fixed...

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

Hi Eric, Nir. $SUBJECT - it's complicated! Imagine a scenario where we have extended the file plugin so you can point it at a directory and it will use the client-supplied export name to select a file to serve. Also we have extended the tar filter so that you can map tar file components into export names. You may be able to see where this is going ... Now we point the file plugin at a

...nded headers. While this may make it easier to + implement a server with fewer considerations for backwards + compatibility, it limits connections to only recent clients. Additionally, if TLS has not been initiated, the server MAY reply with `NBD_REP_ERR_TLS_REQD` (instead of `NBD_REP_ERR_UNKNOWN`) to @@ -1350,6 +1451,9 @@ of the newstyle negotiation. server MUST use structured replies to the `NBD_CMD_READ` transmission request. Other extensions that require structured replies may now be negotiated. + - `NBD_REP_ERR_EXT_HEADER_REQD`: The client has already + su...

...if (finish_newstyle_options (&exportsize) == -1) { - if (backend_finalize (backend) == -1) + if (backend_finalize (top) == -1) return -1; - backend_close (backend); + backend_close (top); if (send_newstyle_option_reply (option, NBD_REP_ERR_UNKNOWN) == -1) return -1; continue; @@ -518,9 +518,9 @@ negotiate_handshake_newstyle_options (void) return -1; if (option == NBD_OPT_INFO) { - if (backend_finalize (backend) == -1) + if (backend_finalize (top) == -1) return -1; - ba...

We should have only one NBD protocol file. Let's make nbdkit's version the canonical one, and use it in libnbd. Rich.

...ackend, conn) == -1) + if (finish_newstyle_options (&exportsize) == -1) { + if (backend_finalize (backend) == -1) return -1; - backend_close (backend, conn); - if (send_newstyle_option_reply (conn, option, - NBD_REP_ERR_UNKNOWN) == -1) + backend_close (backend); + if (send_newstyle_option_reply (option, NBD_REP_ERR_UNKNOWN) == -1) return -1; continue; } - if (send_newstyle_option_reply_info_export (conn, option, + if (send_newstyle_option_reply_info_export...

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

This series is based on my blog posting here: https://rwmj.wordpress.com/2019/09/13/nbdkit-supports-exportnames/ It depends on the fix for realloc: https://www.redhat.com/archives/libguestfs/2019-September/thread.html#00103 This series adds a fun plugin, and also an semi-related feature I've long thought to be desirable. You can consider patches 1 & 4, and patches 2 & 3 as forming

v2 was here: https://lists.debian.org/nbd/2022/11/msg00030.html The bulk of the changes since then are: - forbid NBD_OPT_EXPORT_NAME once extended headers are negotiated (Wouter) - consistently use 'maximum payload', rather than a haphazard mix of 'maximum block payload' (Vladimir) At this point, I want to make sure we are happy with the spec before re-posting patches for