On Thu, May 25, 2023 at 08:01:05AM -0500, Eric Blake wrote:
> Very similar to the recent addition of nbd_opt_structured_reply,
> giving us fine-grained control over an extended headers request.
>
> Because nbdkit does not yet support extended headers, testsuite
> coverage is limited to interop testing with qemu-nbd. It shows that
> extended headers imply structured replies, regardless of which order
> the two are manually negotiated in.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> generator/API.ml | 79 +++++++--
> .../states-newstyle-opt-extended-headers.c | 30 +++-
> generator/states-newstyle.c | 3 +
> lib/opt.c | 44 +++++
> interop/Makefile.am | 6 +
> interop/opt-extended-headers.c | 153 ++++++++++++++++++
> interop/opt-extended-headers.sh | 29 ++++
> .gitignore | 1 +
> 8 files changed, 329 insertions(+), 16 deletions(-)
> create mode 100644 interop/opt-extended-headers.c
> create mode 100755 interop/opt-extended-headers.sh
>
> diff --git a/generator/API.ml b/generator/API.ml
> index 078f140f..85625bbd 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -825,6 +825,7 @@ "set_request_extended_headers", {
> if L<nbd_set_request_structured_replies(3)> is also set to false,
> since the use of extended headers implies structured replies.";
> see_also = [Link "get_request_extended_headers";
> + Link "opt_extended_headers";
> Link "set_handshake_flags"; Link "set_strict_mode";
> Link "get_extended_headers_negotiated";
> Link "zero"; Link "trim"; Link "cache";
> @@ -856,7 +857,9 @@ "get_extended_headers_negotiated", {
> shortdesc = "see if extended headers are in use";
> longdesc = "\
> After connecting you may call this to find out if the connection is
> -using extended headers.
> +using extended headers. Note that this setting is sticky; this
> +can return true even after a second L<nbd_opt_extended_headers(3)>
> +returns false because the server detected a duplicate request.
>
> When extended headers are not in use, commands are limited to a
> 32-bit length, even when the libnbd API uses a 64-bit parameter
> @@ -938,7 +941,7 @@ "get_structured_replies_negotiated", {
> attempted.";
> see_also = [Link "set_request_structured_replies";
> Link "get_request_structured_replies";
> - Link "opt_structured_reply";
> + Link "opt_structured_reply"; Link "opt_extended_headers";
> Link "get_protocol";
> Link "get_extended_headers_negotiated"];
> };
> @@ -1211,12 +1214,13 @@ "set_opt_mode", {
> newstyle server. This setting has no effect when connecting to an
> oldstyle server.
>
> -Note that libnbd defaults to attempting C<NBD_OPT_STARTTLS> and
> -C<NBD_OPT_STRUCTURED_REPLY> before letting you control remaining
> -negotiation steps; if you need control over these steps as well,
> -first set L<nbd_set_tls(3)> to C<LIBNBD_TLS_DISABLE> and
> -L<nbd_set_request_structured_replies(3)> to false before starting
> -the connection attempt.
> +Note that libnbd defaults to attempting C<NBD_OPT_STARTTLS>,
> +C<NBD_OPT_EXTENDED_HEADERS>, and C<NBD_OPT_STRUCTURED_REPLY>
> +before letting you control remaining negotiation steps; if you
> +need control over these steps as well, first set L<nbd_set_tls(3)>
> +to C<LIBNBD_TLS_DISABLE>, and L<nbd_set_request_extended_headers(3)>
> +or L<nbd_set_request_structured_replies(3)> to false, before
> +starting the connection attempt.
>
> When option mode is enabled, you have fine-grained control over which
> options are negotiated, compared to the default of the server
> @@ -1324,6 +1328,35 @@ "opt_starttls", {
> Link "supports_tls"]
> };
>
> + "opt_extended_headers", {
> + default_call with
> + args = []; ret = RBool;
> + permitted_states = [ Negotiating ];
> + shortdesc = "request the server to enable extended headers";
> + longdesc = "\
> +Request that the server use extended headers, by sending
> +C<NBD_OPT_EXTENDED_HEADERS>. This can only be used if
> +L<nbd_set_opt_mode(3)> enabled option mode; furthermore, libnbd
> +defaults to automatically requesting this unless you use
> +L<nbd_set_request_extended_headers(3)> or
> +L<nbd_set_request_structured_replies(3)> prior to connecting.
> +This function is mainly useful for integration testing of corner
> +cases in server handling.
> +
> +This function returns true if the server replies with success,
> +false if the server replies with an error, and fails only if
> +the server does not reply (such as for a loss of connection).
> +Note that some servers fail a second request as redundant;
> +libnbd assumes that once one request has succeeded, then
> +extended headers are supported (as visible by
> +L<nbd_get_extended_headers_negotiated(3)>) regardless if
> +later calls to this function return false. If this function
> +returns true, the use of structured replies is implied.";
> + see_also = [Link "set_opt_mode"; Link "aio_opt_extended_headers";
> + Link "opt_go"; Link "set_request_extended_headers";
> + Link "set_request_structured_replies"]
> + };
> +
> "opt_structured_reply", {
> default_call with
> args = []; ret = RBool;
> @@ -1345,7 +1378,9 @@ "opt_structured_reply", {
> libnbd assumes that once one request has succeeded, then
> structured replies are supported (as visible by
> L<nbd_get_structured_replies_negotiated(3)>) regardless if
> -later calls to this function return false.";
> +later calls to this function return false. Similarly, a
> +server may fail this request if extended headers are already
> +negotiated, since extended headers take priority.";
> see_also = [Link "set_opt_mode"; Link "aio_opt_structured_reply";
> Link "opt_go"; Link "set_request_structured_replies"]
> };
> @@ -3146,6 +3181,30 @@ "aio_opt_starttls", {
> see_also = [Link "set_opt_mode"; Link "opt_starttls"];
> };
>
> + "aio_opt_extended_headers", {
> + default_call with
> + args = [];
> + optargs = [ OClosure completion_closure ];
> + ret = RErr;
> + permitted_states = [ Negotiating ];
> + shortdesc = "request the server to enable extended headers";
> + longdesc = "\
> +Request that the server use extended headers, by sending
> +C<NBD_OPT_EXTENDED_HEADERS>. This behaves like the synchronous
> +counterpart L<nbd_opt_extended_headers(3)>, except that it does
> +not wait for the server's response.
> +
> +To determine when the request completes, wait for
> +L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
> +C<completion_callback> which will be invoked as described in
> +L<libnbd(3)/Completion callbacks>, except that it is automatically
> +retired regardless of return value. Note that detecting whether the
> +server returns an error (as is done by the return value of the
> +synchronous counterpart) is only possible with a completion
> +callback.";
> + see_also = [Link "set_opt_mode"; Link "opt_extended_headers"];
> + };
> +
> "aio_opt_structured_reply", {
> default_call with
> args = [];
> @@ -4122,6 +4181,8 @@ let first_version =
> "set_request_extended_headers", (1, 18);
> "get_request_extended_headers", (1, 18);
> "get_extended_headers_negotiated", (1, 18);
> + "opt_extended_headers", (1, 18);
> + "aio_opt_extended_headers", (1, 18);
>
> (* These calls are proposed for a future version of libnbd, but
> * have not been added to any released version so far.
> diff --git a/generator/states-newstyle-opt-extended-headers.c
> b/generator/states-newstyle-opt-extended-headers.c
> index 1ec25e97..5017a629 100644
> --- a/generator/states-newstyle-opt-extended-headers.c
> +++ b/generator/states-newstyle-opt-extended-headers.c
> @@ -21,11 +21,14 @@
> 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;
> + if (h->opt_current == NBD_OPT_EXTENDED_HEADERS)
> + assert (h->opt_mode);
> + else {
> + 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);
> @@ -68,6 +71,7 @@ NEWSTYLE.OPT_EXTENDED_HEADERS.RECV_REPLY_PAYLOAD:
>
> NEWSTYLE.OPT_EXTENDED_HEADERS.CHECK_REPLY:
> uint32_t reply;
> + int err = ENOTSUP;
>
> reply = be32toh (h->sbuf.or.option_reply.reply);
> switch (reply) {
> @@ -76,19 +80,31 @@ NEWSTYLE.OPT_EXTENDED_HEADERS.CHECK_REPLY:
> h->extended_headers = true;
> /* Extended headers trump structured replies, so skip ahead. */
> h->structured_replies = true;
> + err = 0;
> break;
> + case NBD_REP_ERR_INVALID:
> + err = EINVAL;
> + /* fallthrough */
> default:
> if (handle_reply_error (h) == -1) {
> SET_NEXT_STATE (%.DEAD);
> return 0;
> }
>
> - debug (h, "extended headers are not supported by this server");
> + if (h->extended_headers)
> + debug (h, "extended headers already negotiated");
> + else
> + debug (h, "extended headers are not supported by this server");
> break;
> }
>
> /* Next option. */
> - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
> + if (h->opt_current == NBD_OPT_EXTENDED_HEADERS)
> + SET_NEXT_STATE (%.NEGOTIATING);
> + else
> + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
> + CALL_CALLBACK (h->opt_cb.completion, &err);
> + nbd_internal_free_option (h);
> return 0;
>
> } /* END STATE MACHINE */
> diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
> index ad5bbf72..45893a8b 100644
> --- a/generator/states-newstyle.c
> +++ b/generator/states-newstyle.c
> @@ -146,6 +146,9 @@ NEWSTYLE.START:
> case NBD_OPT_STRUCTURED_REPLY:
> SET_NEXT_STATE (%OPT_STRUCTURED_REPLY.START);
> return 0;
> + case NBD_OPT_EXTENDED_HEADERS:
> + SET_NEXT_STATE (%OPT_EXTENDED_HEADERS.START);
> + return 0;
> case NBD_OPT_STARTTLS:
> SET_NEXT_STATE (%OPT_STARTTLS.START);
> return 0;
> diff --git a/lib/opt.c b/lib/opt.c
> index f58d5e19..d48acdd1 100644
> --- a/lib/opt.c
> +++ b/lib/opt.c
> @@ -164,6 +164,31 @@ nbd_unlocked_opt_starttls (struct nbd_handle *h)
> return r;
> }
>
> +/* Issue NBD_OPT_EXTENDED_HEADERS and wait for the reply. */
> +int
> +nbd_unlocked_opt_extended_headers (struct nbd_handle *h)
> +{
> + int err;
> + nbd_completion_callback c = { .callback = go_complete, .user_data = &err };
> + int r = nbd_unlocked_aio_opt_extended_headers (h, &c);
> +
> + if (r == -1)
> + return r;
> +
> + r = wait_for_option (h);
> + if (r == 0) {
> + if (nbd_internal_is_state_negotiating (get_next_state (h)))
> + r = err == 0;
> + else {
> + assert (nbd_internal_is_state_dead (get_next_state (h)));
> + set_error (err,
> + "failed to get response to opt_extended_headers request");
> + r = -1;
> + }
> + }
> + return r;
> +}
> +
> /* Issue NBD_OPT_STRUCTURED_REPLY and wait for the reply. */
> int
> nbd_unlocked_opt_structured_reply (struct nbd_handle *h)
> @@ -386,6 +411,25 @@ nbd_unlocked_aio_opt_starttls (struct nbd_handle *h,
> #endif
> }
>
> +/* Issue NBD_OPT_EXTENDED_HEADERS without waiting. */
> +int
> +nbd_unlocked_aio_opt_extended_headers (struct nbd_handle *h,
> + nbd_completion_callback *complete)
> +{
> + if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
> + set_error (ENOTSUP, "server is not using fixed newstyle protocol");
> + return -1;
> + }
> +
> + h->opt_current = NBD_OPT_EXTENDED_HEADERS;
> + h->opt_cb.completion = *complete;
> + SET_CALLBACK_TO_NULL (*complete);
> +
> + if (nbd_internal_run (h, cmd_issue) == -1)
> + debug (h, "option queued, ignoring state machine failure");
> + return 0;
> +}
> +
> /* Issue NBD_OPT_STRUCTURED_REPLY without waiting. */
> int
> nbd_unlocked_aio_opt_structured_reply (struct nbd_handle *h,
> diff --git a/interop/Makefile.am b/interop/Makefile.am
> index ec8ea0b2..3f81df0c 100644
> --- a/interop/Makefile.am
> +++ b/interop/Makefile.am
> @@ -25,6 +25,7 @@ EXTRA_DIST = \
> list-exports-test-dir/disk1 \
> list-exports-test-dir/disk2 \
> structured-read.sh \
> + opt-extended-headers.sh \
> $(NULL)
>
> TESTS_ENVIRONMENT = \
> @@ -134,6 +135,7 @@ check_PROGRAMS += \
> socket-activation-qemu-nbd \
> dirty-bitmap \
> structured-read \
> + opt-extended-headers \
> $(NULL)
> TESTS += \
> interop-qemu-nbd \
> @@ -144,6 +146,7 @@ TESTS += \
> dirty-bitmap.sh \
> structured-read.sh \
> interop-qemu-block-size.sh \
> + opt-extended-headers.sh \
> $(NULL)
>
> interop_qemu_nbd_SOURCES = \
> @@ -235,6 +238,9 @@ dirty_bitmap_LDADD = $(top_builddir)/lib/libnbd.la
> structured_read_SOURCES = structured-read.c
> structured_read_LDADD = $(top_builddir)/lib/libnbd.la
>
> +opt_extended_headers_SOURCES = opt-extended-headers.c
> +opt_extended_headers_LDADD = $(top_builddir)/lib/libnbd.la
> +
> endif HAVE_QEMU_NBD
>
> #----------------------------------------------------------------------
> diff --git a/interop/opt-extended-headers.c b/interop/opt-extended-headers.c
> new file mode 100644
> index 00000000..f50cd78f
> --- /dev/null
> +++ b/interop/opt-extended-headers.c
> @@ -0,0 +1,153 @@
> +/* NBD client library in userspace
> + * 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
> + */
> +
> +/* Demonstrate low-level use of nbd_opt_extended_headers(). */
> +
> +#include <config.h>
> +
> +#include <inttypes.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <sys/stat.h>
> +
> +#include <libnbd.h>
> +
> +#define check(got, exp) do_check (#got, got, exp)
> +
> +static void
> +do_check (const char *act, int64_t got, int64_t exp)
> +{
> + fprintf (stderr, "trying %s\n", act);
> + if (got == -1)
> + fprintf (stderr, "%s\n", nbd_get_error ());
> + else
> + fprintf (stderr, "succeeded, result %" PRId64 "\n", got);
> + if (got != exp) {
> + fprintf (stderr, "got %" PRId64 ", but expected %" PRId64 "\n", got,
> exp);
> + exit (EXIT_FAILURE);
> + }
> +}
> +
> +static int
> +cb (void *data, const char *metacontext, uint64_t offset,
> + nbd_extent *entries, size_t nr_entries, int *error)
> +{
> + /* If we got here, extents worked, implying at least structured replies */
> + bool *seen = data;
> +
> + *seen = true;
> + return 0;
> +}
> +
> +struct nbd_handle *
> +prep (bool sr, bool eh, char **cmd)
> +{
> + struct nbd_handle *nbd;
> +
> + nbd = nbd_create ();
> + if (nbd == NULL) {
> + fprintf (stderr, "%s\n", nbd_get_error ());
> + exit (EXIT_FAILURE);
> + }
> +
> + /* Connect to the server in opt mode, disable client-side failsafes so
> + * that we are testing server response even when client breaks protocol.
> + */
> + check (nbd_set_opt_mode (nbd, true), 0);
> + check (nbd_set_strict_mode (nbd, 0), 0);
> + check (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION), 0);
> + check (nbd_set_request_structured_replies (nbd, sr), 0);
> + check (nbd_set_request_extended_headers (nbd, eh), 0);
> + check (nbd_connect_systemd_socket_activation (nbd, cmd), 0);
> +
> + return nbd;
> +}
> +
> +void
> +cleanup (struct nbd_handle *nbd, bool extents_exp)
> +{
> + bool extents = false;
> +
> + check (nbd_opt_go (nbd), 0);
> + check (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION),
> + extents_exp);
> + check (nbd_block_status_64 (nbd, 512, 0,
> + (nbd_extent64_callback) { .callback = cb,
> + .user_data =
> &extents },
> + 0), extents_exp ? 0 : -1);
> + check (extents, extents_exp);
> + nbd_close (nbd);
> +}
> +
> +int
> +main (int argc, char *argv[])
> +{
> + struct nbd_handle *nbd;
> + int64_t bytes_sent;
> +
> + if (argc < 2) {
> + fprintf (stderr, "%s qemu-nbd [args ...]\n", argv[0]);
> + exit (EXIT_FAILURE);
> + }
> +
> + /* Default setup tries eh first, and skips sr request when eh works... */
> + nbd = prep (true, true, &argv[1]);
> + bytes_sent = nbd_stats_bytes_sent (nbd);
> + check (nbd_get_extended_headers_negotiated (nbd), true);
> + check (nbd_get_structured_replies_negotiated (nbd), true);
> + /* Duplicate eh request is no-op as redundant, but does not change state */
> + check (nbd_opt_extended_headers (nbd), false);
> + /* Trying sr after eh is no-op as redundant, but does not change state */
> + check (nbd_opt_structured_reply (nbd), false);
> + check (nbd_get_extended_headers_negotiated (nbd), true);
> + check (nbd_get_structured_replies_negotiated (nbd), true);
> + cleanup (nbd, true);
> +
> + /* ...which should result in the same amount of initial negotiation
> + * traffic as explicitly requesting just structured replies, albeit
> + * with different results on what got negotiated.
> + */
> + nbd = prep (true, false, &argv[1]);
> + check (nbd_stats_bytes_sent (nbd), bytes_sent);
> + check (nbd_get_extended_headers_negotiated (nbd), false);
> + check (nbd_get_structured_replies_negotiated (nbd), true);
> + cleanup (nbd, true);
> +
> + /* request_eh is ignored if request_sr is false. */
> + nbd = prep (false, true, &argv[1]);
> + check (nbd_get_extended_headers_negotiated (nbd), false);
> + check (nbd_get_structured_replies_negotiated (nbd), false);
> + cleanup (nbd, false);
> +
> + /* Swap order, requesting structured replies before extended headers */
> + nbd = prep (false, false, &argv[1]);
> + check (nbd_get_extended_headers_negotiated (nbd), false);
> + check (nbd_get_structured_replies_negotiated (nbd), false);
> + check (nbd_opt_structured_reply (nbd), true);
> + check (nbd_get_extended_headers_negotiated (nbd), false);
> + check (nbd_get_structured_replies_negotiated (nbd), true);
> + check (nbd_opt_extended_headers (nbd), true);
> + check (nbd_get_extended_headers_negotiated (nbd), true);
> + check (nbd_get_structured_replies_negotiated (nbd), true);
> + cleanup (nbd, true);
> +
> + exit (EXIT_SUCCESS);
> +}
> diff --git a/interop/opt-extended-headers.sh b/interop/opt-extended-headers.sh
> new file mode 100755
> index 00000000..41322f36
> --- /dev/null
> +++ b/interop/opt-extended-headers.sh
> @@ -0,0 +1,29 @@
> +#!/usr/bin/env bash
> +# nbd client library in userspace
> +# 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
> +
> +# Test low-level nbd_opt_extended_headers() details with qemu-nbd
> +
> +source ../tests/functions.sh
> +set -e
> +set -x
> +
> +requires qemu-nbd --version
> +requires nbdinfo --can extended-headers -- [ qemu-nbd -r -f raw "$0" ]
> +
> +# Run the test.
> +$VG ./opt-extended-headers qemu-nbd -r -f raw "$0"
> diff --git a/.gitignore b/.gitignore
> index bc7c2c37..24642748 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -118,6 +118,7 @@ Makefile.in
> /interop/list-exports-nbdkit
> /interop/list-exports-qemu-nbd
> /interop/nbd-server-tls.conf
> +/interop/opt-extended-headers
> /interop/requires.c
> /interop/socket-activation-nbdkit
> /interop/socket-activation-qemu-nbd
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
