This replaces 13/13 of my v2 series; and now that it has pretty good testsuite coverage and demonstrable performance improvement to nbdinfo, I'm going ahead and pushing this now. We may still want to add further nbd_opt_* commands for other fine-grained tuning of negotiation, but for now, I think things have stabilized on this end, and I can return to polishing .list_exports on the nbdkit end. Eric Blake (2): api: add nbd_opt_info, nbd_aio_opt_info info: Use nbd_opt_info for fewer handles during --list docs/libnbd.pod | 4 + info/nbdinfo.pod | 2 + generator/API.ml | 116 ++++++--- generator/states-newstyle-opt-go.c | 26 +- generator/states-newstyle.c | 1 + lib/flags.c | 12 + lib/opt.c | 38 +++ python/t/230-opt-info.py | 82 +++++++ ocaml/tests/Makefile.am | 3 + ocaml/tests/test_230_opt_info.ml | 108 +++++++++ tests/Makefile.am | 11 + tests/newstyle-limited.c | 8 + tests/opt-info.c | 196 +++++++++++++++ tests/opt-info.sh | 45 ++++ .gitignore | 1 + TODO | 13 - .../libnbd/libnbd_230_opt_info_test.go | 225 ++++++++++++++++++ info/nbdinfo.c | 56 +++-- 18 files changed, 872 insertions(+), 75 deletions(-) create mode 100644 python/t/230-opt-info.py create mode 100644 ocaml/tests/test_230_opt_info.ml create mode 100644 tests/opt-info.c create mode 100755 tests/opt-info.sh create mode 100644 golang/src/libguestfs.org/libnbd/libnbd_230_opt_info_test.go -- 2.28.0
Eric Blake
2020-Aug-19 01:53 UTC
[Libguestfs] [libnbd PATCH 1/2] api: add nbd_opt_info, nbd_aio_opt_info
Make it possible to query details about a potential export without actually connecting. This reuses the NBD_OPT_GO state machine, with the main difference being that we can now return to negotiating state even on success; thus checking for progression to Ready state is no longer sufficient for detecting failure, and we have to rely on the completion callback. And now that we can get to Negotiation after a successful query, we have to enable a lot more functions for use in that state to read the results of the info query. The next patch will then update nbdinfo to take advantage of this. Meanwhile, this adds testsuite coverage of all our language bindings (while the test is transcribed to be the same between them, I'm less certain my ocaml and golang versions are idiomatic). --- docs/libnbd.pod | 4 + generator/API.ml | 116 ++++++--- generator/states-newstyle-opt-go.c | 26 +- generator/states-newstyle.c | 1 + lib/flags.c | 12 + lib/opt.c | 38 +++ python/t/230-opt-info.py | 82 +++++++ ocaml/tests/Makefile.am | 3 + ocaml/tests/test_230_opt_info.ml | 108 +++++++++ tests/Makefile.am | 11 + tests/newstyle-limited.c | 8 + tests/opt-info.c | 196 +++++++++++++++ tests/opt-info.sh | 45 ++++ .gitignore | 1 + .../libnbd/libnbd_230_opt_info_test.go | 225 ++++++++++++++++++ 15 files changed, 838 insertions(+), 38 deletions(-) create mode 100644 python/t/230-opt-info.py create mode 100644 ocaml/tests/test_230_opt_info.ml create mode 100644 tests/opt-info.c create mode 100755 tests/opt-info.sh create mode 100644 golang/src/libguestfs.org/libnbd/libnbd_230_opt_info_test.go diff --git a/docs/libnbd.pod b/docs/libnbd.pod index 4ee0815..9cdf152 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -493,6 +493,10 @@ Note that there are some servers (like L<nbdkit(1)> E<le> 1.14) which ignore this, and other servers (like L<qemu-nbd(8)>) which require it to be set correctly but cannot serve different content. +These APIs are also available after a successful L<nbd_opt_info(3)> +during the negotiation phase, if you used L<nbd_set_opt_mode(3)> prior +to connecting. + =head2 Flag calls After connecting the server will send back a set of flags describing diff --git a/generator/API.ml b/generator/API.ml index d5a27fe..80b2145 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -294,12 +294,14 @@ before beginning a connection. However, when L<nbd_set_opt_mode(3)> has enabled option mode, it is possible to change the export name prior to L<nbd_opt_go(3)>. In particular, the use of L<nbd_opt_list(3)> during negotiation can be used to determine -a name the server is likely to accept. +a name the server is likely to accept, and L<nbd_opt_info(3)> can +be used to learn details about an export before connecting. This call may be skipped if using L<nbd_connect_uri(3)> to connect to a URI that includes an export name."; see_also = [Link "get_export_name"; Link "connect_uri"; - Link "set_opt_mode"; Link "opt_go"; Link "opt_list"]; + Link "set_opt_mode"; Link "opt_go"; Link "opt_list"; + Link "opt_info"]; }; "get_export_name", { @@ -353,7 +355,7 @@ Return the state of the full info request flag on this handle."; "get_canonical_export_name", { default_call with args = []; ret = RString; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "return the canonical export name, if the server has one"; longdesc = "\ The NBD protocol permits a server to report an optional canonical @@ -373,7 +375,7 @@ client specifically hinted about wanting it, via L<nbd_set_full_info(3)>."; "get_export_description", { default_call with args = []; ret = RString; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "return the export description, if the server has one"; longdesc = "\ The NBD protocol permits a server to report an optional export @@ -715,7 +717,8 @@ export name before trying again. You may also use L<nbd_opt_abort(3)> 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_abort"; Link "opt_go"; Link "opt_list"; + Link "opt_info"]; }; "get_opt_mode", { @@ -744,7 +747,7 @@ possible to attempt another option such as a different export name; although older servers will instead have killed the connection."; 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 "set_export_name"; Link "connect_uri"; Link "opt_info"]; }; "opt_abort", { @@ -799,6 +802,29 @@ description is set with I<-D>."; Link "set_export_name"]; }; + "opt_info", { + default_call with + args = []; ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "request the server for information about an export"; + longdesc = "\ +Request that the server supply information about the most recent +export name set by L<nbd_set_export_name(3)>. This can +only be used if L<nbd_set_opt_mode(3)> enabled option mode. + +If successful, functions like L<nbd_is_read_only(3)> and +L<nbd_get_size(3)> will report details about that export. In +general, if L<nbd_opt_go(3)> is called next, that call will +likely succeed with the details remaining the same, although this +is not guaranteed by all servers. + +Not all servers understand this request, and even when it is +understood, the server might fail the request even when a +corresponding L<nbd_opt_go(3)> would succeed."; + see_also = [Link "set_opt_mode"; Link "aio_opt_info"; Link "opt_go"; + Link "set_export_name"]; + }; + "add_meta_context", { default_call with args = [ String "name" ]; ret = RErr; @@ -1181,27 +1207,27 @@ is killed."; "is_read_only", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "is the NBD export read-only?"; longdesc = "\ Returns true if the NBD export is read-only; writes and write-like operations will fail." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"]; + see_also = [SectionLink "Flag calls"; Link "opt_info"]; example = Some "examples/server-flags.c"; }; "can_flush", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the flush command?"; longdesc = "\ Returns true if the server supports the flush command (see L<nbd_flush(3)>, L<nbd_aio_flush(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "flush"; Link "aio_flush"]; example = Some "examples/server-flags.c"; }; @@ -1209,13 +1235,13 @@ the server does not." "can_fua", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the FUA flag?"; longdesc = "\ Returns true if the server supports the FUA flag on certain commands (see L<nbd_pwrite(3)>)." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; Link "pwrite"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "pwrite"; Link "zero"; Link "trim"]; example = Some "examples/server-flags.c"; }; @@ -1223,7 +1249,7 @@ certain commands (see L<nbd_pwrite(3)>)." "is_rotational", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "is the NBD disk rotational (like a disk)?"; longdesc = "\ Returns true if the disk exposed over NBD is rotational @@ -1231,21 +1257,21 @@ Returns true if the disk exposed over NBD is rotational the disk has no penalty for random access (like an SSD or RAM disk)." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"]; + see_also = [SectionLink "Flag calls"; Link "opt_info"]; example = Some "examples/server-flags.c"; }; "can_trim", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the trim command?"; longdesc = "\ Returns true if the server supports the trim command (see L<nbd_trim(3)>, L<nbd_aio_trim(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "trim"; Link "aio_trim"]; example = Some "examples/server-flags.c"; }; @@ -1253,14 +1279,14 @@ the server does not." "can_zero", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the zero command?"; longdesc = "\ Returns true if the server supports the zero command (see L<nbd_zero(3)>, L<nbd_aio_zero(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "zero"; Link "aio_zero"; Link "can_fast_zero"]; example = Some "examples/server-flags.c"; @@ -1269,7 +1295,7 @@ the server does not." "can_fast_zero", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the fast zero flag?"; longdesc = "\ Returns true if the server supports the use of the @@ -1277,7 +1303,7 @@ C<LIBNBD_CMD_FLAG_FAST_ZERO> flag to the zero command (see L<nbd_zero(3)>, L<nbd_aio_zero(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "zero"; Link "aio_zero"; Link "can_zero"]; example = Some "examples/server-flags.c"; }; @@ -1285,7 +1311,7 @@ the server does not." "can_df", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the don't fragment flag to pread?"; longdesc = "\ Returns true if the server supports structured reads with an @@ -1293,7 +1319,7 @@ ability to request a non-fragmented read (see L<nbd_pread_structured(3)>, L<nbd_aio_pread_structured(3)>). Returns false if the server either lacks structured reads or if it does not support a non-fragmented read request." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "pread_structured"; Link "aio_pread_structured"]; example = Some "examples/server-flags.c"; @@ -1302,7 +1328,7 @@ structured reads or if it does not support a non-fragmented read request." "can_multi_conn", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support multi-conn?"; longdesc = "\ Returns true if the server supports multi-conn. Returns @@ -1315,21 +1341,22 @@ way to check for this is to open one connection, check this flag is true, then open further connections as required." ^ non_blocking_test_call_description; - see_also = [SectionLink "Multi-conn"]; + see_also = [SectionLink "Flag calls"; SectionLink "Multi-conn"; + Link "opt_info"]; example = Some "examples/server-flags.c"; }; "can_cache", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the cache command?"; longdesc = "\ Returns true if the server supports the cache command (see L<nbd_cache(3)>, L<nbd_aio_cache(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "cache"; Link "aio_cache"]; example = Some "examples/server-flags.c"; }; @@ -1337,7 +1364,7 @@ the server does not." "can_meta_context", { default_call with args = [String "metacontext"]; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support a specific meta context?"; longdesc = "\ Returns true if the server supports the given meta context @@ -1350,7 +1377,7 @@ B<E<lt>libnbd.hE<gt>> includes defined constants for well-known namespace contexts beginning with C<LIBNBD_CONTEXT_>, but you are free to pass in other contexts." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "add_meta_context"; Link "block_status"; Link "aio_block_status"]; }; @@ -1377,19 +1404,19 @@ Most modern NBD servers use C<\"newstyle-fixed\">. "get_size", { default_call with args = []; ret = RInt64; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "return the export size"; longdesc = "\ Returns the size in bytes of the NBD export." ^ non_blocking_test_call_description; - see_also = [SectionLink "Size of the export"]; + see_also = [SectionLink "Size of the export"; Link "opt_info"]; example = Some "examples/get-size.c"; }; "get_block_size", { default_call with args = [Enum ("size_type", block_size_enum)]; ret = RInt64; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "return a specific server block size constraint"; longdesc = "\ Returns a specific size constraint advertised by the server, if any. If @@ -1432,7 +1459,7 @@ Future NBD extensions may result in additional C<size_type> values. " ^ non_blocking_test_call_description; see_also = [Link "get_protocol"; - Link "get_size"] + Link "get_size"; Link "opt_info"] }; "pread", { @@ -1937,6 +1964,29 @@ callback."; see_also = [Link "set_opt_mode"; Link "opt_list"]; }; + "aio_opt_info", { + default_call with + args = []; + optargs = [ OClosure completion_closure ]; + ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "request the server for information about an export"; + longdesc = "\ +Request that the server supply information about the most recent +export name set by L<nbd_set_export_name(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>. Note that direct detection of +whether the server returns an error (as is done by the return value +of the synchronous counterpart) is only possible with a completion +callback; however, indirect detection of failure is possible by +checking whether L<nbd_is_read_only(3)> and similar also fail."; + see_also = [Link "set_opt_mode"; Link "opt_info"; Link "is_read_only"]; + }; + "aio_pread", { default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset" ]; @@ -2556,9 +2606,11 @@ let first_version = [ "opt_go", (1, 4); "opt_abort", (1, 4); "opt_list", (1, 4); + "opt_info", (1, 4); "aio_opt_go", (1, 4); "aio_opt_abort", (1, 4); "aio_opt_list", (1, 4); + "aio_opt_info", (1, 4); (* 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-go.c b/generator/states-newstyle-opt-go.c index be19e1c..eed769b 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -23,8 +23,15 @@ STATE_MACHINE { uint16_t nrinfos = h->full_info ? 3 : 1; assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); + if (h->opt_current == NBD_OPT_INFO) + assert (h->opt_mode); + else if (!h->opt_current) { + assert (!h->opt_mode); + assert(CALLBACK_IS_NULL(h->opt_cb.completion)); + h->opt_current = NBD_OPT_GO; + } h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); - h->sbuf.option.option = htobe32 (NBD_OPT_GO); + h->sbuf.option.option = htobe32 (h->opt_current); h->sbuf.option.optlen htobe32 (/* exportnamelen */ 4 + strlen (h->export_name) + sizeof nrinfos + 2 * nrinfos); @@ -101,7 +108,7 @@ STATE_MACHINE { switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: - if (prepare_for_reply_payload (h, NBD_OPT_GO) == -1) { + if (prepare_for_reply_payload (h, h->opt_current) == -1) { SET_NEXT_STATE (%.DEAD); return 0; } @@ -206,9 +213,12 @@ STATE_MACHINE { SET_NEXT_STATE (%RECV_REPLY); return 0; case NBD_REP_ERR_UNSUP: - debug (h, "server is confused by NBD_OPT_GO, continuing anyway"); - SET_NEXT_STATE (%^OPT_EXPORT_NAME.START); - return 0; + if (h->opt_current == NBD_OPT_GO) { + debug (h, "server is confused by NBD_OPT_GO, continuing anyway"); + SET_NEXT_STATE (%^OPT_EXPORT_NAME.START); + return 0; + } + /* fallthrough */ default: if (handle_reply_error (h) == -1) { SET_NEXT_STATE (%.DEAD); @@ -216,6 +226,10 @@ STATE_MACHINE { } /* Decode expected known errors into a nicer string */ switch (reply) { + case NBD_REP_ERR_UNSUP: + assert (h->opt_current == NBD_OPT_INFO); + set_error (ENOTSUP, "handshake: server lacks NBD_OPT_INFO support"); + break; case NBD_REP_ERR_POLICY: case NBD_REP_ERR_PLATFORM: set_error (0, "handshake: server policy prevents NBD_OPT_GO"); @@ -249,7 +263,7 @@ STATE_MACHINE { break; } - if (err == 0) + if (err == 0 && h->opt_current == NBD_OPT_GO) SET_NEXT_STATE (%^FINISHED); else if (h->opt_mode) SET_NEXT_STATE (%.NEGOTIATING); diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index 290cbf6..92cf5c9 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -119,6 +119,7 @@ STATE_MACHINE { */ switch (h->opt_current) { case NBD_OPT_GO: + case NBD_OPT_INFO: if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) SET_NEXT_STATE (%OPT_EXPORT_NAME.START); else diff --git a/lib/flags.c b/lib/flags.c index 09eac72..c76cdd5 100644 --- a/lib/flags.c +++ b/lib/flags.c @@ -196,6 +196,12 @@ nbd_unlocked_can_meta_context (struct nbd_handle *h, const char *name) { struct meta_context *meta_context; + if (h->eflags == 0) { + set_error (EINVAL, "server has not returned export flags, " + "you need to connect to the server first"); + return -1; + } + for (meta_context = h->meta_contexts; meta_context; meta_context = meta_context->next) @@ -222,6 +228,12 @@ nbd_unlocked_get_size (struct nbd_handle *h) int64_t nbd_unlocked_get_block_size (struct nbd_handle *h, int type) { + if (h->eflags == 0) { + set_error (EINVAL, "server has not returned export flags, " + "you need to connect to the server first"); + return -1; + } + switch (type) { case LIBNBD_SIZE_MINIMUM: return h->block_minimum; diff --git a/lib/opt.c b/lib/opt.c index 3c0c539..cc0c145 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -90,6 +90,26 @@ nbd_unlocked_opt_go (struct nbd_handle *h) return r; } +/* Issue NBD_OPT_INFO and wait for the reply. */ +int +nbd_unlocked_opt_info (struct nbd_handle *h) +{ + int err; + nbd_completion_callback c = { .callback = go_complete, .user_data = &err }; + int r = nbd_unlocked_aio_opt_info (h, c); + + if (r == -1) + return r; + + r = wait_for_option (h); + if (r == 0 && err) { + assert (nbd_internal_is_state_negotiating (get_next_state (h))); + set_error (err, "server replied with error to opt_info request"); + return -1; + } + return r; +} + /* Issue NBD_OPT_ABORT and wait for the state change. */ int nbd_unlocked_opt_abort (struct nbd_handle *h) @@ -158,6 +178,24 @@ nbd_unlocked_aio_opt_go (struct nbd_handle *h, return 0; } +/* Issue NBD_OPT_INFO without waiting. */ +int +nbd_unlocked_aio_opt_info (struct nbd_handle *h, + nbd_completion_callback complete) +{ + if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) { + set_error (ENOTSUP, "server is not using fixed newstyle protocol"); + return -1; + } + + h->opt_current = NBD_OPT_INFO; + h->opt_cb.completion = complete; + + if (nbd_internal_run (h, cmd_issue) == -1) + debug (h, "option queued, ignoring state machine failure"); + return 0; +} + /* Issue NBD_OPT_ABORT without waiting. */ int nbd_unlocked_aio_opt_abort (struct nbd_handle *h) diff --git a/python/t/230-opt-info.py b/python/t/230-opt-info.py new file mode 100644 index 0000000..f36d119 --- /dev/null +++ b/python/t/230-opt-info.py @@ -0,0 +1,82 @@ +# 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 +import errno +import os + +script = ("%s/../tests/opt-info.sh" % os.getenv ("srcdir", ".")) + +def must_fail (f, *args, **kwds): + try: + f (*args, **kwds) + assert (False) + except nbd.Error as ex: + pass + +h = nbd.NBD () +h.set_opt_mode (True) +h.connect_command (["nbdkit", "-s", "--exit-with-parent", "-v", + "sh", script]) +h.add_meta_context (nbd.CONTEXT_BASE_ALLOCATION) + +# No size, flags, or meta-contexts yet */ +must_fail (h.get_size) +must_fail (h.is_read_only) +must_fail (h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION) + +# info with no prior name gets info on "" +h.opt_info () +assert h.get_size () == 0 +assert h.is_read_only () == 1 +assert h.can_meta_context (nbd.CONTEXT_BASE_ALLOCATION) == 1 + +# info on something not present fails, wipes out prior info +h.set_export_name ("a") +must_fail (h.opt_info) +must_fail (h.get_size) +must_fail (h.is_read_only) +must_fail (h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION) + +# info for a different export +h.set_export_name ("b") +h.opt_info () +assert h.get_size () == 1 +assert h.is_read_only () == 0 +assert h.can_meta_context (nbd.CONTEXT_BASE_ALLOCATION) == 1 + +# go on something not present +h.set_export_name ("a") +must_fail (h.opt_go) +must_fail (h.get_size) +must_fail (h.is_read_only) +must_fail (h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION) + +# go on a valid export +h.set_export_name ("good") +h.opt_go () +assert h.get_size () == 4 +assert h.is_read_only () == 1 +assert h.can_meta_context (nbd.CONTEXT_BASE_ALLOCATION) == 1 + +# now info is no longer valid, but does not wipe data +must_fail (h.set_export_name, "a") +assert h.get_export_name () == "good" +must_fail (h.opt_info) +assert h.get_size () == 4 + +h.shutdown () diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am index 3af13a2..3215a03 100644 --- a/ocaml/tests/Makefile.am +++ b/ocaml/tests/Makefile.am @@ -25,6 +25,7 @@ EXTRA_DIST = \ test_200_connect_command.ml \ test_210_opt_abort.ml \ test_220_opt_list.ml \ + test_230_opt_info.ml \ test_300_get_size.ml \ test_400_pread.ml \ test_405_pread_structured.ml \ @@ -47,6 +48,7 @@ tests_bc = \ test_200_connect_command.bc \ test_210_opt_abort.bc \ test_220_opt_list.bc \ + test_230_opt_info.bc \ test_300_get_size.bc \ test_400_pread.bc \ test_405_pread_structured.bc \ @@ -66,6 +68,7 @@ tests_opt = \ test_200_connect_command.opt \ test_210_opt_abort.opt \ test_220_opt_list.opt \ + test_230_opt_info.opt \ test_300_get_size.opt \ test_400_pread.opt \ test_405_pread_structured.opt \ diff --git a/ocaml/tests/test_230_opt_info.ml b/ocaml/tests/test_230_opt_info.ml new file mode 100644 index 0000000..657dfe6 --- /dev/null +++ b/ocaml/tests/test_230_opt_info.ml @@ -0,0 +1,108 @@ +(* 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 + *) + +open Printf + +let script + try + let srcdir = Sys.getenv "srcdir" in + sprintf "%s/../../tests/opt-info.sh" srcdir + with + Not_found -> failwith "error: srcdir is not defined" + +let fail_unary f nbd + try + let _ = f nbd in + assert (false) + with + NBD.Error (errstr, errno) -> () + +let fail_binary f nbd arg + try + let _ = f nbd arg in + assert (false) + with + NBD.Error (errstr, errno) -> () + +let () + let nbd = NBD.create () in + NBD.set_opt_mode nbd true; + NBD.connect_command nbd + ["nbdkit"; "-s"; "--exit-with-parent"; "-v"; + "sh"; script]; + NBD.add_meta_context nbd NBD.context_base_allocation; + + (* No size, flags, or meta-contexts yet *) + fail_unary NBD.get_size nbd; + fail_unary NBD.is_read_only nbd; + fail_binary NBD.can_meta_context nbd NBD.context_base_allocation; + + (* info with no prior name gets info on "" *) + NBD.opt_info nbd; + let size = NBD.get_size nbd in + assert (size = 0L); + let ro = NBD.is_read_only nbd in + assert (ro = true); + let meta = NBD.can_meta_context nbd NBD.context_base_allocation in + assert (meta = true); + + (* info on something not present fails, wipes out prior info *) + NBD.set_export_name nbd "a"; + fail_unary NBD.opt_info nbd; + fail_unary NBD.get_size nbd; + fail_unary NBD.is_read_only nbd; + fail_binary NBD.can_meta_context nbd NBD.context_base_allocation; + + (* info for a different export *) + NBD.set_export_name nbd "b"; + NBD.opt_info nbd; + let size = NBD.get_size nbd in + assert (size = 1L); + let ro = NBD.is_read_only nbd in + assert (ro = false); + let meta = NBD.can_meta_context nbd NBD.context_base_allocation in + assert (meta = true); + + (* go on something not present *) + NBD.set_export_name nbd "a"; + fail_unary NBD.opt_go nbd; + fail_unary NBD.get_size nbd; + fail_unary NBD.is_read_only nbd; + fail_binary NBD.can_meta_context nbd NBD.context_base_allocation; + + (* go on a valid export *) + NBD.set_export_name nbd "good"; + NBD.opt_go nbd; + let size = NBD.get_size nbd in + assert (size = 4L); + let ro = NBD.is_read_only nbd in + assert (ro = true); + let meta = NBD.can_meta_context nbd NBD.context_base_allocation in + assert (meta = true); + + (* now info is no longer valid, but does not wipe data *) + fail_binary NBD.set_export_name nbd "a"; + let name = NBD.get_export_name nbd in + assert (name = "good"); + fail_unary NBD.opt_info nbd; + let size = NBD.get_size nbd in + assert (size = 4L); + + NBD.shutdown nbd + +let () = Gc.compact () diff --git a/tests/Makefile.am b/tests/Makefile.am index 79e7475..be7c83c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -36,6 +36,7 @@ EXTRA_DIST = \ functions.sh.in \ make-pki.sh \ meta-base-allocation.sh \ + opt-info.sh \ opt-list.sh \ synch-parallel.sh \ synch-parallel-tls.sh \ @@ -166,6 +167,7 @@ check_PROGRAMS += \ newstyle-limited \ opt-abort \ opt-list \ + opt-info \ connect-unix \ connect-tcp \ aio-parallel \ @@ -203,6 +205,7 @@ TESTS += \ newstyle-limited \ opt-abort \ opt-list \ + opt-info \ connect-unix \ connect-tcp \ aio-parallel.sh \ @@ -391,6 +394,14 @@ opt_list_CPPFLAGS = \ opt_list_CFLAGS = $(WARNINGS_CFLAGS) opt_list_LDADD = $(top_builddir)/lib/libnbd.la +opt_info_SOURCES = opt-info.c +opt_info_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -DSCRIPT='"$(abs_srcdir)/opt-info.sh"' \ + $(NULL) +opt_info_CFLAGS = $(WARNINGS_CFLAGS) +opt_info_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/newstyle-limited.c b/tests/newstyle-limited.c index 5b2bb2b..fd89620 100644 --- a/tests/newstyle-limited.c +++ b/tests/newstyle-limited.c @@ -152,6 +152,14 @@ main (int argc, char *argv[]) fprintf (stderr, "%s: wrong errno value after failed opt_list\n", argv[0]); exit (EXIT_FAILURE); } + if (nbd_opt_info (nbd) != -1) { + fprintf (stderr, "nbd_opt_info: expected failure\n"); + exit (EXIT_FAILURE); + } + if (nbd_get_errno () != ENOTSUP) { + fprintf (stderr, "%s: wrong errno value after failed opt_info\n", argv[0]); + exit (EXIT_FAILURE); + } if (nbd_opt_abort (nbd) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); diff --git a/tests/opt-info.c b/tests/opt-info.c new file mode 100644 index 0000000..b9739a5 --- /dev/null +++ b/tests/opt-info.c @@ -0,0 +1,196 @@ +/* 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_info. */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <inttypes.h> +#include <string.h> +#include <errno.h> + +#include <libnbd.h> + +int +main (int argc, char *argv[]) +{ + struct nbd_handle *nbd; + int64_t r; + char *s; + char *args[] = { "nbdkit", "-s", "--exit-with-parent", "-v", + "sh", SCRIPT, NULL }; + + /* Get into negotiating state. */ + nbd = nbd_create (); + if (nbd == NULL || + nbd_set_opt_mode (nbd, true) == -1 || + nbd_connect_command (nbd, args) == -1 || + nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + /* No size, flags, or meta-contexts yet */ + if (nbd_get_size (nbd) != -1) { + fprintf (stderr, "expecting error for get_size\n"); + exit (EXIT_FAILURE); + } + if (nbd_is_read_only (nbd) != -1) { + fprintf (stderr, "expecting error for is_read_only\n"); + exit (EXIT_FAILURE); + } + if (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) != -1) { + fprintf (stderr, "expecting error for can_meta_context\n"); + exit (EXIT_FAILURE); + } + + /* info with no prior name gets info on "" */ + if (nbd_opt_info (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_size (nbd)) != 0) { + fprintf (stderr, "expecting size of 0, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_is_read_only (nbd)) != 1) { + fprintf (stderr, "expecting read-only export, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) { + fprintf (stderr, "expecting can_meta_context true, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + + /* info on something not present fails, wipes out prior info */ + if (nbd_set_export_name (nbd, "a") == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_opt_info (nbd) != -1) { + fprintf (stderr, "expecting error for opt_info\n"); + exit (EXIT_FAILURE); + } + if (nbd_get_size (nbd) != -1) { + fprintf (stderr, "expecting error for get_size\n"); + exit (EXIT_FAILURE); + } + if (nbd_is_read_only (nbd) != -1) { + fprintf (stderr, "expecting error for is_read_only\n"); + exit (EXIT_FAILURE); + } + if (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) != -1) { + fprintf (stderr, "expecting error for can_meta_context\n"); + exit (EXIT_FAILURE); + } + + /* info for a different export */ + if (nbd_set_export_name (nbd, "b") == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_opt_info (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_size (nbd)) != 1) { + fprintf (stderr, "expecting size of 1, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_is_read_only (nbd)) != 0) { + fprintf (stderr, "expecting read-write export, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) { + fprintf (stderr, "expecting can_meta_context true, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + + /* go on something not present */ + if (nbd_set_export_name (nbd, "a") == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_opt_go (nbd) != -1) { + fprintf (stderr, "expecting error for opt_go\n"); + exit (EXIT_FAILURE); + } + if (nbd_get_size (nbd) != -1) { + fprintf (stderr, "expecting error for get_size\n"); + exit (EXIT_FAILURE); + } + if (nbd_is_read_only (nbd) != -1) { + fprintf (stderr, "expecting error for is_read_only\n"); + exit (EXIT_FAILURE); + } + if (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) != -1) { + fprintf (stderr, "expecting error for can_meta_context\n"); + exit (EXIT_FAILURE); + } + + /* go on a valid export */ + if (nbd_set_export_name (nbd, "good") == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_opt_go (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_size (nbd)) != 4) { + fprintf (stderr, "expecting size of 4, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_is_read_only (nbd)) != 1) { + fprintf (stderr, "expecting read-only export, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) { + fprintf (stderr, "expecting can_meta_context true, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + + /* now info is no longer valid, but does not wipe data */ + if (nbd_set_export_name (nbd, "a") != -1) { + fprintf (stderr, "expecting error for set_export_name\n"); + exit (EXIT_FAILURE); + } + if ((s = nbd_get_export_name (nbd)) == NULL) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (strcmp (s, "good") != 0) { + fprintf (stderr, "expecting export name 'good', got '%s'\n", s); + exit (EXIT_FAILURE); + } + if (nbd_opt_info (nbd) != -1) { + fprintf (stderr, "expecting error for opt_info\n"); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_size (nbd)) != 4) { + fprintf (stderr, "expecting size of 4, got %" PRId64 "\n", r); + exit (EXIT_FAILURE); + } + + nbd_shutdown (nbd, 0); + nbd_close (nbd); + exit (EXIT_SUCCESS); +} diff --git a/tests/opt-info.sh b/tests/opt-info.sh new file mode 100755 index 0000000..1a8c373 --- /dev/null +++ b/tests/opt-info.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash +# nbd client library in userspace +# Copyright (C) 2019-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 + +# This is used to test nbd_opt_info in various language bindings. +# See tests/opt-info.c and test 230 in language bindings. + +# Export "a" is unavailable, other exports reflect their name as content. +# Export "b" is writeable, others are read-only. +# Thus, size and read-only status can be used to check info/go requests. + +case "$1" in + open) + if test "$3" = a; then + echo ENOENT export not available + exit 1 + fi + echo "$3" ;; + get_size) + printf %s "$2" | wc -c ;; + can_write) + if test "$2" != b; then + exit 3; + fi ;; + pread) + printf %s "$2" | dd bs=1 count=$3 skip=$4 ;; + pwrite) + dd of=/dev/null ;; + *) + exit 2 ;; +esac diff --git a/.gitignore b/.gitignore index 2352bc8..6812fe8 100644 --- a/.gitignore +++ b/.gitignore @@ -173,6 +173,7 @@ Makefile.in /tests/newstyle-limited /tests/oldstyle /tests/opt-abort +/tests/opt-info /tests/opt-list /tests/pki/ /tests/read-only-flag diff --git a/golang/src/libguestfs.org/libnbd/libnbd_230_opt_info_test.go b/golang/src/libguestfs.org/libnbd/libnbd_230_opt_info_test.go new file mode 100644 index 0000000..014a323 --- /dev/null +++ b/golang/src/libguestfs.org/libnbd/libnbd_230_opt_info_test.go @@ -0,0 +1,225 @@ +/* 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 ( + "os" + "testing" +) + +func Test230OptInfo(t *testing.T) { + srcdir := os.Getenv("srcdir") + script := srcdir + "/../../../../tests/opt-info.sh" + + 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", "sh", script, + }) + if err != nil { + t.Fatalf("could not connect: %s", err) + } + + err = h.AddMetaContext(context_base_allocation) + if err != nil { + t.Fatalf("could not add meta context: %s", err) + } + + /* No size, flags, or meta-contexts yet */ + _, err = h.GetSize() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.IsReadOnly() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.CanMetaContext(context_base_allocation) + if err == nil { + t.Fatalf("expected error") + } + + /* info with no prior name gets info on "" */ + err = h.OptInfo() + if err != nil { + t.Fatalf("opt_info failed unexpectedly: %s", err) + } + size, err := h.GetSize() + if err != nil { + t.Fatalf("size failed unexpectedly: %s", err) + } + if size != 0 { + t.Fatalf("unexpected size") + } + ro, err := h.IsReadOnly() + if err != nil { + t.Fatalf("readonly failed unexpectedly: %s", err) + } + if !ro { + t.Fatalf("unexpected readonly") + } + meta, err := h.CanMetaContext(context_base_allocation) + if err != nil { + t.Fatalf("can_meta failed unexpectedly: %s", err) + } + if !meta { + t.Fatalf("unexpected meta context") + } + + /* info on something not present fails, wipes out prior info */ + err = h.SetExportName("a") + if err != nil { + t.Fatalf("set export name failed unexpectedly: %s", err) + } + err = h.OptInfo() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.GetSize() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.IsReadOnly() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.CanMetaContext(context_base_allocation) + if err == nil { + t.Fatalf("expected error") + } + + /* info for a different export */ + err = h.SetExportName("b") + if err != nil { + t.Fatalf("set export name failed unexpectedly: %s", err) + } + err = h.OptInfo() + if err != nil { + t.Fatalf("opt_info failed unexpectedly: %s", err) + } + size, err = h.GetSize() + if err != nil { + t.Fatalf("size failed unexpectedly: %s", err) + } + if size != 1 { + t.Fatalf("unexpected size") + } + ro, err = h.IsReadOnly() + if err != nil { + t.Fatalf("readonly failed unexpectedly: %s", err) + } + if ro { + t.Fatalf("unexpected readonly") + } + meta, err = h.CanMetaContext(context_base_allocation) + if err != nil { + t.Fatalf("can_meta failed unexpectedly: %s", err) + } + if !meta { + t.Fatalf("unexpected meta context") + } + + /* go on something not present */ + err = h.SetExportName("a") + if err != nil { + t.Fatalf("set export name failed unexpectedly: %s", err) + } + err = h.OptGo() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.GetSize() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.IsReadOnly() + if err == nil { + t.Fatalf("expected error") + } + _, err = h.CanMetaContext(context_base_allocation) + if err == nil { + t.Fatalf("expected error") + } + + /* go on a valid export */ + err = h.SetExportName("good") + if err != nil { + t.Fatalf("set export name failed unexpectedly: %s", err) + } + err = h.OptGo() + if err != nil { + t.Fatalf("opt_go failed unexpectedly: %s", err) + } + size, err = h.GetSize() + if err != nil { + t.Fatalf("size failed unexpectedly: %s", err) + } + if size != 4 { + t.Fatalf("unexpected size") + } + ro, err = h.IsReadOnly() + if err != nil { + t.Fatalf("readonly failed unexpectedly: %s", err) + } + if !ro { + t.Fatalf("unexpected readonly") + } + meta, err = h.CanMetaContext(context_base_allocation) + if err != nil { + t.Fatalf("can_meta failed unexpectedly: %s", err) + } + if !meta { + t.Fatalf("unexpected meta context") + } + + /* now info is no longer valid, but does not wipe data */ + err = h.SetExportName("a") + if err == nil { + t.Fatalf("expected error") + } + name, err := h.GetExportName() + if err != nil { + t.Fatalf("get export name failed unexpectedly: %s", err) + } + if *name != "good" { + t.Fatalf("wrong name returned") + } + err = h.OptInfo() + if err == nil { + t.Fatalf("expected error") + } + size, err = h.GetSize() + if err != nil { + t.Fatalf("size failed unexpectedly: %s", err) + } + if size != 4 { + t.Fatalf("unexpected size") + } + + h.Shutdown(nil) +} -- 2.28.0
Eric Blake
2020-Aug-19 01:53 UTC
[Libguestfs] [libnbd PATCH 2/2] info: Use nbd_opt_info for fewer handles during --list
We have now resolved some TODO entries, making nbdinfo --list able to use a single connection except when doing content probing. --- info/nbdinfo.pod | 2 ++ TODO | 13 ----------- info/nbdinfo.c | 56 +++++++++++++++++++++++++++--------------------- 3 files changed, 34 insertions(+), 37 deletions(-) diff --git a/info/nbdinfo.pod b/info/nbdinfo.pod index d0d20a9..19305bf 100644 --- a/info/nbdinfo.pod +++ b/info/nbdinfo.pod @@ -156,6 +156,8 @@ L<qemu-nbd(8)>. Richard W.M. Jones +Eric Blake + =head1 COPYRIGHT Copyright (C) 2020 Red Hat Inc. diff --git a/TODO b/TODO index 239e0b0..4a0cd22 100644 --- a/TODO +++ b/TODO @@ -9,12 +9,8 @@ Example code integrating with ppoll, pollfd, APR pollset (and others?). Example command line utils to copy in/out (like qemu-img convert). -NBD_OPT_INFO mode (like qemu-nbd -L). - NBD resize extension. -NBD_INFO_BLOCK_SIZE. - TLS should properly shut down the session (calling gnutls_bye). Performance: Chart it over various buffer sizes and threads, as that @@ -71,12 +67,3 @@ Suggested API improvements: maybe nbd_shutdown should wait for the subprocess or there should be another API to do this - capture error message when nbd_connect_command fails - - list exports - - It should be possible to get details from each export without - needing to reconnect. This would make nbdinfo --list much more - efficient. This involves issuing an NBD_OPT_INFO request per - export (but only in list mode). See how qemu-nbd -L is - implemented. The main difficulty is how to express this through - the API since we can no longer user nbd_can_* and nbd_is_* to - unpack the flags. diff --git a/info/nbdinfo.c b/info/nbdinfo.c index bd3615e..cdff958 100644 --- a/info/nbdinfo.c +++ b/info/nbdinfo.c @@ -209,11 +209,12 @@ main (int argc, char *argv[]) fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } - /* Disconnect from the server to move the handle into a closed - * state, in case the server serializes further connections. - * But we can ignore errors in this case. - */ - nbd_opt_abort (nbd); + if (probe_content) + /* Disconnect from the server to move the handle into a closed + * state, in case the server serializes further connections. + * But we can ignore errors in this case. + */ + nbd_opt_abort (nbd); } if (size_only) { @@ -456,9 +457,6 @@ list_one_export (struct nbd_handle *nbd, const char *desc, free (export_name); } -/* XXX Inefficient and hacky. See TODO for a suggestion on how to - * improve this. - */ static void list_all_exports (struct nbd_handle *nbd1, const char *uri) { @@ -468,31 +466,41 @@ list_all_exports (struct nbd_handle *nbd1, const char *uri) printf ("\t\"exports\": []\n"); for (i = 0; i < export_list.len; ++i) { - const char *name; + const char *name = export_list.names[i]; struct nbd_handle *nbd2; - /* Connect to the original URI, but using opt mode to alter the export. */ - name = export_list.names[i]; - nbd2 = nbd_create (); - if (nbd2 == NULL) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } - nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */ - nbd_set_opt_mode (nbd2, true); + if (probe_content) { + /* Connect to the original URI, but using opt mode to alter the export. */ + nbd2 = nbd_create (); + if (nbd2 == NULL) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */ + nbd_set_opt_mode (nbd2, true); - if (nbd_connect_uri (nbd2, uri) == -1 || - nbd_set_export_name (nbd2, name) == -1 || - nbd_opt_go (nbd2) == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); + if (nbd_connect_uri (nbd2, uri) == -1 || + nbd_set_export_name (nbd2, name) == -1 || + nbd_opt_go (nbd2) == -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) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + nbd2 = nbd1; } /* List the metadata of this export. */ list_one_export (nbd2, export_list.descs[i], i == 0, i + 1 == export_list.len); - nbd_close (nbd2); + if (probe_content) + nbd_close (nbd2); } } -- 2.28.0
Richard W.M. Jones
2020-Aug-19 12:29 UTC
Re: [Libguestfs] [libnbd PATCH 2/2] info: Use nbd_opt_info for fewer handles during --list
Series seems pretty good to me. If you wanted to push it now that'd be fine. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Possibly Parallel Threads
- [libnbd PATCH v2 0/2] opt_list_meta_context
- [libnbd PATCH v2 00/13] Adding nbd_set_opt_mode to improve nbdinfo
- [libnbd PATCH v2 0/5] Add knobs for client- vs. server-side validation
- [libnbd PATCH 0/3] opt_list_meta_context
- [libnbd PATCH v3 0/2] Implementing NBD_OPT_LIST