search for: option_reply

...load = sizeof h->sbuf.or.payload; - switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; case 0: - /* Read the following payload if it is short enough to fit in the - * static buffer. If it's too long, skip it. - */ - len = be32toh (h->sbuf.or.option_reply.replylen); - if (len <= maxpayload) - h->rbuf = &h->sbuf.or.payload; - else - h->rbuf = NULL; - h->rlen = len; + if (prepare_for_reply_payload (h, NBD_OPT_GO) == -1) { + SET_NEXT_STATE (%.DEAD); + return -1; + } SET_NEXT_STATE (%RECV_REPLY_...

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

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

Proposal for new APIs to list exports. The general shape of the API can probably best be seen from the examples/list-exports.c example. Rich.

Some of these affect attempts to connect to older qemu-nbd versions, some of them were triggered by manual 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

...uf = &h->sbuf; + h->wlen = sizeof (h->sbuf.option); + SET_NEXT_STATE (%SEND); + return 0; + + NEWSTYLE.OPT_LIST.SEND: + switch (send_from_wbuf (h)) { + case -1: SET_NEXT_STATE (%.DEAD); return 0; + case 0: + h->rbuf = &h->sbuf; + h->rlen = sizeof (h->sbuf.or.option_reply); + SET_NEXT_STATE (%RECV_REPLY); + } + return 0; + + NEWSTYLE.OPT_LIST.RECV_REPLY: + switch (recv_into_rbuf (h)) { + case -1: SET_NEXT_STATE (%.DEAD); return 0; + case 0: + if (prepare_for_reply_payload (h, NBD_OPT_LIST) == -1) { + SET_NEXT_STATE (%.DEAD); + return 0; + } +...

...ck an unexpected server reply. If it is an error, log any > + * message from the server and return 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.payloa...

...le *h, uint32_t opt) return 0; } +/* Check an unexpected server reply. If it is an error, log any + * message from the server and return 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); + if (len > NBD_MAX_STRING)...

.....97f25f8 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -106,6 +106,8 @@ uint32_t reply; uint32_t len; const size_t maxpayload = sizeof conn->sbuf.or.payload; + uint64_t exportsize; + uint16_t eflags; magic = be64toh (conn->sbuf.or.option_reply.magic); option = be32toh (conn->sbuf.or.option_reply.option); @@ -129,14 +131,11 @@ if (len <= maxpayload /* see RECV_NEWSTYLE_OPT_GO_REPLY */) { if (len >= sizeof conn->sbuf.or.payload.export) { if (be16toh (conn->sbuf.or.payload.export.info) == NBD_INFO_EXPO...

...nerator/states-newstyle-opt-go.c index 0568695..95cc041 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -124,6 +124,7 @@ STATE_MACHINE { uint64_t exportsize; uint16_t eflags; uint32_t min, pref, max; + int err; reply = be32toh (h->sbuf.or.option_reply.reply); len = be32toh (h->sbuf.or.option_reply.replylen); @@ -241,15 +242,21 @@ STATE_MACHINE { reply); } nbd_internal_reset_size_and_flags (h); - if (h->opt_mode) - SET_NEXT_STATE (%.NEGOTIATING); - else - SET_NEXT_STATE (%^PREPARE_OPT_ABORT); +...

...ONTEXT.SEND_QUERY: switch (send_from_wbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: @@ -133,13 +133,13 @@ STATE_MACHINE { } return 0; - NEWSTYLE.OPT_SET_META_CONTEXT.PREPARE_FOR_REPLY: + NEWSTYLE.OPT_META_CONTEXT.PREPARE_FOR_REPLY: h->rbuf = &h->sbuf.or.option_reply; h->rlen = sizeof h->sbuf.or.option_reply; SET_NEXT_STATE (%RECV_REPLY); return 0; - NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY: + NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY: switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: @@ -151,14 +151,14 @@ STA...

I'm posting this now, as I'm at the end of a workday and I got things working for manual experimentation. Still to do: - write interop tests for qemu-nbd and nbdkit (including my proposed patch addition of qemu-nbd -A to show qemu:allocation-depth) - figure out if we can make 'nbdinfo --map' use the new API to automatically select all contexts advertised by the server Eric Blake

...c @@ -64,9 +64,12 @@ prepare_for_reply_payload (struct nbd_handle *h, uint32_t opt) /* Read the following payload if it is short enough to fit in the * static buffer. If it's too long, skip it. */ - /* XXX Move to DEAD if len > MAX_REQUEST_SIZE */ len = be32toh (h->sbuf.or.option_reply.replylen); - if (len <= maxpayload) + if (len > MAX_REQUEST_SIZE) { + set_error (0, "handshake: invalid option reply length"); + return -1; + } + else if (len <= maxpayload) h->rbuf = &h->sbuf.or.payload; else h->rbuf = NULL; diff --git a/gener...

Per recent discussion here: https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00175

...nst size_t maxpayload = sizeof h->sbuf.or.payload.context; struct meta_context *meta_context; + uint32_t opt; + int err = 0; + + if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) + opt = h->opt_current; + else + opt = NBD_OPT_SET_META_CONTEXT; reply = be32toh (h->sbuf.or.option_reply.reply); len = be32toh (h->sbuf.or.option_reply.replylen); switch (reply) { case NBD_REP_ACK: /* End of list of replies. */ - SET_NEXT_STATE (%^OPT_GO.START); + if (opt == NBD_OPT_LIST_META_CONTEXT) { + SET_NEXT_STATE (%.NEGOTIATING); + CALL_CALLBACK (h->opt_...

...good addition on its own. > + NEWSTYLE.OPT_LIST.CHECK_REPLY: > + const size_t maxpayload = sizeof h->sbuf.or.payload.server; > + uint32_t reply; > + uint32_t len; > + uint32_t elen; > + char *name; > + char **new_exports; > + > + reply = be32toh (h->sbuf.or.option_reply.reply); > + len = be32toh (h->sbuf.or.option_reply.replylen); > + switch (reply) { > + case NBD_REP_SERVER: > + /* Got one export. */ > + if (len > maxpayload) > + debug (h, "skipping too large export name reply"); > + else { > + elen =...

An incremental improvement on top of listing exports. I still think it's worth experimenting with revisiting how our API for list mode should actually work [1] (so that we can reuse a single connection for both grabbing the list and finally using NBD_OPT_GO), but this change was easier to whip together while still thinking about that. [1]

...h->wbuf = &h->sbuf; @@ -68,7 +68,7 @@ NEWSTYLE.OPT_GO.SEND_NRINFOS: switch (send_from_wbuf (h)) { - case -1: SET_NEXT_STATE (%.DEAD); return -1; + case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: h->rbuf = &h->sbuf; h->rlen = sizeof h->sbuf.or.option_reply; @@ -78,11 +78,11 @@ NEWSTYLE.OPT_GO.RECV_REPLY: switch (recv_into_rbuf (h)) { - case -1: SET_NEXT_STATE (%.DEAD); return -1; + case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: if (prepare_for_reply_payload (h, NBD_OPT_GO) == -1) { SET_NEXT_STATE (%.DEAD); - return -1...

...;h->sbuf; + h->wlen = sizeof h->sbuf.option; + SET_NEXT_STATE (%SEND); + return 0; + + NEWSTYLE.OPT_EXTENDED_HEADERS.SEND: + switch (send_from_wbuf (h)) { + case -1: SET_NEXT_STATE (%.DEAD); return 0; + case 0: + h->rbuf = &h->sbuf; + h->rlen = sizeof h->sbuf.or.option_reply; + SET_NEXT_STATE (%RECV_REPLY); + } + return 0; + + NEWSTYLE.OPT_EXTENDED_HEADERS.RECV_REPLY: + switch (recv_into_rbuf (h)) { + case -1: SET_NEXT_STATE (%.DEAD); return 0; + case 0: + if (prepare_for_reply_payload (h, NBD_OPT_EXTENDED_HEADERS) == -1) { + SET_NEXT_STATE (%.DEAD); +...

...l opt_mode; uint8_t current_opt; - - /* Results of nbd_opt_list. */ - size_t nr_exports; - struct export *exports; + struct command_cb opt_cb; /* Full info mode. */ bool full_info; @@ -186,7 +192,7 @@ struct nbd_handle { union { struct { struct nbd_fixed_new_option_reply_server server; - char str[NBD_MAX_STRING * 2]; /* name and description */ + char str[NBD_MAX_STRING * 2 + 1]; /* name, description, NUL */ } __attribute__((packed)) server; struct nbd_fixed_new_option_reply_info_export export; struct nbd_fixed_new_option...