search for: nbd_rep_is_err

...ATE (%^OPT_GO.START); diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c index e994ffd..61f254f 100644 --- a/generator/states-newstyle-opt-starttls.c +++ b/generator/states-newstyle-opt-starttls.c @@ -83,9 +83,10 @@ return 0; default: - if (!NBD_REP_IS_ERR (reply)) - debug (h, - "server is confused by NBD_OPT_STARTTLS, continuing anyway"); + if (handle_reply_error (h) == -1) { + SET_NEXT_STATE (%.DEAD); + return -1; + } /* Server refused to upgrade to TLS. If h->tls is not require (2) * then...

...turn 0; otherwise, return -1. > + */ > +static int > +handle_reply_error (struct nbd_handle *h) > +{ > + uint32_t len; > + uint32_t reply; > + > + len = be32toh (h->sbuf.or.option_reply.replylen); > + reply = be32toh (h->sbuf.or.option_reply.reply); > + if (!NBD_REP_IS_ERR (reply)) { > + set_error (0, "handshake: unexpected option reply type %d", reply); > + return -1; > + } > + > + assert (NBD_MAX_STRING < sizeof h->sbuf.or.payload); Missing an addition of #include <assert.h>. -- Eric Blake, Principal Software Engineer...

...} SET_NEXT_STATE (%.DEAD); return -1; } diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h index 405af3e..3e3fb4e 100644 --- a/lib/nbd-protocol.h +++ b/lib/nbd-protocol.h @@ -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...

I'm still playing with ideas on how to split rstate from wstate (so that we can send a request without waiting for POLLIN to complete a pending reply), but this is some preliminary refactoring I found useful. I also fixed a couple of bugs while in the area (already pushed). There's a question of whether we want nbd_handle to be nearly 5k, or if we should instead keep it small and add one

...ack 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 either way, we can't connect. */ + if (NBD_REP_IS_ERR (reply.reply)) + if (reply.replylen) + nbdkit_error ("server rejected NBD_OPT_GO with %s: %s", + name_of_nbd_rep (reply.reply), buffer); + else + nbdkit_error ("server rejected NBD_OPT_GO with %s", + n...

...tyle handshake only). */ -extern const char *name_of_nbd_opt (int); #define NBD_OPT_EXPORT_NAME 1 #define NBD_OPT_ABORT 2 #define NBD_OPT_LIST 3 @@ -113,7 +110,6 @@ extern const char *name_of_nbd_opt (int); #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -extern const char *name_of_nbd_rep (int); #define NBD_REP_ACK 1 #define NBD_REP_SERVER 2 #define NBD_REP_INFO 3 @@ -124,7 +120,6 @@ extern const char *name_of_nbd_rep (int); #define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) #define NBD_REP...

...nbd_fixed_new_option_reply { uint64_t magic; /* NBD_REP_MAGIC */ uint32_t option; /* option we are replying to */ uint32_t reply; /* NBD_REP_* */ @@ -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 (...

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

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

Updated based on other changes that have happened in the meantime: - rely more on cleanup.h (throughout) - split structured read for easier review (patch 2 and 3 were combined in v1) - rely on nbdkit not leaking a server's partial answer (patch 3) - add tests (patch 5) - other bug fixes I found while testing it - drop EOVERFLOW patch for now; it will be separate once upstream NBD protocol

...this failure to fall back to NBD_OPT_EXPORT_NAME */ - nbdkit_debug ("server lacks NBD_OPT_GO support"); - return 0; - default: - /* Unexpected. Either the server sent a legitimate error or an - unexpected reply, but either way, we can't connect. */ - if (NBD_REP_IS_ERR (reply.reply)) - if (reply.replylen) - nbdkit_error ("server rejected NBD_OPT_GO with %s: %s", - name_of_nbd_rep (reply.reply), (char *) buffer); - else - nbdkit_error ("server rejected NBD_OPT_GO with %s", -...

...this failure to fall back to NBD_OPT_EXPORT_NAME */ - nbdkit_debug ("server lacks NBD_OPT_GO support"); - return 0; - default: - /* Unexpected. Either the server sent a legitimate error or an - unexpected reply, but either way, we can't connect. */ - if (NBD_REP_IS_ERR (reply.reply)) - if (reply.replylen) - nbdkit_error ("server rejected NBD_OPT_GO with %s: %s", - name_of_nbd_rep (reply.reply), (char *) buffer); - else - nbdkit_error ("server rejected NBD_OPT_GO with %s", -...

...this failure to fall back to NBD_OPT_EXPORT_NAME */ - nbdkit_debug ("server lacks NBD_OPT_GO support"); - return 0; - default: - /* Unexpected. Either the server sent a legitimate error or an - unexpected reply, but either way, we can't connect. */ - if (NBD_REP_IS_ERR (reply.reply)) - if (reply.replylen) - nbdkit_error ("server rejected NBD_OPT_GO with %s: %s", - name_of_nbd_rep (reply.reply), (char *) buffer); - else - nbdkit_error ("server rejected NBD_OPT_GO with %s", -...

Patch 1 played with an early draft of Rich's Fedora 30 libnbd package: https://bugzilla.redhat.com/show_bug.cgi?id=1713767#c17 Note that comment 21 provides a newer package 0.1.1-1 with a different API; and that libnbd has more unreleased API changes in the pipeline (whether that will be called 0.2 or 0.1.2); so we'll have to tweak things based on what is actually available in distros.

libnbd-0.1.2-1 is now available in Fedora 29/30 updates-testing, although it was not compiled against libxml2 so it lacks uri support (I ended up testing patch 4 with a self-built libnbd). Diffs since v1 - rebase to master, bump from libnbd 0.1 to 0.1.2, add URI support, better timing results Still not done - patch 5 needs associated tests Eric Blake (5): nbd: Check for libnbd nbd:

libnbd-0.1.4-1 is now available in Fedora 29/30 updates testing. Diffs since v2 - rebase to master, bump from libnbd 0.1.2 to 0.1.3+, add tests to TLS usage which flushed out the need to turn relative pathnames into absolute, doc tweaks Now that the testsuite covers TLS and libnbd has been fixed to provide the things I found lacking when developing v2, I'm leaning towards pushing this on

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must