Right now, we require the user to supply potential metacontext names in advance, without knowing what the server actually supports until after the connection or nbd_opt_info call is complete. But the NBD protocol also supports a client being able to query what contexts a server supports, including where a client asks with a prefix and the server replies back with all answers that match the prefix. Not to mention this will allow nbdinfo to gain full feature parity with 'qemu-nbd --list' in an upcoming patch. And as promised in commit 909af17e, this adds some multi-language testsuite coverage of not only this API, but also the recent additions related to the meta context request list. --- lib/internal.h | 1 + generator/API.ml | 84 ++++++++- generator/states-newstyle-opt-meta-context.c | 83 +++++++-- generator/states-newstyle.c | 3 + lib/opt.c | 73 ++++++++ python/t/240-opt-list-meta.py | 79 +++++++++ ocaml/tests/Makefile.am | 3 + ocaml/tests/test_240_opt_list_meta.ml | 80 +++++++++ tests/Makefile.am | 10 ++ tests/opt-list-meta.c | 167 ++++++++++++++++++ .gitignore | 1 + TODO | 11 ++ golang/Makefile.am | 1 + .../libnbd/libnbd_240_opt_list_meta_test.go | 146 +++++++++++++++ 14 files changed, 724 insertions(+), 18 deletions(-) create mode 100644 python/t/240-opt-list-meta.py create mode 100644 ocaml/tests/test_240_opt_list_meta.ml create mode 100644 tests/opt-list-meta.c create mode 100644 golang/src/libguestfs.org/libnbd/libnbd_240_opt_list_meta_test.go diff --git a/lib/internal.h b/lib/internal.h index cde5dcd..ad1eeb9 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -77,6 +77,7 @@ struct command_cb { nbd_extent_callback extent; nbd_chunk_callback chunk; nbd_list_callback list; + nbd_context_callback context; } fn; nbd_completion_callback completion; }; diff --git a/generator/API.ml b/generator/API.ml index bf21aff..d1e56e4 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -144,8 +144,13 @@ let list_closure = { cbname = "list"; cbargs = [ CBString "name"; CBString "description" ] } +let context_closure = { + cbname = "context"; + cbargs = [ CBString "name" ] +} let all_closures = [ chunk_closure; completion_closure; - debug_closure; extent_closure; list_closure ] + debug_closure; extent_closure; list_closure; + context_closure ] (* Enums. *) let tls_enum = { @@ -854,7 +859,7 @@ to end the connection without finishing negotiation."; example = Some "examples/list-exports.c"; see_also = [Link "get_opt_mode"; Link "aio_is_negotiating"; Link "opt_abort"; Link "opt_go"; Link "opt_list"; - Link "opt_info"]; + Link "opt_info"; Link "opt_list_meta_context"]; }; "get_opt_mode", { @@ -964,6 +969,56 @@ corresponding L<nbd_opt_go(3)> would succeed."; Link "set_export_name"]; }; + "opt_list_meta_context", { + default_call with + args = [ Closure context_closure ]; ret = RInt; + permitted_states = [ Negotiating ]; + shortdesc = "request the server to list available meta contexts"; + longdesc = "\ +Request that the server list available meta contexts associated 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. + +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. +When the list is empty, a server will typically reply with all +contexts that it supports; when the list is non-empty, the server +will reply only with supported contexts that match the client's +request. Note that a reply by the server might be encoded to +represent several feasible contexts within one string, rather than +multiple strings per actual context name that would actually succeed +during L<nbd_opt_go(3)>; so it is still necessary to use +L<nbd_can_meta_context(3)> after connecting to see which contexts +are actually supported. + +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. Remember that it is not safe to call +L<nbd_add_meta_context(3)> from within the context of the +callback function; rather, your code must copy any C<name> needed for +later use after this function completes. 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. Likewise, +the NBD protocol does not specify an upper bound for the number of +replies that might be advertised, so client code should be aware that +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"]; + }; + "add_meta_context", { default_call with args = [ String "name" ]; ret = RErr; @@ -2237,6 +2292,29 @@ callback."; see_also = [Link "set_opt_mode"; Link "opt_info"; Link "is_read_only"]; }; + "aio_opt_list_meta_context", { + default_call with + args = [ Closure context_closure ]; ret = RInt; + optargs = [ OClosure completion_closure ]; + permitted_states = [ Negotiating ]; + shortdesc = "request the server to list available meta contexts"; + longdesc = "\ +Request that the server list available meta contexts associated 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. + +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_list_meta_context"]; + }; + "aio_pread", { default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset" ]; @@ -2888,6 +2966,8 @@ let first_version = [ "get_nr_meta_contexts", (1, 6); "get_meta_context", (1, 6); "clear_meta_contexts", (1, 6); + "opt_list_meta_context", (1, 6); + "aio_opt_list_meta_context", (1, 6); (* 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 0dc48af..fe82252 100644 --- a/generator/states-newstyle-opt-meta-context.c +++ b/generator/states-newstyle-opt-meta-context.c @@ -21,18 +21,28 @@ STATE_MACHINE { NEWSTYLE.OPT_META_CONTEXT.START: size_t i, nr_queries; - uint32_t len; + 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. + * contexts, when doing SET (but an empty LIST is okay). */ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); nbd_internal_reset_size_and_flags (h); - if (!h->structured_replies || - nbd_internal_string_list_length (h->request_meta_contexts) == 0) { - SET_NEXT_STATE (%^OPT_GO.START); - return 0; + if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) { + assert (h->opt_mode); + assert (h->structured_replies); + assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context)); + opt = h->opt_current; + } + else { + assert (CALLBACK_IS_NULL (h->opt_cb.fn.context)); + opt = NBD_OPT_SET_META_CONTEXT; + if (!h->structured_replies || + nbd_internal_string_list_length (h->request_meta_contexts) == 0) { + SET_NEXT_STATE (%^OPT_GO.START); + return 0; + } } assert (h->meta_contexts == NULL); @@ -44,7 +54,7 @@ STATE_MACHINE { len += 4 /* length of query */ + strlen (h->request_meta_contexts[i]); h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); - h->sbuf.option.option = htobe32 (NBD_OPT_SET_META_CONTEXT); + h->sbuf.option.option = htobe32 (opt); h->sbuf.option.optlen = htobe32 (len); h->wbuf = &h->sbuf; h->wlen = sizeof (h->sbuf.option); @@ -98,7 +108,8 @@ STATE_MACHINE { return 0; NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY: - const char *query = h->request_meta_contexts[h->querynum]; + const char *query = !h->request_meta_contexts ? NULL + : h->request_meta_contexts[h->querynum]; if (query == NULL) { /* end of list of requested meta contexts */ SET_NEXT_STATE (%PREPARE_FOR_REPLY); @@ -140,10 +151,17 @@ STATE_MACHINE { return 0; NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY: + uint32_t opt; + + if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) + opt = h->opt_current; + else + opt = NBD_OPT_SET_META_CONTEXT; + switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: - if (prepare_for_reply_payload (h, NBD_OPT_SET_META_CONTEXT) == -1) { + if (prepare_for_reply_payload (h, opt) == -1) { SET_NEXT_STATE (%.DEAD); return 0; } @@ -163,12 +181,25 @@ STATE_MACHINE { uint32_t len; const 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_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) @@ -194,21 +225,41 @@ STATE_MACHINE { } debug (h, "negotiated %s with context ID %" PRIu32, meta_context->name, meta_context->context_id); - meta_context->next = h->meta_contexts; - h->meta_contexts = meta_context; + if (opt == NBD_OPT_LIST_META_CONTEXT) { + CALL_CALLBACK (h->opt_cb.fn.context, meta_context->name); + free (meta_context->name); + free (meta_context); + } + else { + meta_context->next = h->meta_contexts; + h->meta_contexts = meta_context; + } } SET_NEXT_STATE (%PREPARE_FOR_REPLY); break; default: - /* Anything else is an error, ignore it */ + /* Anything else is an error, ignore it for SET, report it for LIST */ if (handle_reply_error (h) == -1) { SET_NEXT_STATE (%.DEAD); return 0; } - debug (h, "handshake: unexpected error from " - "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")", reply); - SET_NEXT_STATE (%^OPT_GO.START); + if (opt == NBD_OPT_LIST_META_CONTEXT) { + /* 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"); + CALL_CALLBACK (h->opt_cb.completion, &err); + nbd_internal_free_option (h); + SET_NEXT_STATE (%.NEGOTIATING); + } + else { + debug (h, "handshake: unexpected error from " + "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")", reply); + SET_NEXT_STATE (%^OPT_GO.START); + } break; } return 0; diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index a0a5928..6fb7548 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -136,6 +136,9 @@ STATE_MACHINE { } SET_NEXT_STATE (%PREPARE_OPT_ABORT); return 0; + case NBD_OPT_LIST_META_CONTEXT: + SET_NEXT_STATE (%OPT_META_CONTEXT.START); + return 0; case 0: break; default: diff --git a/lib/opt.c b/lib/opt.c index 6ea8326..2317b72 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -32,6 +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) + FREE_CALLBACK (h->opt_cb.fn.context); FREE_CALLBACK (h->opt_cb.completion); } @@ -166,6 +168,51 @@ nbd_unlocked_opt_list (struct nbd_handle *h, nbd_list_callback *list) return s.count; } +struct context_helper { + int count; + nbd_context_callback context; + int err; +}; +static int +context_visitor (void *opaque, const char *name) +{ + struct context_helper *h = opaque; + if (h->count < INT_MAX) + h->count++; + CALL_CALLBACK (h->context, name); + return 0; +} +static int +context_complete (void *opaque, int *err) +{ + struct context_helper *h = opaque; + h->err = *err; + FREE_CALLBACK (h->context); + return 0; +} + +/* Issue NBD_OPT_LIST_META_CONTEXT and wait for the reply. */ +int +nbd_unlocked_opt_list_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_list_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 list 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, @@ -230,3 +277,29 @@ nbd_unlocked_aio_opt_list (struct nbd_handle *h, nbd_list_callback *list, debug (h, "option queued, ignoring state machine failure"); return 0; } + +/* Issue NBD_OPT_LIST_META_CONTEXT without waiting. */ +int +nbd_unlocked_aio_opt_list_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; + } + if (!h->structured_replies) { + set_error (ENOTSUP, "server lacks structured replies"); + 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_LIST_META_CONTEXT; + if (nbd_internal_run (h, cmd_issue) == -1) + debug (h, "option queued, ignoring state machine failure"); + return 0; +} diff --git a/python/t/240-opt-list-meta.py b/python/t/240-opt-list-meta.py new file mode 100644 index 0000000..8341f9c --- /dev/null +++ b/python/t/240-opt-list-meta.py @@ -0,0 +1,79 @@ +# libnbd Python bindings +# Copyright (C) 2010-2020 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +import nbd + + +count = 0 +seen = False + + +def f(user_data, name): + global count + global seen + assert user_data == 42 + count = count + 1 + if name == nbd.CONTEXT_BASE_ALLOCATION: + seen = True + + +h = nbd.NBD() +h.set_opt_mode(True) +h.connect_command(["nbdkit", "-s", "--exit-with-parent", "-v", + "memory", "size=1M"]) + +# First pass: empty query should give at least "base:allocation". +count = 0 +seen = False +r = h.opt_list_meta_context(lambda *args: f(42, *args)) +assert r == count +assert r >= 1 +assert seen +max = count + +# Second pass: bogus query has no response. +count = 0 +seen = False +h.add_meta_context("x-nosuch:") +r = h.opt_list_meta_context(lambda *args: f(42, *args)) +assert r == 0 +assert r == count +assert not seen + +# Third pass: specific query should have one match. +count = 0 +seen = False +h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION) +assert h.get_nr_meta_contexts() == 2 +assert h.get_meta_context(1) == nbd.CONTEXT_BASE_ALLOCATION +r = h.opt_list_meta_context(lambda *args: f(42, *args)) +assert r == 1 +assert count == 1 +assert seen + +# Final pass: "base:" query should get at least "base:allocation" +count = 0 +seen = False +h.clear_meta_contexts() +h.add_meta_context("base:") +r = h.opt_list_meta_context(lambda *args: f(42, *args)) +assert r >= 1 +assert r <= max +assert r == count +assert seen + +h.opt_abort() diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am index 8fbf38f..c5dfe9e 100644 --- a/ocaml/tests/Makefile.am +++ b/ocaml/tests/Makefile.am @@ -28,6 +28,7 @@ EXTRA_DIST = \ test_210_opt_abort.ml \ test_220_opt_list.ml \ test_230_opt_info.ml \ + test_240_opt_list_meta.ml \ test_300_get_size.ml \ test_400_pread.ml \ test_405_pread_structured.ml \ @@ -53,6 +54,7 @@ tests_bc = \ test_210_opt_abort.bc \ test_220_opt_list.bc \ test_230_opt_info.bc \ + test_240_opt_list_meta.bc \ test_300_get_size.bc \ test_400_pread.bc \ test_405_pread_structured.bc \ @@ -75,6 +77,7 @@ tests_opt = \ test_210_opt_abort.opt \ test_220_opt_list.opt \ test_230_opt_info.opt \ + test_240_opt_list_meta.opt \ test_300_get_size.opt \ test_400_pread.opt \ test_405_pread_structured.opt \ diff --git a/ocaml/tests/test_240_opt_list_meta.ml b/ocaml/tests/test_240_opt_list_meta.ml new file mode 100644 index 0000000..87a2dfc --- /dev/null +++ b/ocaml/tests/test_240_opt_list_meta.ml @@ -0,0 +1,80 @@ +(* hey emacs, this is OCaml code: -*- tuareg -*- *) +(* libnbd OCaml test case + * Copyright (C) 2013-2020 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 + *) + +let count = ref 0 +let seen = ref false +let f user_data name = + assert (user_data = 42); + count := !count + 1; + if name = NBD.context_base_allocation then + seen := true; + 0 + +let () = + let nbd = NBD.create () in + NBD.set_opt_mode nbd true; + NBD.connect_command nbd + ["nbdkit"; "-s"; "--exit-with-parent"; "-v"; + "memory"; "size=1M"]; + + (* First pass: empty query should give at least "base:allocation". *) + count := 0; + seen := false; + let r = NBD.opt_list_meta_context nbd (f 42) in + assert (r = !count); + assert (r >= 1); + assert !seen; + let max = !count in + + (* Second pass: bogus query has no response. *) + count := 0; + seen := false; + NBD.add_meta_context nbd "x-nosuch:"; + let r = NBD.opt_list_meta_context nbd (f 42) in + assert (r = 0); + assert (r = !count); + assert (!seen = false); + + (* Third pass: specific query should have one match. *) + count := 0; + seen := false; + NBD.add_meta_context nbd NBD.context_base_allocation; + let c = NBD.get_nr_meta_contexts nbd in + assert (c = 2); + let n = NBD.get_meta_context nbd 1 in + assert (n = NBD.context_base_allocation); + let r = NBD.opt_list_meta_context nbd (f 42) in + assert (r = 1); + assert (r = !count); + assert !seen; + + (* Final pass: "base:" query should get at least "base:allocation" *) + count := 0; + seen := false; + NBD.clear_meta_contexts nbd; + NBD.add_meta_context nbd "base:"; + let r = NBD.opt_list_meta_context nbd (f 42) in + assert (r >= 1); + assert (r <= max); + assert (r = !count); + assert !seen; + + NBD.opt_abort nbd + +let () = Gc.compact () diff --git a/tests/Makefile.am b/tests/Makefile.am index 007c69e..9d36c43 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -38,6 +38,7 @@ EXTRA_DIST = \ meta-base-allocation.sh \ opt-info.sh \ opt-list.sh \ + opt-list-meta.sh \ synch-parallel.sh \ synch-parallel-tls.sh \ $(NULL) @@ -169,6 +170,7 @@ check_PROGRAMS += \ opt-abort \ opt-list \ opt-info \ + opt-list-meta \ connect-unix \ connect-tcp \ aio-parallel \ @@ -208,6 +210,7 @@ TESTS += \ opt-abort \ opt-list \ opt-info \ + opt-list-meta \ connect-unix \ connect-tcp \ aio-parallel.sh \ @@ -409,6 +412,13 @@ opt_info_CPPFLAGS = \ opt_info_CFLAGS = $(WARNINGS_CFLAGS) opt_info_LDADD = $(top_builddir)/lib/libnbd.la +opt_list_meta_SOURCES = opt-list-meta.c +opt_list_meta_CPPFLAGS = \ + -I$(top_srcdir)/include \ + $(NULL) +opt_list_meta_CFLAGS = $(WARNINGS_CFLAGS) +opt_list_meta_LDADD = $(top_builddir)/lib/libnbd.la + connect_unix_SOURCES = connect-unix.c connect_unix_CPPFLAGS = -I$(top_srcdir)/include connect_unix_CFLAGS = $(WARNINGS_CFLAGS) diff --git a/tests/opt-list-meta.c b/tests/opt-list-meta.c new file mode 100644 index 0000000..2a4b3c0 --- /dev/null +++ b/tests/opt-list-meta.c @@ -0,0 +1,167 @@ +/* NBD client library in userspace + * Copyright (C) 2013-2020 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_list_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> + +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; + char *args[] = { "nbdkit", "-s", "--exit-with-parent", "-v", + "memory", "size=1M", NULL }; + int max; + char *tmp; + + /* 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); + } + + /* First pass: empty query should give at least "base:allocation". */ + p = (struct progress) { .count = 0 }; + r = nbd_opt_list_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) { + fprintf (stderr, "inconsistent return value %d, expected %d\n", r, p.count); + exit (EXIT_FAILURE); + } + if (r < 1 || !p.seen) { + fprintf (stderr, "server did not reply with base:allocation\n"); + exit (EXIT_FAILURE); + } + max = p.count; + + /* Second pass: bogus query has no response. */ + p = (struct progress) { .count = 0 }; + r = nbd_add_meta_context (nbd, "x-nosuch:"); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + r = nbd_opt_list_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 != 0 || p.seen) { + fprintf (stderr, "expecting no contexts, got %d\n", r); + exit (EXIT_FAILURE); + } + + /* Third pass: specific query should have one match. */ + p = (struct progress) { .count = 0 }; + r = nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_get_nr_meta_contexts (nbd) != 2) { + fprintf (stderr, "expecting 2 meta requests\n"); + exit (EXIT_FAILURE); + } + tmp = nbd_get_meta_context (nbd, 1); + if (!tmp) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (strcmp (tmp, LIBNBD_CONTEXT_BASE_ALLOCATION) != 0) { + fprintf (stderr, "expecting base:allocation, got %s\n", tmp); + exit (EXIT_FAILURE); + } + free (tmp); + r = nbd_opt_list_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 exactly one context, got %d\n", r); + exit (EXIT_FAILURE); + } + + /* Final pass: "base:" query should get at least "base:allocation" */ + p = (struct progress) { .count = 0 }; + r = nbd_clear_meta_contexts (nbd); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + r = nbd_add_meta_context (nbd, "base:"); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + r = nbd_opt_list_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 || r > max || r != p.count || !p.seen) { + fprintf (stderr, "expecting at least one context, got %d\n", r); + exit (EXIT_FAILURE); + } + + nbd_opt_abort (nbd); + nbd_close (nbd); + exit (EXIT_SUCCESS); +} diff --git a/.gitignore b/.gitignore index aa9e0bb..3afa9f3 100644 --- a/.gitignore +++ b/.gitignore @@ -175,6 +175,7 @@ Makefile.in /tests/opt-abort /tests/opt-info /tests/opt-list +/tests/opt-list-meta /tests/pki/ /tests/read-only-flag /tests/read-write-flag diff --git a/TODO b/TODO index 6dff4df..10b1280 100644 --- a/TODO +++ b/TODO @@ -13,6 +13,17 @@ NBD resize extension. TLS should properly shut down the session (calling gnutls_bye). +Potential deadlock with nbd_add_meta_context: if a client sends enough +requests to the server that it blocks while writing, but the server +replies to requests as they come in rather than waiting for the end of +the client request, then the server can likewise block in writing +replies that libnbd is not yet reading. Not an issue for existing +servers that don't have enough contexts to reply with enough data to +fill buffers, but could be an issue with qemu-nbd if it is taught to +exports many dirty bitmaps simultaneously. Revamping the +states-newstyle-meta-context.c state machine to let libnbd handle +NBD_REP_META_CONTEXT while still writing queries could be hairy. + Performance: Chart it over various buffer sizes and threads, as that should make it easier to identify systematic issues. diff --git a/golang/Makefile.am b/golang/Makefile.am index c4fa564..fbc7c8c 100644 --- a/golang/Makefile.am +++ b/golang/Makefile.am @@ -36,6 +36,7 @@ source_files = \ src/$(pkg)/libnbd_210_opt_abort_test.go \ src/$(pkg)/libnbd_220_opt_list_test.go \ src/$(pkg)/libnbd_230_opt_info_test.go \ + src/$(pkg)/libnbd_240_opt_list_meta_test.go \ src/$(pkg)/libnbd_300_get_size_test.go \ src/$(pkg)/libnbd_400_pread_test.go \ src/$(pkg)/libnbd_405_pread_structured_test.go \ diff --git a/golang/src/libguestfs.org/libnbd/libnbd_240_opt_list_meta_test.go b/golang/src/libguestfs.org/libnbd/libnbd_240_opt_list_meta_test.go new file mode 100644 index 0000000..afddff6 --- /dev/null +++ b/golang/src/libguestfs.org/libnbd/libnbd_240_opt_list_meta_test.go @@ -0,0 +1,146 @@ +/* libnbd golang tests + * Copyright (C) 2013-2020 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +package libnbd + +import ( + "testing" +) + +var count uint +var seen bool + +func listmetaf(user_data int, name string) int { + if user_data != 42 { + panic("expected user_data == 42") + } + count++ + if (name == context_base_allocation) { + seen = true + } + return 0 +} + +func Test240OptListMeta(t *testing.T) { + h, err := Create() + if err != nil { + t.Fatalf("could not create handle: %s", err) + } + defer h.Close() + + err = h.SetOptMode(true) + if err != nil { + t.Fatalf("could not set opt mode: %s", err) + } + + err = h.ConnectCommand([]string{ + "nbdkit", "-s", "--exit-with-parent", "-v", + "memory", "size=1M", + }) + if err != nil { + t.Fatalf("could not connect: %s", err) + } + + /* First pass: empty query should give at least "base:allocation". */ + count = 0 + seen = false + r, err := h.OptListMetaContext(func(name string) int { + return listmetaf(42, name) + }) + if err != nil { + t.Fatalf("could not request opt_list_meta_context: %s", err) + } + if r != count || r < 1 || !seen { + t.Fatalf("unexpected count after opt_list_meta_context") + } + max := count + + /* Second pass: bogus query has no response. */ + count = 0 + seen = false + err = h.AddMetaContext("x-nosuch:") + if err != nil { + t.Fatalf("could not request add_meta_context: %s", err) + } + r, err = h.OptListMetaContext(func(name string) int { + return listmetaf(42, name) + }) + if err != nil { + t.Fatalf("could not request opt_list_meta_context: %s", err) + } + if r != 0 || r != count || seen { + t.Fatalf("unexpected count after opt_list_meta_context") + } + + /* Third pass: specific query should have one match. */ + count = 0 + seen = false + err = h.AddMetaContext(context_base_allocation) + if err != nil { + t.Fatalf("could not request add_meta_context: %s", err) + } + r, err = h.GetNrMetaContexts() + if err != nil { + t.Fatalf("could not request get_nr_meta_contexts: %s", err) + } + if r != 2 { + t.Fatalf("wrong number of meta_contexts: %d", r) + } + tmp, err := h.GetMetaContext(1) + if err != nil { + t.Fatalf("could not request get_meta_context: %s", err) + } + if *tmp != context_base_allocation { + t.Fatalf("wrong result of get_meta_context: %s", *tmp) + } + r, err = h.OptListMetaContext(func(name string) int { + return listmetaf(42, name) + }) + if err != nil { + t.Fatalf("could not request opt_list_meta_context: %s", err) + } + if r != 1 || r != count || !seen { + t.Fatalf("unexpected count after opt_list_meta_context") + } + + /* Final pass: "base:" query should get at least "base:allocation" */ + count = 0 + seen = false + err = h.ClearMetaContexts() + if err != nil { + t.Fatalf("could not request clear_meta_contexts: %s", err) + } + err = h.AddMetaContext("base:") + if err != nil { + t.Fatalf("could not request add_meta_context: %s", err) + } + r, err = h.OptListMetaContext(func(name string) int { + return listmetaf(42, name) + }) + if err != nil { + t.Fatalf("could not request opt_list_meta_context: %s", err) + } + if r < 1 || r > max || r != count || !seen { + t.Fatalf("unexpected count after opt_list_meta_context") + } + + err = h.OptAbort() + if err != nil { + t.Fatalf("could not request opt_abort: %s", err) + } +} -- 2.28.0

On 10/2/20 8:20 AM, Eric Blake wrote: For the record: this failed on a server with legacy "newstyle" protocol (nbdkit --mask-handshake=0) - such a server lacks nbd_opt_info support, and you HAVE to use nbd_opt_go to get size and other details. But when I fixed this line, it in turn flushed out an assertion failure present since 1.4 (now fixed) where nbd_opt_go() on a legacy newstyle accessed uninitialized memory, and nbd_aio_opt_go() could leak resources, because of a forgotten completion callback invocation. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org