search for: nbd_rep_err_unsup

...line 206, in _receive_go_reply .format(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

...@ -123,15 +123,19 @@ struct nbd_fixed_new_option_reply { #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -#define NBD_REP_ACK 1 -#define NBD_REP_SERVER 2 -#define NBD_REP_INFO 3 -#define NBD_REP_META_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_ACK 1 +#define NBD_REP_SERVER 2 +#define NBD_REP...

....8 and above. > > (c) Add a workaround in nbdkit. > > > > And what workaround would that be? Looking at the qemu patch, the > problem is that qemu asked for "base:allocation" and nbdkit replied with > 0 contexts. The workaround would either be to reply with > NBD_REP_ERR_UNSUP (the behavior before NBD_OPT_SET_META_CONTEXT was > added), or to implement "base:allocation" (even if we implement the > poor-man's version that always reports that the entire image is data, > for all requests, without any feedback from the plugins). The former > feels o...

...opt_index += querylen; + nr_queries--; + } + if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + return -1; + } + } + break; + default: /* Unknown option. */ if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) -- 2.20.1

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

...like NBD_OPT_EXPORT_NAME, NBD_OPT_GO sends back an ACK + * or ERROR packet. + */ + if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + return -1; + + break; + default: /* Unknown option. */ if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) @@ -712,10 +860,10 @@ _negotiate_handshake_newstyle_options (struct connection *conn) } /* Note, since it's not very clear from the protocol doc, that the - * client must send NBD_OPT_EXPORT_NAME last, and that ends option - * negotiation. + * client must send NB...

Experimental and only very lightly tested so far. Rich.

...) { /* 32 bit export length + 16 bit nr info */ debug ("newstyle negotiation: %s option length < 6", optname); @@ -880,10 +900,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn) /* Unknown option. */ if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) return -1; - if (conn->recv (conn, data, optlen) == -1) { - nbdkit_error ("read: %m"); + if (conn_recv_full (conn, data, optlen, + "read: %m") == -1) return -1; - } } /* Note, since it's n...

.../ @@ -110,33 +119,37 @@ struct fixed_new_option_reply { #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -#define NBD_REP_ACK 1 -#define NBD_REP_SERVER 2 -#define NBD_REP_INFO 3 -#define NBD_REP_META_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_ACK 1 +#define NBD_REP_SERVER 2 +#define NBD_REP...

...data, optlen) == -1) { + nbdkit_error ("read: %m"); + return -1; + } continue; } @@ -351,6 +355,10 @@ _negotiate_handshake_newstyle_options (struct connection *conn) /* Unknown option. */ if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) return -1; + if (xread (conn->sockin, data, optlen) == -1) { + nbdkit_error ("read: %m"); + return -1; + } } /* Note, since it's not very clear from the protocol doc, that the -- 2.5.5

...wn by > this server implementation [message=]) 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...

..., Mar 23, 2019 at 06:57:14AM -0500, Eric Blake wrote: > > And what workaround would that be? Looking at the qemu patch, the > > problem is that qemu asked for "base:allocation" and nbdkit replied with > > 0 contexts. The workaround would either be to reply with > > NBD_REP_ERR_UNSUP (the behavior before NBD_OPT_SET_META_CONTEXT was > > added), or to implement "base:allocation" (even if we implement the > > poor-man's version that always reports that the entire image is data, > > for all requests, without any feedback from the plugins). The form...

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.

...lso right that guessing timelines is hard. > > (c) Add a workaround in nbdkit. > And what workaround would that be? Looking at the qemu patch, the problem is that qemu asked for "base:allocation" and nbdkit replied with 0 contexts. The workaround would either be to reply with NBD_REP_ERR_UNSUP (the behavior before NBD_OPT_SET_META_CONTEXT was added), or to implement "base:allocation" (even if we implement the poor-man's version that always reports that the entire image is data, for all requests, without any feedback from the plugins). The former feels odd, but the latter s...

nbdkit (upstream 5a7a394c699) currently fails with qemu 2.12.0: $ ./nbdkit memory size=64M --run 'qemu-img convert $nbd /var/tmp/out' nbdkit: memory.2: error: invalid request: unknown command (7) ignored qemu-img: Protocol error: simple reply when structured reply chunk was expected This was a bug in qemu which was fixed upstream quite a long time ago by the commit I've

...* END STATE MACHINE */ diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index 200d16f..42bae8a 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -149,10 +149,10 @@ SET_NEXT_STATE (%RECV_REPLY); return 0; case NBD_REP_ERR_UNSUP: - /* XXX fall back to NBD_OPT_EXPORT_NAME */ - SET_NEXT_STATE (%.DEAD); - set_error (0, "handshake: server does not support NBD_OPT_GO"); - return -1; + debug (conn->h, + "server is confused by NBD_OPT_GO, continuing anyway"); + SET_NEXT_STATE (%^OPT_EXP...

...h is observable since h->flags will be set */ + assert (!buffer); + if (!h->flags) { + nbdkit_error ("server omitted NBD_INFO_EXPORT reply to NBD_OPT_GO"); + return -1; + } + nbdkit_debug ("NBD_OPT_GO complete"); + return 1; + case NBD_REP_ERR_UNSUP: + /* Special case this failure to fall back to NBD_OPT_EXPORT_NAME */ + nbdkit_debug ("server lacks NBD_OPT_GO support"); + free (buffer); + return 0; + default: + /* Unexpected. Either the server sent a legitimate error or an + unexpected reply, but e...

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

...ptlen, - "read: %s: %m", name_of_nbd_opt (option)) == -1) - return -1; - continue; - } - - if (tls == 0) { /* --tls=off (NOTLS mode). */ -#ifdef HAVE_GNUTLS -#define NO_TLS_REPLY NBD_REP_ERR_POLICY -#else -#define NO_TLS_REPLY NBD_REP_ERR_UNSUP -#endif - if (send_newstyle_option_reply (conn, option, NO_TLS_REPLY) == -1) - return -1; - } - else /* --tls=on or --tls=require */ { - /* We can't upgrade to TLS twice on the same connection. */ - if (conn->using_tls) { - if (send_newstyle_o...

...ed request header, with proper use of > > + `NBD_CMD_FLAG_PAYLOAD_LEN` for all commands sending a payload; > > + and the server MUST use the 32-byte extended reply header. > > + - For backwards compatibility, clients SHOULD be prepared to also > > + handle `NBD_REP_ERR_UNSUP`; in this case, only the compact > > + transmission headers will be used. > > + > > + Note that a response of `NBD_REP_ERR_BLOCK_SIZE_REQD` does not > > + make sense in response to this command, but a server MAY fail with > > + that error for a later `NBD...