Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 11/12] api: Add nbd_[aio_]opt_set_meta_context
Make it possible for the client to choose when to manually request meta contexts when using nbd_set_opt_mode(). By itself, this patch allows us to test how a server reacts to NBD_OPT_SET_META_CONTEXT without a previous negotiation of structured headers; then an upcoming patch will also add APIs for choosing when to negotiate OPT_STARTTLS and OPT_STRUCTURED_REPLY for even more fine-grained testing control. Most users won't need this API (the automatic defaults are good enough), but when writing a server, this level of integration testing can be useful. Overhaul the comment on OPT_META_CONTEXT.START to explain 5 different scenarios that this state group can be reached in, to better document which conditionals are in use later in the state machine: - h->current_opt == opt is a witness of whether we are doing a mandatory manual LIST/SET request with an expected callback and transition to NEGOTIATIONG, or a conditional SET request dependent on nbd_set_request_meta_context() and prior negotiation and transition to OPT_GO - h->current_opt == NBD_OPT_LIST_META_CONTEXT is a witness of whether we are stateless or modifying h->meta_contexts As this is a new API, the unit test in C is included in this patch; there is no point in splitting it as reordering the patches would not compile. However, porting the tests to other languages will be done in the next commit. --- Patches to add nbd_opt_starttls and nbd_opt_structured_reply are not written yet... --- generator/API.ml | 115 ++++++++- generator/states-newstyle-opt-meta-context.c | 38 +-- generator/states-newstyle.c | 1 + lib/opt.c | 47 +++- tests/Makefile.am | 9 + tests/opt-set-meta.c | 235 +++++++++++++++++++ .gitignore | 1 + 7 files changed, 420 insertions(+), 26 deletions(-) create mode 100644 tests/opt-set-meta.c diff --git a/generator/API.ml b/generator/API.ml index adafac6..24cbabc 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -762,17 +762,20 @@ "set_request_meta_context", { 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; 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)>. +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."; see_also = [Link "set_opt_mode"; Link "opt_go"; Link "opt_info"; - Link "opt_list_meta_context"; + Link "opt_list_meta_context"; Link "opt_set_meta_context"; Link "get_structured_replies_negotiated"; Link "get_request_meta_context"; Link "can_meta_context"]; }; @@ -1036,7 +1039,9 @@ "opt_go", { By default, libnbd will automatically request all meta contexts registered by L<nbd_add_meta_context(3)> as part of this call; but -this can be suppressed with L<nbd_set_request_meta_context(3)>. +this can be suppressed with L<nbd_set_request_meta_context(3)>, +particularly if L<nbd_opt_set_meta_context(3)> was used earlier +in the negotiation sequence. If this fails, the server may still be in negotiation, where it is possible to attempt another option such as a different export name; @@ -1044,7 +1049,8 @@ "opt_go", { example = Some "examples/list-exports.c"; see_also = [Link "set_opt_mode"; Link "aio_opt_go"; Link "opt_abort"; Link "set_export_name"; Link "connect_uri"; Link "opt_info"; - Link "add_meta_context"; Link "set_request_meta_context"]; + Link "add_meta_context"; Link "set_request_meta_context"; + Link "opt_set_meta_context"]; }; "opt_abort", { @@ -1179,7 +1185,64 @@ "opt_list_meta_context", { a server may send a lengthy list."; see_also = [Link "set_opt_mode"; Link "aio_opt_list_meta_context"; Link "add_meta_context"; Link "clear_meta_contexts"; - Link "opt_go"; Link "set_export_name"]; + Link "opt_go"; Link "set_export_name"; + Link "opt_set_meta_context"]; + }; + + "opt_set_meta_context", { + default_call with + args = [ Closure context_closure ]; ret = RInt; + permitted_states = [ Negotiating ]; + shortdesc = "request the server to select specific meta contexts"; + longdesc = "\ +Request that the server supply all recognized meta contexts +registered through prior calls to L<nbd_add_meta_context(3)>, in +conjunction with the export previously specified by the most +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)>. + +The NBD protocol allows a client to decide how many queries to ask +the server. Rather than taking that list of queries as a parameter +to this function, libnbd reuses the current list of requested meta +contexts as set by L<nbd_add_meta_context(3)>; you can use +L<nbd_clear_meta_contexts(3)> to set up a different list of queries. +Since this function is primarily designed for testing servers, libnbd +does not prevent the use of this function on an empty list or when +L<nbd_set_request_structured_replies(3)> has disabled structured +replies, in order to see how a server behaves. + +The C<context> function is called once per server reply, with any +C<user_data> passed to this function, and with C<name> supplied by +the server. Additionally, each server name will remain visible through +L<nbd_can_meta_context(3)> until the next attempt at +L<nbd_set_export_name(3)> or L<nbd_opt_set_meta_context(3)>, as +well as L<nbd_opt_go(3)> or L<nbd_opt_info(3)> that trigger an +automatic meta context request. Remember that it is not safe to +call any C<nbd_*> APIs from within the context of the callback +function. At present, the return value of the callback is +ignored, although a return of -1 should be avoided. + +For convenience, when this function succeeds, it returns the number +of replies returned by the server. + +Not all servers understand this request, and even when it is understood, +the server might intentionally send an empty list because it does not +support the requested context, or may encounter a failure after +delivering partial results. Thus, this function may succeed even when +no contexts are reported, or may fail but have a non-empty list."; + see_also = [Link "set_opt_mode"; Link "aio_opt_set_meta_context"; + Link "add_meta_context"; Link "clear_meta_contexts"; + Link "opt_go"; Link "set_export_name"; + Link "opt_list_meta_context"; Link "set_request_meta_context"; + Link "can_meta_context"]; }; "add_meta_context", { @@ -1850,7 +1913,7 @@ "can_meta_context", { see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "add_meta_context"; Link "block_status"; Link "aio_block_status"; - Link "set_request_meta_context"]; + Link "set_request_meta_context"; Link "opt_set_meta_context"]; }; "get_protocol", { @@ -2592,6 +2655,38 @@ "aio_opt_list_meta_context", { see_also = [Link "set_opt_mode"; Link "opt_list_meta_context"]; }; + "aio_opt_set_meta_context", { + default_call with + args = [ Closure context_closure ]; ret = RInt; + optargs = [ OClosure completion_closure ]; + permitted_states = [ Negotiating ]; + shortdesc = "request the server to select specific meta contexts"; + longdesc = "\ +Request that the server supply all recognized meta contexts +registered through prior calls to L<nbd_add_meta_context(3)>, in +conjunction with the export previously specified by the most +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)>. + +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_set_meta_context"]; + }; + "aio_pread", { default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset" ]; @@ -3302,6 +3397,8 @@ let first_version (* Added in 1.13.x development cycle, will be stable and supported in 1.14. *) "set_request_meta_context", (1, 14); "get_request_meta_context", (1, 14); + "opt_set_meta_context", (1, 14); + "aio_opt_set_meta_context", (1, 14); (* 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-meta-context.c b/generator/states-newstyle-opt-meta-context.c index a281b05..03b63c6 100644 --- a/generator/states-newstyle-opt-meta-context.c +++ b/generator/states-newstyle-opt-meta-context.c @@ -34,10 +34,14 @@ STATE_MACHINE { opt = h->opt_current; } else { - assert (CALLBACK_IS_NULL (h->opt_cb.fn.context)); + if (h->opt_current == NBD_OPT_SET_META_CONTEXT) + assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context)); + else { + assert (CALLBACK_IS_NULL (h->opt_cb.fn.context)); + nbd_internal_reset_size_and_flags (h); + } opt = NBD_OPT_SET_META_CONTEXT; - nbd_internal_reset_size_and_flags (h); - if (h->request_meta) { + if (h->request_meta || h->opt_current == opt) { for (i = 0; i < h->meta_contexts.len; ++i) free (h->meta_contexts.ptr[i].name); meta_vector_reset (&h->meta_contexts); @@ -194,15 +198,15 @@ STATE_MACHINE { len = be32toh (h->sbuf.or.option_reply.replylen); switch (reply) { case NBD_REP_ACK: /* End of list of replies. */ - if (opt == NBD_OPT_LIST_META_CONTEXT) { - SET_NEXT_STATE (%.NEGOTIATING); - CALL_CALLBACK (h->opt_cb.completion, &err); - nbd_internal_free_option (h); - } - else { - SET_NEXT_STATE (%^OPT_GO.START); + if (opt == NBD_OPT_SET_META_CONTEXT) h->meta_valid = true; + if (opt == h->opt_current) { + SET_NEXT_STATE (%.NEGOTIATING); + CALL_CALLBACK (h->opt_cb.completion, &err); + nbd_internal_free_option (h); } + else + SET_NEXT_STATE (%^OPT_GO.START); break; case NBD_REP_META_CONTEXT: /* A context. */ if (len > maxpayload) @@ -221,10 +225,10 @@ STATE_MACHINE { } debug (h, "negotiated %s with context ID %" PRIu32, meta_context.name, meta_context.context_id); - if (opt == NBD_OPT_LIST_META_CONTEXT) { + if (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context)) CALL_CALLBACK (h->opt_cb.fn.context, meta_context.name); + if (opt == NBD_OPT_LIST_META_CONTEXT) free (meta_context.name); - } else if (meta_vector_append (&h->meta_contexts, meta_context) == -1) { set_error (errno, "realloc"); free (meta_context.name); @@ -235,25 +239,27 @@ STATE_MACHINE { SET_NEXT_STATE (%PREPARE_FOR_REPLY); break; default: - /* Anything else is an error, ignore it for SET, report it for LIST */ + /* Anything else is an error, report it for explicit LIST/SET, ignore it + * for automatic progress (nbd_connect_*, nbd_opt_info, nbd_opt_go). + */ if (handle_reply_error (h) == -1) { SET_NEXT_STATE (%.DEAD); return 0; } - if (opt == NBD_OPT_LIST_META_CONTEXT) { + if (opt == h->opt_current) { /* XXX Should we decode specific expected errors, like * REP_ERR_UNKNOWN to ENOENT or REP_ERR_TOO_BIG to ERANGE? */ err = ENOTSUP; set_error (err, "unexpected response, possibly the server does not " - "support listing contexts"); + "support meta contexts"); CALL_CALLBACK (h->opt_cb.completion, &err); nbd_internal_free_option (h); SET_NEXT_STATE (%.NEGOTIATING); } else { - debug (h, "handshake: unexpected error from " + debug (h, "handshake: ignoring unexpected error from " "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")", reply); SET_NEXT_STATE (%^OPT_GO.START); } diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index 41e7cc3..62f2372 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -140,6 +140,7 @@ STATE_MACHINE { SET_NEXT_STATE (%PREPARE_OPT_ABORT); return 0; case NBD_OPT_LIST_META_CONTEXT: + case NBD_OPT_SET_META_CONTEXT: SET_NEXT_STATE (%OPT_META_CONTEXT.START); return 0; case 0: diff --git a/lib/opt.c b/lib/opt.c index d9114f4..2ad8c9e 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -32,7 +32,8 @@ nbd_internal_free_option (struct nbd_handle *h) { if (h->opt_current == NBD_OPT_LIST) FREE_CALLBACK (h->opt_cb.fn.list); - else if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) + else if (h->opt_current == NBD_OPT_LIST_META_CONTEXT || + h->opt_current == NBD_OPT_SET_META_CONTEXT) FREE_CALLBACK (h->opt_cb.fn.context); FREE_CALLBACK (h->opt_cb.completion); } @@ -215,6 +216,28 @@ nbd_unlocked_opt_list_meta_context (struct nbd_handle *h, return s.count; } +/* Issue NBD_OPT_SET_META_CONTEXT and wait for the reply. */ +int +nbd_unlocked_opt_set_meta_context (struct nbd_handle *h, + nbd_context_callback *context) +{ + struct context_helper s = { .context = *context }; + nbd_context_callback l = { .callback = context_visitor, .user_data = &s }; + nbd_completion_callback c = { .callback = context_complete, .user_data = &s }; + + if (nbd_unlocked_aio_opt_set_meta_context (h, &l, &c) == -1) + return -1; + + SET_CALLBACK_TO_NULL (*context); + if (wait_for_option (h) == -1) + return -1; + if (s.err) { + set_error (s.err, "server replied with error to set meta context request"); + return -1; + } + return s.count; +} + /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ int nbd_unlocked_aio_opt_go (struct nbd_handle *h, @@ -301,3 +324,25 @@ nbd_unlocked_aio_opt_list_meta_context (struct nbd_handle *h, debug (h, "option queued, ignoring state machine failure"); return 0; } + +/* Issue NBD_OPT_SET_META_CONTEXT without waiting. */ +int +nbd_unlocked_aio_opt_set_meta_context (struct nbd_handle *h, + nbd_context_callback *context, + 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; + } + + assert (CALLBACK_IS_NULL (h->opt_cb.fn.context)); + h->opt_cb.fn.context = *context; + SET_CALLBACK_TO_NULL (*context); + h->opt_cb.completion = *complete; + SET_CALLBACK_TO_NULL (*complete); + h->opt_current = NBD_OPT_SET_META_CONTEXT; + if (nbd_internal_run (h, cmd_issue) == -1) + debug (h, "option queued, ignoring state machine failure"); + return 0; +} diff --git a/tests/Makefile.am b/tests/Makefile.am index 7fc7dec..1c6f137 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -213,6 +213,7 @@ check_PROGRAMS += \ opt-list \ opt-info \ opt-list-meta \ + opt-set-meta \ connect-systemd-socket-activation \ connect-unix \ connect-tcp \ @@ -278,6 +279,7 @@ TESTS += \ opt-list \ opt-info \ opt-list-meta \ + opt-set-meta \ connect-systemd-socket-activation \ connect-unix \ connect-tcp \ @@ -533,6 +535,13 @@ opt_info_LDADD = $(top_builddir)/lib/libnbd.la opt_list_meta_SOURCES = opt-list-meta.c opt_list_meta_LDADD = $(top_builddir)/lib/libnbd.la +opt_set_meta_SOURCES = opt-set-meta.c +opt_set_meta_CPPFLAGS = \ + $(AM_CPPFLAGS) \ + -I$(top_srcdir)/common/include \ + $(NULL) +opt_set_meta_LDADD = $(top_builddir)/lib/libnbd.la + connect_systemd_socket_activation_SOURCES = \ connect-systemd-socket-activation.c \ requires.c \ diff --git a/tests/opt-set-meta.c b/tests/opt-set-meta.c new file mode 100644 index 0000000..01c9604 --- /dev/null +++ b/tests/opt-set-meta.c @@ -0,0 +1,235 @@ +/* NBD client library in userspace + * Copyright (C) 2013-2022 Red Hat Inc. + * + * 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 behavior of nbd_opt_set_meta_context. */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <stdbool.h> +#include <inttypes.h> +#include <string.h> +#include <errno.h> + +#include <libnbd.h> + +#include "array-size.h" + +struct progress { + int count; + bool seen; +}; + +static int +check (void *user_data, const char *name) +{ + struct progress *p = user_data; + + p->count++; + if (strcmp (name, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) + p->seen = true; + return 0; +} + +int +main (int argc, char *argv[]) +{ + struct nbd_handle *nbd; + int r; + struct progress p; + /* Leave room for --no-sr in second process */ + char *args[] = { "nbdkit", "-s", "--exit-with-parent", "-v", + "memory", "size=1M", NULL, NULL }; + char *tmp; + + /* First process, with structured replies. Get into negotiating state. */ + nbd = nbd_create (); + if (nbd == NULL || + nbd_set_opt_mode (nbd, true) == -1 || + nbd_connect_command (nbd, args) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + /* No contexts negotiated yet; can_meta should be error if any requested */ + if ((r = nbd_get_structured_replies_negotiated (nbd)) != 1) { + fprintf (stderr, "structured replies got %d, expected 1\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) { + fprintf (stderr, "can_meta_context got %d, expected 0\n", r); + exit (EXIT_FAILURE); + } + if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != -1) { + fprintf (stderr, "can_meta_context got %d, expected -1\n", r); + exit (EXIT_FAILURE); + } + + /* FIXME: Once nbd_opt_structured_reply exists, check that set before + * SR fails server-side, then enable SR for rest of process. + */ + + /* nbdkit does not match wildcard for SET, even though it does for LIST */ + if (nbd_clear_meta_contexts (nbd) == -1 || + nbd_add_meta_context (nbd, "base:") == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + p = (struct progress) { .count = 0 }; + r = nbd_opt_set_meta_context (nbd, + (nbd_context_callback) { .callback = check, + .user_data = &p}); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (r != p.count || r != 0 || p.seen) { + fprintf (stderr, "inconsistent return value %d, expected %d\n", r, p.count); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) { + fprintf (stderr, "can_meta_context got %d, expected 0\n", r); + exit (EXIT_FAILURE); + } + + /* Negotiating with no contexts is not an error, but selects nothing */ + r = nbd_clear_meta_contexts (nbd); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + p = (struct progress) { .count = 0 }; + r = nbd_opt_set_meta_context (nbd, + (nbd_context_callback) { .callback = check, + .user_data = &p}); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (r != 0 || p.count || p.seen) { + fprintf (stderr, "expecting set_meta to select nothing\n"); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) { + fprintf (stderr, "can_meta_context got %d, expected 0\n", r); + exit (EXIT_FAILURE); + } + + /* Request 2 with expectation of 1; with set_request_meta_context off */ + if (nbd_add_meta_context (nbd, "x-nosuch:context") == -1 || + nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1 || + nbd_set_request_meta_context (nbd, 0) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + p = (struct progress) { .count = 0 }; + r = nbd_opt_set_meta_context (nbd, + (nbd_context_callback) { .callback = check, + .user_data = &p}); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (r != 1 || p.count != 1 || !p.seen) { + fprintf (stderr, "expecting one context, got %d\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) { + fprintf (stderr, "can_meta_context got %d, expected 1\n", r); + exit (EXIT_FAILURE); + } + + /* Transition to transmission phase; our last set should remain active */ + if (nbd_clear_meta_contexts (nbd) == -1 || + nbd_add_meta_context (nbd, "x-nosuch:context") == -1 || + nbd_opt_go (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) { + fprintf (stderr, "can_meta_context got %d, expected 1\n", r); + exit (EXIT_FAILURE); + } + + /* Now too late to set; but should not lose earlier state */ + p = (struct progress) { .count = 0 }; + r = nbd_opt_set_meta_context (nbd, + (nbd_context_callback) { .callback = check, + .user_data = &p}); + if (r != -1 || p.count || p.seen) { + fprintf (stderr, "expecting set_meta failure\n"); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) { + fprintf (stderr, "can_meta_context got %d, expected 1\n", r); + exit (EXIT_FAILURE); + } + + nbd_shutdown (nbd, 0); + nbd_close (nbd); + + /* Second process, this time without structured replies server-side. */ + args[ARRAY_SIZE (args) - 2] = (char *) "--no-sr"; + nbd = nbd_create (); + if (nbd == NULL || + nbd_set_opt_mode (nbd, true) == -1 || + nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1 || + nbd_connect_command (nbd, args) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_structured_replies_negotiated (nbd)) != 0) { + fprintf (stderr, "structured replies got %d, expected 0\n", r); + exit (EXIT_FAILURE); + } + + /* Expect server-side failure here */ + p = (struct progress) { .count = 0 }; + r = nbd_opt_set_meta_context (nbd, + (nbd_context_callback) { .callback = check, + .user_data = &p}); + if (r != -1 || p.count || p.seen) { + fprintf (stderr, "expecting set_meta failure\n"); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != -1) { + fprintf (stderr, "can_meta_context got %d, expected -1\n", r); + exit (EXIT_FAILURE); + } + + /* Even though can_meta fails after failed SET, it returns 0 after go */ + if (nbd_opt_go (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) { + fprintf (stderr, "can_meta_context got %d, expected 0\n", r); + exit (EXIT_FAILURE); + } + + nbd_shutdown (nbd, 0); + nbd_close (nbd); + + exit (EXIT_SUCCESS); +} diff --git a/.gitignore b/.gitignore index bd4650d..5603de5 100644 --- a/.gitignore +++ b/.gitignore @@ -231,6 +231,7 @@ Makefile.in /tests/opt-info /tests/opt-list /tests/opt-list-meta +/tests/opt-set-meta /tests/pki/ /tests/pread-initialize /tests/private-data -- 2.37.2
Eric Blake
2022-Aug-31 18:28 UTC
[Libguestfs] [libnbd PATCH v2 11/12] api: Add nbd_[aio_]opt_set_meta_context
On Wed, Aug 31, 2022 at 09:39:27AM -0500, Eric Blake wrote:> Make it possible for the client to choose when to manually request > meta contexts when using nbd_set_opt_mode(). By itself, this patch > allows us to test how a server reacts to NBD_OPT_SET_META_CONTEXT > without a previous negotiation of structured headers; then an upcoming > patch will also add APIs for choosing when to negotiate OPT_STARTTLS > and OPT_STRUCTURED_REPLY for even more fine-grained testing control. > Most users won't need this API (the automatic defaults are good > enough), but when writing a server, this level of integration testing > can be useful. > > Overhaul the comment on OPT_META_CONTEXT.START to explain 5 different > scenarios that this state group can be reached in, to better document > which conditionals are in use later in the state machine: > - h->current_opt == opt is a witness of whether we are doing a > mandatory manual LIST/SET request with an expected callback and > transition to NEGOTIATIONG, or a conditional SET request dependent on > nbd_set_request_meta_context() and prior negotiation and transition to > OPT_GO > - h->current_opt == NBD_OPT_LIST_META_CONTEXT is a witness of whether > we are stateless or modifying h->meta_contexts > > As this is a new API, the unit test in C is included in this patch; > there is no point in splitting it as reordering the patches would not > compile. However, porting the tests to other languages will be done > in the next commit. > > ---Also intended to be squashed into this patch: diff --git a/generator/states-newstyle-opt-meta-context.c b/generator/states-newstyle-opt-meta-context.c index 03b63c6..49e08bc 100644 --- a/generator/states-newstyle-opt-meta-context.c +++ b/generator/states-newstyle-opt-meta-context.c @@ -23,9 +23,24 @@ STATE_MACHINE { size_t i; uint32_t len, opt; - /* If the server doesn't support SRs then we must skip this group. - * Also we skip the group if the client didn't request any metadata - * contexts, when doing SET (but an empty LIST is okay). + /* This state group is reached from: + * h->opt_mode == false (h->current_opt == 0): + * nbd_connect_*() + * -> conditionally use SET, next state OPT_GO + * h->opt_mode == true (h->current_opt matches calling API): + * nbd_opt_info(), nbd_opt_go() + * -> conditionally use SET, next state OPT_GO + * nbd_opt_list_meta_context() + * -> unconditionally use LIST, next state NEGOTIATING + * nbd_opt_set_meta_context() + * -> unconditionally use SET, next state NEGOTIATING + * + * If the next state is OPT_GO, we want to clear h->exportsize and friends. + * If SET is conditional, we skip it if h->request_meta is false, if + * structured replies were not negotiated, or if no contexts to request. + * There is a callback if and only if the command is unconditional. + * If we use SET, we clear h->meta_contexts and rebuild it. + * If we use LIST, we are stateless. */ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) { -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Laszlo Ersek
2022-Sep-09 07:02 UTC
[Libguestfs] [libnbd PATCH v2 11/12] api: Add nbd_[aio_]opt_set_meta_context
On 08/31/22 20:28, Eric Blake wrote:> On Wed, Aug 31, 2022 at 09:39:27AM -0500, Eric Blake wrote: >> Make it possible for the client to choose when to manually request >> meta contexts when using nbd_set_opt_mode(). By itself, this patch >> allows us to test how a server reacts to NBD_OPT_SET_META_CONTEXT >> without a previous negotiation of structured headers; then an upcoming >> patch will also add APIs for choosing when to negotiate OPT_STARTTLS >> and OPT_STRUCTURED_REPLY for even more fine-grained testing control. >> Most users won't need this API (the automatic defaults are good >> enough), but when writing a server, this level of integration testing >> can be useful. >> >> Overhaul the comment on OPT_META_CONTEXT.START to explain 5 different >> scenarios that this state group can be reached in, to better document >> which conditionals are in use later in the state machine: >> - h->current_opt == opt is a witness of whether we are doing a >> mandatory manual LIST/SET request with an expected callback and >> transition to NEGOTIATIONG, or a conditional SET request dependent on >> nbd_set_request_meta_context() and prior negotiation and transition to >> OPT_GO >> - h->current_opt == NBD_OPT_LIST_META_CONTEXT is a witness of whether >> we are stateless or modifying h->meta_contexts >> >> As this is a new API, the unit test in C is included in this patch; >> there is no point in splitting it as reordering the patches would not >> compile. However, porting the tests to other languages will be done >> in the next commit. >> >> --- > > Also intended to be squashed into this patch: > > diff --git a/generator/states-newstyle-opt-meta-context.c b/generator/states-newstyle-opt-meta-context.c > index 03b63c6..49e08bc 100644 > --- a/generator/states-newstyle-opt-meta-context.c > +++ b/generator/states-newstyle-opt-meta-context.c > @@ -23,9 +23,24 @@ STATE_MACHINE { > size_t i; > uint32_t len, opt; > > - /* If the server doesn't support SRs then we must skip this group. > - * Also we skip the group if the client didn't request any metadata > - * contexts, when doing SET (but an empty LIST is okay). > + /* This state group is reached from: > + * h->opt_mode == false (h->current_opt == 0): > + * nbd_connect_*() > + * -> conditionally use SET, next state OPT_GO > + * h->opt_mode == true (h->current_opt matches calling API): > + * nbd_opt_info(), nbd_opt_go() > + * -> conditionally use SET, next state OPT_GO > + * nbd_opt_list_meta_context() > + * -> unconditionally use LIST, next state NEGOTIATING > + * nbd_opt_set_meta_context() > + * -> unconditionally use SET, next state NEGOTIATING > + * > + * If the next state is OPT_GO, we want to clear h->exportsize and friends. > + * If SET is conditional, we skip it if h->request_meta is false, if > + * structured replies were not negotiated, or if no contexts to request. > + * There is a callback if and only if the command is unconditional. > + * If we use SET, we clear h->meta_contexts and rebuild it. > + * If we use LIST, we are stateless. > */ > assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); > if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) { > >I'd like to review this one in its final v3 state. Thanks Laszlo