On Thu, May 25, 2023 at 08:01:04AM -0500, Eric Blake wrote:
> This is the culmination of the previous patches' preparation work for
> using extended headers when possible. The new states in the state
> machine are copied extensively from our handling of
> OPT_STRUCTURED_REPLY. The next patch will then expose a new API
> nbd_opt_extended_headers() for manual control.
>
> At the same time I posted this patch, I had patches for qemu-nbd to
> support extended headers as server (nbdkit is a bit tougher). The
> next patches will add some interop tests that pass when using a new
> enough qemu-nbd, showing that we have cross-project interoperability
> and therefore an extension worth standardizing.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> generator/API.ml | 87 ++++++++---------
> generator/Makefile.am | 1 +
> generator/state_machine.ml | 41 ++++++++
> .../states-newstyle-opt-extended-headers.c | 94 +++++++++++++++++++
> generator/states-newstyle-opt-starttls.c | 6 +-
> 5 files changed, 184 insertions(+), 45 deletions(-)
> create mode 100644 generator/states-newstyle-opt-extended-headers.c
>
> diff --git a/generator/API.ml b/generator/API.ml
> index 7616990a..078f140f 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 meta contexts is
> -not always desirable: performing both info and go on the same
> -export works without needing to re-negotiate contexts on the
> -second call; integration testing of other servers may benefit
> -from manual invocation of L<nbd_opt_set_meta_context(3)> at
> -other times in the negotiation sequence; and even when using just
> -L<nbd_opt_info(3)>, it can be faster to collect the server's
> +the server supports structured replies or extended headers and
> +any contexts were registered by L<nbd_add_meta_context(3)>. The
> +default setting is true; however the extra step of negotiating
> +meta contexts is not always desirable: performing both info and
> +go on the same export works without needing to re-negotiate
> +contexts on the second call; integration testing of other servers
> +may benefit from manual invocation of L<nbd_opt_set_meta_context(3)>
> +at other times in the negotiation sequence; and even when using
> +just L<nbd_opt_info(3)>, it can be faster to collect the server's
> results by relying on the callback function passed to
> L<nbd_opt_list_meta_context(3)> than a series of post-process
> calls to L<nbd_can_meta_context(3)>.
>
> Note that this control has no effect if the server does not
> -negotiate structured replies, or if the client did not request
> -any contexts via L<nbd_add_meta_context(3)>. Setting this
> -control to false may cause L<nbd_block_status(3)> to fail.";
> +negotiate structured replies or extended headers, or if the
> +client did not request any contexts via L<nbd_add_meta_context(3)>.
> +Setting this control to false may cause L<nbd_block_status(3)>
> +to fail.";
> see_also = [Link "set_opt_mode"; Link "opt_go"; Link "opt_info";
> Link "opt_list_meta_context"; Link "opt_set_meta_context";
> Link "get_structured_replies_negotiated";
> @@ -1404,11 +1405,11 @@ "opt_info", {
> If successful, functions like L<nbd_is_read_only(3)> and
> L<nbd_get_size(3)> will report details about that export. If
> L<nbd_set_request_meta_context(3)> is set (the default) and
> -structured replies were negotiated, it is also valid to use
> -L<nbd_can_meta_context(3)> after this call. However, it may be
> -more efficient to clear that setting and manually utilize
> -L<nbd_opt_list_meta_context(3)> with its callback approach, for
> -learning which contexts an export supports. In general, if
> +structured replies or extended headers were negotiated, it is also
> +valid to use L<nbd_can_meta_context(3)> after this call. However,
> +it may be more efficient to clear that setting and manually
> +utilize L<nbd_opt_list_meta_context(3)> with its callback approach,
> +for learning which contexts an export supports. In general, if
> L<nbd_opt_go(3)> is called next, that call will likely succeed
> with the details remaining the same, although this is not
> guaranteed by all servers.
> @@ -1538,12 +1539,12 @@ "opt_set_meta_context", {
> recent L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.
> This can only be used if L<nbd_set_opt_mode(3)> enabled option
> mode. Normally, this function is redundant, as L<nbd_opt_go(3)>
> -automatically does the same task if structured replies have
> -already been negotiated. But manual control over meta context
> -requests can be useful for fine-grained testing of how a server
> -handles unusual negotiation sequences. Often, use of this
> -function is coupled with L<nbd_set_request_meta_context(3)> to
> -bypass the automatic context request normally performed by
> +automatically does the same task if structured replies or extended
> +headers have already been negotiated. But manual control over
> +meta context requests can be useful for fine-grained testing of
> +how a server handles unusual negotiation sequences. Often, use
> +of this function is coupled with L<nbd_set_request_meta_context(3)>
> +to bypass the automatic context request normally performed by
> L<nbd_opt_go(3)>.
>
> The NBD protocol allows a client to decide how many queries to ask
> @@ -1597,12 +1598,13 @@ "opt_set_meta_context_queries", {
> or L<nbd_connect_uri(3)>. This can only be used if
> L<nbd_set_opt_mode(3)> enabled option mode. Normally, this
> function is redundant, as L<nbd_opt_go(3)> automatically does
> -the same task if structured replies have already been
> -negotiated. But manual control over meta context requests can
> -be useful for fine-grained testing of how a server handles
> -unusual negotiation sequences. Often, use of this function is
> -coupled with L<nbd_set_request_meta_context(3)> to bypass the
> -automatic context request normally performed by L<nbd_opt_go(3)>.
> +the same task if structured replies or extended headers have
> +already been negotiated. But manual control over meta context
> +requests can be useful for fine-grained testing of how a server
> +handles unusual negotiation sequences. Often, use of this
> +function is coupled with L<nbd_set_request_meta_context(3)> to
> +bypass the automatic context request normally performed by
> +L<nbd_opt_go(3)>.
>
> The NBD protocol allows a client to decide how many queries to ask
> the server. This function takes an explicit list of queries; to
> @@ -3281,13 +3283,13 @@ "aio_opt_set_meta_context", {
> recent L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.
> This can only be used if L<nbd_set_opt_mode(3)> enabled option
> mode. Normally, this function is redundant, as L<nbd_opt_go(3)>
> -automatically does the same task if structured replies have
> -already been negotiated. But manual control over meta context
> -requests can be useful for fine-grained testing of how a server
> -handles unusual negotiation sequences. Often, use of this
> -function is coupled with L<nbd_set_request_meta_context(3)> to
> -bypass the automatic context request normally performed by
> -L<nbd_opt_go(3)>.
> +automatically does the same task if structured replies or
> +extended headers have already been negotiated. But manual
> +control over meta context requests can be useful for fine-grained
> +testing of how a server handles unusual negotiation sequences.
> +Often, use of this function is coupled with
> +L<nbd_set_request_meta_context(3)> to bypass the automatic
> +context request normally performed by L<nbd_opt_go(3)>.
>
> To determine when the request completes, wait for
> L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
> @@ -3314,12 +3316,13 @@ "aio_opt_set_meta_context_queries", {
> or L<nbd_connect_uri(3)>. This can only be used
> if L<nbd_set_opt_mode(3)> enabled option mode. Normally, this
> function is redundant, as L<nbd_opt_go(3)> automatically does
> -the same task if structured replies have already been
> -negotiated. But manual control over meta context requests can
> -be useful for fine-grained testing of how a server handles
> -unusual negotiation sequences. Often, use of this function is
> -coupled with L<nbd_set_request_meta_context(3)> to bypass the
> -automatic context request normally performed by L<nbd_opt_go(3)>.
> +the same task if structured replies or extended headers have
> +already been negotiated. But manual control over meta context
> +requests can be useful for fine-grained testing of how a server
> +handles unusual negotiation sequences. Often, use of this
> +function is coupled with L<nbd_set_request_meta_context(3)> to
> +bypass the automatic context request normally performed by
> +L<nbd_opt_go(3)>.
>
> To determine when the request completes, wait for
> L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
> diff --git a/generator/Makefile.am b/generator/Makefile.am
> index 91dbde5c..fc79b1b9 100644
> --- a/generator/Makefile.am
> +++ b/generator/Makefile.am
> @@ -30,6 +30,7 @@ states_code = \
> states-issue-command.c \
> states-magic.c \
> states-newstyle-opt-export-name.c \
> + states-newstyle-opt-extended-headers.c \
> states-newstyle-opt-list.c \
> states-newstyle-opt-go.c \
> states-newstyle-opt-meta-context.c \
> diff --git a/generator/state_machine.ml b/generator/state_machine.ml
> index 1f0d00b0..d09ac792 100644
> --- a/generator/state_machine.ml
> +++ b/generator/state_machine.ml
> @@ -297,6 +297,7 @@ and
> * NEGOTIATING after OPT_STRUCTURED_REPLY or any failed OPT_GO.
> *)
> Group ("OPT_STARTTLS", newstyle_opt_starttls_state_machine);
> + Group ("OPT_EXTENDED_HEADERS",
> newstyle_opt_extended_headers_state_machine);
> Group ("OPT_STRUCTURED_REPLY",
> newstyle_opt_structured_reply_state_machine);
> Group ("OPT_META_CONTEXT", newstyle_opt_meta_context_state_machine);
> Group ("OPT_GO", newstyle_opt_go_state_machine);
> @@ -441,6 +442,46 @@ and
> };
> ]
>
> +(* Fixed newstyle NBD_OPT_EXTENDED_HEADERS option.
> + * Implementation: generator/states-newstyle-opt-extended-headers.c
> + *)
> +and newstyle_opt_extended_headers_state_machine = [
> + State {
> + default_state with
> + name = "START";
> + comment = "Try to negotiate newstyle NBD_OPT_EXTENDED_HEADERS";
> + external_events = [];
> + };
> +
> + State {
> + default_state with
> + name = "SEND";
> + comment = "Send newstyle NBD_OPT_EXTENDED_HEADERS negotiation request";
> + external_events = [ NotifyWrite, "" ];
> + };
> +
> + State {
> + default_state with
> + name = "RECV_REPLY";
> + comment = "Receive newstyle NBD_OPT_EXTENDED_HEADERS option reply";
> + external_events = [ NotifyRead, "" ];
> + };
> +
> + State {
> + default_state with
> + name = "RECV_REPLY_PAYLOAD";
> + comment = "Receive any newstyle NBD_OPT_EXTENDED_HEADERS reply payload";
> + external_events = [ NotifyRead, "" ];
> + };
> +
> + State {
> + default_state with
> + name = "CHECK_REPLY";
> + comment = "Check newstyle NBD_OPT_EXTENDED_HEADERS option reply";
> + external_events = [];
> + };
> +]
> +
> (* Fixed newstyle NBD_OPT_STRUCTURED_REPLY option.
> * Implementation: generator/states-newstyle-opt-structured-reply.c
> *)
> diff --git a/generator/states-newstyle-opt-extended-headers.c
> b/generator/states-newstyle-opt-extended-headers.c
> new file mode 100644
> index 00000000..1ec25e97
> --- /dev/null
> +++ b/generator/states-newstyle-opt-extended-headers.c
> @@ -0,0 +1,94 @@
> +/* nbd client library in userspace: state machine
> + * Copyright Red Hat
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +/* State machine for negotiating NBD_OPT_EXTENDED_HEADERS. */
> +
> +STATE_MACHINE {
> + NEWSTYLE.OPT_EXTENDED_HEADERS.START:
> + assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
> + assert (h->opt_current != NBD_OPT_EXTENDED_HEADERS);
> + assert (CALLBACK_IS_NULL (h->opt_cb.completion));
> + if (!h->request_eh || !h->request_sr) {
> + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
> + return 0;
> + }
> +
> + h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
> + h->sbuf.option.option = htobe32 (NBD_OPT_EXTENDED_HEADERS);
> + h->sbuf.option.optlen = htobe32 (0);
> + h->chunks_sent++;
> + h->wbuf = &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);
> + return 0;
> + }
> + SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
> + }
> + return 0;
> +
> + NEWSTYLE.OPT_EXTENDED_HEADERS.RECV_REPLY_PAYLOAD:
> + switch (recv_into_rbuf (h)) {
> + case -1: SET_NEXT_STATE (%.DEAD); return 0;
> + case 0: SET_NEXT_STATE (%CHECK_REPLY);
> + }
> + return 0;
> +
> + NEWSTYLE.OPT_EXTENDED_HEADERS.CHECK_REPLY:
> + uint32_t reply;
> +
> + reply = be32toh (h->sbuf.or.option_reply.reply);
> + switch (reply) {
> + case NBD_REP_ACK:
> + debug (h, "negotiated extended headers on this connection");
> + h->extended_headers = true;
> + /* Extended headers trump structured replies, so skip ahead. */
> + h->structured_replies = true;
> + break;
> + default:
> + if (handle_reply_error (h) == -1) {
> + SET_NEXT_STATE (%.DEAD);
> + return 0;
> + }
> +
> + debug (h, "extended headers are not supported by this server");
> + break;
> + }
> +
> + /* Next option. */
> + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
> + return 0;
> +
> +} /* END STATE MACHINE */
> diff --git a/generator/states-newstyle-opt-starttls.c
> b/generator/states-newstyle-opt-starttls.c
> index e497548c..1e2997a3 100644
> --- a/generator/states-newstyle-opt-starttls.c
> +++ b/generator/states-newstyle-opt-starttls.c
> @@ -26,7 +26,7 @@ NEWSTYLE.OPT_STARTTLS.START:
> else {
> /* If TLS was not requested we skip this option and go to the next one.
> */
> if (h->tls == LIBNBD_TLS_DISABLE) {
> - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
> + SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START);
> return 0;
> }
> assert (CALLBACK_IS_NULL (h->opt_cb.completion));
> @@ -128,7 +128,7 @@ NEWSTYLE.OPT_STARTTLS.CHECK_REPLY:
> SET_NEXT_STATE (%.NEGOTIATING);
> else {
> debug (h, "continuing with unencrypted connection");
> - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
> + SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START);
> }
> return 0;
> }
> @@ -185,7 +185,7 @@ NEWSTYLE.OPT_STARTTLS.TLS_HANDSHAKE_DONE:
> if (h->opt_current == NBD_OPT_STARTTLS)
> SET_NEXT_STATE (%.NEGOTIATING);
> else
> - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
> + SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START);
> return 0;
>
> } /* END STATE MACHINE */
Seems pretty straightforward. We add a new state machine between
NEWSTYLE.OPT_STARTTLS and NEWSTYLE.OPT_STRUCTURED_REPLY which sends
the new extended headers option (if enabled).
Reviewed-by: Richard W.M. Jones <rjo...@redhat.com>
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
