In v2: ack'ed preliminary patches have been pushed, and I've added a lot of testsuite coverage as well as putting the new API to use in nbdinfo. Eric Blake (2): api: Add nbd_opt_list_meta_context info: List available meta-contexts 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 +++++++++++++++ info/info-json.sh | 2 + info/info-list-json.sh | 4 + info/info-list.sh | 2 + info/info-text.sh | 2 + info/nbdinfo.c | 89 +++++++++- 19 files changed, 815 insertions(+), 26 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 -- 2.28.0
Eric Blake
2020-Oct-02 13:20 UTC
[Libguestfs] [libnbd PATCH v2 1/2] api: Add nbd_opt_list_meta_context
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
Eric Blake
2020-Oct-02 13:20 UTC
[Libguestfs] [libnbd PATCH v2 2/2] info: List available meta-contexts
Use the just-added nbd_opt_list_meta_context() API to give more
details about each export (matching what 'qemu-nbd --list' already
does). Note that this requires some shuffling: listing meta exports
requires being in opt mode, but is easiest to do in list_one_export(),
which requires setting opt_mode in more code paths, and deferring
ready mode until the last possible minute during get_content().
As written, the code displays the list in reverse order from how the
server presented it, thanks to my use of a simple linked list. We
could use a different data type if we don't like the data being
reversed, although the information being presented really is a set
with no inherent meaning to its presentation order.
---
info/info-json.sh | 2 +
info/info-list-json.sh | 4 ++
info/info-list.sh | 2 +
info/info-text.sh | 2 +
info/nbdinfo.c | 89 ++++++++++++++++++++++++++++++++++++++----
5 files changed, 91 insertions(+), 8 deletions(-)
diff --git a/info/info-json.sh b/info/info-json.sh
index 0f1c9fd..10bbac0 100755
--- a/info/info-json.sh
+++ b/info/info-json.sh
@@ -33,3 +33,5 @@ jq . < $out
test $( jq -r '.protocol' < $out ) != "newstyle"
test $( jq -r '.exports[0]."export-size"' < $out ) !=
"null"
test $( jq -r '.exports[0].is_read_only' < $out ) = "true"
+test $( jq -r '.exports[0].contexts[] | select(. ==
"base:allocation")' \
+ < $out ) = "base:allocation"
diff --git a/info/info-list-json.sh b/info/info-list-json.sh
index 4cf5cc0..dc4d25a 100755
--- a/info/info-list-json.sh
+++ b/info/info-list-json.sh
@@ -38,6 +38,8 @@ jq . < $out
grep '"export-name": "hello"' $out
grep '"description": "world"' $out
grep '"export-size": 1048576' $out
+test $( jq -r '.exports[0].contexts[] | select(. ==
"base:allocation")' \
+ < $out ) = "base:allocation"
# ...and again with the export name included
nbdkit -U - -e hello --filter=exportname memory 1M \
@@ -49,3 +51,5 @@ jq . < $out
grep '"export-name": "hello"' $out
grep '"description": "world"' $out
grep '"export-size": 1048576' $out
+test $( jq -r '.exports[0].contexts[] | select(. ==
"base:allocation")' \
+ < $out ) = "base:allocation"
diff --git a/info/info-list.sh b/info/info-list.sh
index a010546..f6e1ce8 100755
--- a/info/info-list.sh
+++ b/info/info-list.sh
@@ -37,6 +37,7 @@ cat $out
grep 'export="hello":' $out
grep 'description: world' $out
grep 'export-size: 1048576' $out
+sed -n '/contexts:/ { N; p; q }; $ q1' $out
# ...and again with the export name included
nbdkit -U - -e hello --filter=exportname memory 1M \
@@ -48,3 +49,4 @@ cat $out
grep 'export="hello":' $out
grep 'description: world' $out
grep 'export-size: 1048576' $out
+sed -n '/contexts:/ { N; p; q }; $ q1' $out
diff --git a/info/info-text.sh b/info/info-text.sh
index 2bebbf1..a476ffa 100755
--- a/info/info-text.sh
+++ b/info/info-text.sh
@@ -31,3 +31,5 @@ nbdkit -U - memory size=1M \
--run '$VG nbdinfo "nbd+unix:///?socket=$unixsocket"'
> $out
cat $out
grep "export-size: $((1024*1024))" $out
+sed -n '/contexts:/ { N; p; q }; $ q1' $out
+
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 6f5d191..5358f82 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -37,12 +37,18 @@ static bool json_output = false;
static const char *map = NULL;
static bool size_only = false;
+struct context_list {
+ char *name;
+ struct context_list *next;
+};
+
static struct export_list {
size_t len;
char **names;
char **descs;
} export_list;
+static int collect_context (void *opaque, const char *name);
static int collect_export (void *opaque, const char *name,
const char *desc);
static void list_one_export (struct nbd_handle *nbd, const char *desc,
@@ -207,10 +213,10 @@ main (int argc, char *argv[])
nbd_set_uri_allow_local_file (nbd, true); /* Allow ?tls-psk-file. */
/* Set optional modes in the handle. */
- if (list_all)
+ if (!map && !size_only) {
nbd_set_opt_mode (nbd, true);
- if (!map && !size_only)
nbd_set_full_info (nbd, true);
+ }
if (map)
nbd_add_meta_context (nbd, map);
@@ -320,10 +326,32 @@ main (int argc, char *argv[])
}
free (export_list.names);
free (export_list.descs);
+ nbd_opt_abort (nbd);
+ nbd_shutdown (nbd, 0);
nbd_close (nbd);
exit (EXIT_SUCCESS);
}
+static int
+collect_context (void *opaque, const char *name)
+{
+ struct context_list **head = opaque;
+ struct context_list *next = malloc (sizeof *next);
+
+ if (!next) {
+ perror ("malloc");
+ exit (EXIT_FAILURE);
+ }
+ next->name = strdup (name);
+ if (!next->name) {
+ perror ("strdup");
+ exit (EXIT_FAILURE);
+ }
+ next->next = *head;
+ *head = next;
+ return 0;
+}
+
static int
collect_export (void *opaque, const char *name, const char *desc)
{
@@ -368,8 +396,15 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
int can_cache, can_df, can_fast_zero, can_flush, can_fua,
can_multi_conn, can_trim, can_zero;
int64_t block_minimum, block_preferred, block_maximum;
+ struct context_list *contexts = NULL;
+ bool show_context = false;
/* Collect the metadata we are going to display. */
+ if (nbd_aio_is_negotiating (nbd) &&
+ nbd_opt_info (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
size = nbd_get_size (nbd);
if (size == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
@@ -387,7 +422,6 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
/* Get description if list didn't already give us one */
if (!desc)
desc = export_desc = nbd_get_export_description (nbd);
- content = get_content (nbd, size);
is_rotational = nbd_is_rotational (nbd);
is_read_only = nbd_is_read_only (nbd);
can_cache = nbd_can_cache (nbd);
@@ -401,6 +435,12 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
block_minimum = nbd_get_block_size (nbd, LIBNBD_SIZE_MINIMUM);
block_preferred = nbd_get_block_size (nbd, LIBNBD_SIZE_PREFERRED);
block_maximum = nbd_get_block_size (nbd, LIBNBD_SIZE_MAXIMUM);
+ if (nbd_opt_list_meta_context (nbd, (nbd_context_callback) {
+ .callback = collect_context, .user_data = &contexts}) != -1)
+ show_context = true;
+
+ /* Get content last, as it moves the connection out of negotiating */
+ content = get_content (nbd, size);
if (!json_output) {
printf ("export=");
@@ -412,6 +452,11 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
printf ("\texport-size: %" PRIi64 "\n", size);
if (content)
printf ("\tcontent: %s\n", content);
+ if (show_context) {
+ printf ("\tcontexts:\n");
+ for (struct context_list *next = contexts; next; next = next->next)
+ printf ("\t\t%s\n", next->name);
+ }
if (is_rotational >= 0)
printf ("\t%s: %s\n", "is_rotational", is_rotational
? "true" : "false");
if (is_read_only >= 0)
@@ -460,6 +505,18 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
printf (",\n");
}
+ if (show_context) {
+ printf ("\t\"contexts\": [\n");
+ for (struct context_list *next = contexts; next; next = next->next) {
+ printf ("\t\t");
+ print_json_string (next->name);
+ if (next->next)
+ putchar(',');
+ putchar('\n');
+ }
+ printf ("\t],\n");
+ }
+
if (is_rotational >= 0)
printf ("\t\"%s\": %s,\n",
"is_rotational", is_rotational ? "true" :
"false");
@@ -508,6 +565,12 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
printf ("\t},\n");
}
+ while (contexts) {
+ struct context_list *next = contexts->next;
+ free (contexts->name);
+ free (contexts);
+ contexts = next;
+ }
free (content);
free (export_name);
free (export_desc);
@@ -534,17 +597,16 @@ list_all_exports (struct nbd_handle *nbd1, const char
*uri)
}
nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */
nbd_set_opt_mode (nbd2, true);
+ nbd_set_full_info (nbd2, true);
if (nbd_connect_uri (nbd2, uri) == -1 ||
- nbd_set_export_name (nbd2, name) == -1 ||
- nbd_opt_go (nbd2) == -1) {
+ nbd_set_export_name (nbd2, name) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
}
else { /* ! probe_content */
- if (nbd_set_export_name (nbd1, name) == -1 ||
- nbd_opt_info (nbd1) == -1) {
+ if (nbd_set_export_name (nbd1, name) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
@@ -555,8 +617,10 @@ list_all_exports (struct nbd_handle *nbd1, const char *uri)
list_one_export (nbd2, export_list.descs[i], i == 0,
i + 1 == export_list.len);
- if (probe_content)
+ if (probe_content) {
+ nbd_shutdown (nbd2, 0);
nbd_close (nbd2);
+ }
}
}
@@ -587,6 +651,9 @@ print_json_string (const char *s)
* If file(1) doesn't work just return NULL because this is
* best-effort. This function will exit with an error on things which
* shouldn't fail, such as out of memory or creating local files.
+ *
+ * Must be called late, and only once per connection, as this kicks
+ * the connection from negotiating to ready.
*/
static char *
get_content (struct nbd_handle *nbd, int64_t size)
@@ -603,6 +670,12 @@ get_content (struct nbd_handle *nbd, int64_t size)
if (!probe_content)
return NULL;
+ if (nbd_aio_is_negotiating (nbd) &&
+ nbd_opt_go (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
/* Write the first part of the NBD export to a temporary file. */
fd = mkstemp (template);
if (fd == -1) {
--
2.28.0
Eric Blake
2020-Oct-02 14:27 UTC
Re: [Libguestfs] [libnbd PATCH v2 2/2] info: List available meta-contexts
On 10/2/20 8:20 AM, Eric Blake wrote:> Use the just-added nbd_opt_list_meta_context() API to give more > details about each export (matching what 'qemu-nbd --list' already > does). Note that this requires some shuffling: listing meta exports > requires being in opt mode, but is easiest to do in list_one_export(), > which requires setting opt_mode in more code paths, and deferring > ready mode until the last possible minute during get_content(). > > As written, the code displays the list in reverse order from how the > server presented it, thanks to my use of a simple linked list. We > could use a different data type if we don't like the data being > reversed, although the information being presented really is a set > with no inherent meaning to its presentation order. > ---> @@ -368,8 +396,15 @@ list_one_export (struct nbd_handle *nbd, const char *desc, > int can_cache, can_df, can_fast_zero, can_flush, can_fua, > can_multi_conn, can_trim, can_zero; > int64_t block_minimum, block_preferred, block_maximum; > + struct context_list *contexts = NULL; > + bool show_context = false; > > /* Collect the metadata we are going to display. */ > + if (nbd_aio_is_negotiating (nbd) && > + nbd_opt_info (nbd) == -1) { > + fprintf (stderr, "%s\n", nbd_get_error ()); > + exit (EXIT_FAILURE); > + }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
Richard W.M. Jones
2020-Oct-03 13:25 UTC
Re: [Libguestfs] [libnbd PATCH v2 2/2] info: List available meta-contexts
ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Seemingly Similar Threads
- [libnbd PATCH 2/2] info: Use nbd_opt_info for fewer handles during --list
- [libnbd PATCH 2/2] info: Expose description in list mode
- [PATCH libnbd] info: Write output atomically.
- [libnbd PATCH v2 11/13] api: Add nbd_aio_opt_list
- [libnbd PATCH v3 2/2] api: Add nbd_aio_opt_list