[Libguestfs] [libnbd PATCH v3 1/2] api: Add nbd_opt_list

Adding NBD_OPT_LIST support will occur across two commits. This patch switches the existing nbd_set_list_exports() (called at a distance compared to where its results are used) to instead be the synchronous nbd_opt_list() command; the next patch will then focus on making the API friendlier to async usage by shifting from libnbd malloc'ing the results to instead having the caller supply a callback function. Thus, the API signature in this patch is not what we intend to be final for 1.4, but rather a division to make review of the transformation easier. Note that this implementation cannot tell the difference between a successful return of 0 exports, and a server error. That will be fixed in the next patch, with the addition of a completion callback. I've added the same test for all of C, python, ocaml, and go; but even though it passes (when new-enough nbdkit is on PATH), I will admit that the latter two might not be idiomatic. --- lib/internal.h | 3 +- generator/API.ml | 84 +++++------- generator/state_machine.ml | 4 +- generator/states-newstyle-opt-list.c | 12 +- generator/states-newstyle-opt-starttls.c | 8 +- generator/states-newstyle.c | 3 + lib/handle.c | 26 +--- lib/opt.c | 34 +++++ python/t/220-opt-list.py | 59 ++++++++ ocaml/tests/Makefile.am | 3 + ocaml/tests/test_220_opt_list.ml | 76 +++++++++++ tests/Makefile.am | 11 ++ tests/meta-base-allocation.sh | 4 +- tests/newstyle-limited.c | 9 +- tests/opt-list.c | 127 ++++++++++++++++++ tests/opt-list.sh | 44 ++++++ examples/list-exports.c | 22 +-- interop/list-exports.c | 11 +- .gitignore | 1 + .../libnbd/libnbd_220_opt_list_test.go | 114 ++++++++++++++++ .../libnbd/libnbd_460_block_status_test.go | 6 +- info/nbdinfo.c | 33 ++--- 22 files changed, 572 insertions(+), 122 deletions(-) create mode 100644 python/t/220-opt-list.py create mode 100644 ocaml/tests/test_220_opt_list.ml create mode 100644 tests/opt-list.c create mode 100755 tests/opt-list.sh create mode 100644 golang/src/libguestfs.org/libnbd/libnbd_220_opt_list_test.go diff --git a/lib/internal.h b/lib/internal.h index 2a9cfca..b418ccf 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -103,8 +103,7 @@ struct nbd_handle { bool opt_mode; uint8_t opt_current; /* 0 or one of NBD_OPT_* */ - /* List exports mode. */ - bool list_exports; + /* Results of nbd_opt_list. */ size_t nr_exports; struct export *exports; diff --git a/generator/API.ml b/generator/API.ml index 3751c4e..a5c253a 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -288,12 +288,14 @@ The encoding of export names is always UTF-8. When option mode is not in use, the export name must be set 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)>. +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. 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 "set_opt_mode"; Link "opt_go"; Link "opt_list"]; }; "get_export_name", { @@ -709,7 +711,7 @@ 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_abort"; Link "opt_go"; Link "opt_list"]; }; "get_opt_mode", { @@ -754,68 +756,55 @@ enabled option mode."; see_also = [Link "set_opt_mode"; Link "aio_opt_abort"; Link "opt_go"]; }; - "set_list_exports", { + "opt_list", { default_call with - args = [Bool "list"]; ret = RErr; - permitted_states = [ Created ]; - shortdesc = "set whether to list server exports"; + args = []; ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "request the server to list all exports during negotiation"; longdesc = "\ -Set this flag to true in order to list NBD exports provided -by the server. +Request that the server list all exports that it supports. This can +only be used if L<nbd_set_opt_mode(3)> enabled option mode. In this mode, during connection we query the server for the list of NBD exports and collect them in the handle. You can query the list of exports provided by the server by calling L<nbd_get_nr_list_exports(3)> and L<nbd_get_list_export_name(3)>. -After choosing the export you want, you should close this handle, -create a new NBD handle (L<nbd_create(3)>), set the export name -(L<nbd_set_export_name(3)>), and connect on the new handle -(C<nbd_connect_*>). +After choosing the export you want, set the export name +(L<nbd_set_export_name(3)>), then finish connecting with L<nbd_opt_go(3)>. Some servers do not support listing exports at all. In that case the connect call will fail with errno C<ENOTSUP> and L<nbd_get_nr_list_exports(3)> will return 0. -Some servers do not respond with all the exports they -support, either because of an incomplete implementation of -this feature, or because they only list exports relevant -to non-TLS or TLS when a corresponding non-TLS or TLS -connection is opened. - -For L<qemu-nbd(8)> when using the I<-x> option you may find -that the original connect call fails with -C<server has no export named ''> and errno C<ENOENT>. -However you can still proceed to call L<nbd_get_nr_list_exports(3)> etc. +Not all servers understand this request, and even when it is understood, +the server might intentionally send an empty list to avoid being an +information leak or may encounter a failure after delivering partial +results. Thus, this function may succeed even when no exports +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 +exports that might be advertised, so client code should be aware that +a server may send a lengthy list; libnbd truncates the server reply +after 10000 exports. For L<nbd-server(1)> you will need to allow clients to make list requests by adding C<allowlist=true> to the C<[generic]> -section of F</etc/nbd-server/config>."; +section of F</etc/nbd-server/config>. For L<qemu-nbd(8)>, a +description is set with I<-D>."; example = Some "examples/list-exports.c"; - see_also = [Link "get_list_exports"; + see_also = [Link "set_opt_mode"; Link "opt_go"; Link "get_nr_list_exports"; Link "get_list_export_name"]; }; - "get_list_exports", { - default_call with - args = []; ret = RBool; - may_set_error = false; - shortdesc = "return whether list exports mode was enabled"; - longdesc = "\ -Return true if list exports mode was enabled on this handle."; - see_also = [Link "set_list_exports"]; - }; - "get_nr_list_exports", { default_call with args = []; ret = RInt; permitted_states = [ Negotiating; Connected; Closed; Dead ]; shortdesc = "return the number of exports returned by the server"; longdesc = "\ -If list exports mode was enabled on the handle and you connected -to the server, this returns the number of exports returned by the -server. This may be 0 or incomplete for reasons given in -L<nbd_set_list_exports(3)>."; - see_also = [Link "set_list_exports"; Link "get_list_export_name"; +If option mode is in effect and you called L<nbd_opt_list(3)>, +this returns the number of exports returned by the server. This +may be 0 or incomplete for reasons given in L<nbd_opt_list(3)>."; + see_also = [Link "opt_list"; Link "get_list_export_name"; Link "get_list_export_description"]; }; @@ -825,10 +814,10 @@ L<nbd_set_list_exports(3)>."; permitted_states = [ Negotiating; Connected; Closed; Dead ]; shortdesc = "return the i'th export name"; longdesc = "\ -If list exports mode was enabled on the handle and you connected -to the server, this can be used to return the i'th export name +If L<nbd_opt_list(3)> succeeded with option mode enabled, +this can be used to return the i'th export name from the list returned by the server."; - see_also = [Link "set_list_exports"; Link "get_list_export_description"]; + see_also = [Link "opt_list"; Link "get_list_export_description"]; }; "get_list_export_description", { @@ -837,13 +826,13 @@ from the list returned by the server."; permitted_states = [ Negotiating; Connected; Closed; Dead ]; shortdesc = "return the i'th export description"; longdesc = "\ -If list exports mode was enabled on the handle and you connected -to the server, this can be used to return the i'th export description +If L<nbd_opt_list(3)> succeeded with option mode enabled, +this can be used to return the i'th export description from the list returned by the server, which may be an empty string. Many servers omit a description. For L<qemu-nbd(8)>, a description is set with I<-D>."; - see_also = [Link "set_list_exports"; Link "get_list_export_name"]; + see_also = [Link "set_opt_mode"; Link "opt_go"]; }; "add_meta_context", { @@ -2561,8 +2550,6 @@ let first_version = [ "set_uri_allow_local_file", (1, 2); (* Added in 1.3.x development cycle, will be stable and supported in 1.4. *) - "set_list_exports", (1, 4); - "get_list_exports", (1, 4); "get_nr_list_exports", (1, 4); "get_list_export_name", (1, 4); "get_list_export_description", (1, 4); @@ -2576,6 +2563,7 @@ let first_version = [ "aio_is_negotiating", (1, 4); "opt_go", (1, 4); "opt_abort", (1, 4); + "opt_list", (1, 4); "aio_opt_go", (1, 4); "aio_opt_abort", (1, 4); diff --git a/generator/state_machine.ml b/generator/state_machine.ml index 2d28c2e..c1fb073 100644 --- a/generator/state_machine.ml +++ b/generator/state_machine.ml @@ -282,12 +282,14 @@ and newstyle_state_machine = [ * NEGOTIATING after OPT_STRUCTURED_REPLY or any failed OPT_GO. *) Group ("OPT_STARTTLS", newstyle_opt_starttls_state_machine); - Group ("OPT_LIST", newstyle_opt_list_state_machine); Group ("OPT_STRUCTURED_REPLY", newstyle_opt_structured_reply_state_machine); Group ("OPT_SET_META_CONTEXT", newstyle_opt_set_meta_context_state_machine); Group ("OPT_GO", newstyle_opt_go_state_machine); Group ("OPT_EXPORT_NAME", newstyle_opt_export_name_state_machine); + (* Options that can be used during negotiation, when opt_mode is enabled. *) + Group ("OPT_LIST", newstyle_opt_list_state_machine); + (* When NBD_OPT_GO fails, or when opt_mode is enabled, option parsing * can be cleanly ended without moving through the %READY state. *) diff --git a/generator/states-newstyle-opt-list.c b/generator/states-newstyle-opt-list.c index f2846b6..19b353e 100644 --- a/generator/states-newstyle-opt-list.c +++ b/generator/states-newstyle-opt-list.c @@ -18,17 +18,13 @@ /* State machine for sending NBD_OPT_LIST to list exports. * - * This is skipped unless list exports mode was enabled on the handle. + * This is only reached via nbd_opt_list during opt_mode. */ STATE_MACHINE { NEWSTYLE.OPT_LIST.START: assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); - if (!h->list_exports) { - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); - return 0; - } - + assert (h->opt_mode && h->exports && !h->nr_exports); h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); h->sbuf.option.option = htobe32 (NBD_OPT_LIST); h->sbuf.option.optlen = 0; @@ -135,7 +131,7 @@ STATE_MACHINE { case NBD_REP_ACK: /* Finished receiving the list. */ - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); + SET_NEXT_STATE (%.NEGOTIATING); return 0; default: @@ -145,7 +141,7 @@ STATE_MACHINE { } set_error (ENOTSUP, "unexpected response, possibly the server does not " "support listing exports"); - SET_NEXT_STATE (%.DEAD); + SET_NEXT_STATE (%.NEGOTIATING); return 0; } return 0; diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c index d107b74..9eab023 100644 --- a/generator/states-newstyle-opt-starttls.c +++ b/generator/states-newstyle-opt-starttls.c @@ -23,7 +23,7 @@ STATE_MACHINE { assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); /* If TLS was not requested we skip this option and go to the next one. */ if (h->tls == LIBNBD_TLS_DISABLE) { - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } @@ -102,7 +102,7 @@ STATE_MACHINE { debug (h, "server refused TLS (%s), continuing with unencrypted connection", reply == NBD_REP_ERR_POLICY ? "policy" : "not supported"); - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } return 0; @@ -121,7 +121,7 @@ STATE_MACHINE { nbd_internal_crypto_debug_tls_enabled (h); /* Continue with option negotiation. */ - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } /* Continue handshake. */ @@ -144,7 +144,7 @@ STATE_MACHINE { debug (h, "connection is using TLS"); /* Continue with option negotiation. */ - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } /* Continue handshake. */ diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index 2e06c75..290cbf6 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -124,6 +124,9 @@ STATE_MACHINE { else SET_NEXT_STATE (%OPT_SET_META_CONTEXT.START); return 0; + case NBD_OPT_LIST: + SET_NEXT_STATE (%OPT_LIST.START); + return 0; case NBD_OPT_ABORT: if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) { SET_NEXT_STATE (%.DEAD); diff --git a/lib/handle.c b/lib/handle.c index a51b923..c308461 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -288,25 +288,11 @@ nbd_unlocked_get_export_description (struct nbd_handle *h) return r; } -int -nbd_unlocked_set_list_exports (struct nbd_handle *h, bool list) -{ - h->list_exports = list; - return 0; -} - -/* NB: may_set_error = false. */ -int -nbd_unlocked_get_list_exports (struct nbd_handle *h) -{ - return h->list_exports; -} - int nbd_unlocked_get_nr_list_exports (struct nbd_handle *h) { - if (!h->list_exports) { - set_error (EINVAL, "list exports mode not selected on this handle"); + if (!h->exports) { + set_error (EINVAL, "nbd_opt_list not yet run on this handle"); return -1; } return (int) h->nr_exports; @@ -318,8 +304,8 @@ nbd_unlocked_get_list_export_name (struct nbd_handle *h, { char *name; - if (!h->list_exports) { - set_error (EINVAL, "list exports mode not selected on this handle"); + if (!h->exports) { + set_error (EINVAL, "nbd_opt_list not yet run on this handle"); return NULL; } if (i < 0 || i >= (int) h->nr_exports) { @@ -340,8 +326,8 @@ nbd_unlocked_get_list_export_description (struct nbd_handle *h, { char *desc; - if (!h->list_exports) { - set_error (EINVAL, "list exports mode not selected on this handle"); + if (!h->exports) { + set_error (EINVAL, "nbd_opt_list not yet run on this handle"); return NULL; } if (i < 0 || i >= (int) h->nr_exports) { diff --git a/lib/opt.c b/lib/opt.c index 735cb62..72e7ac3 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -20,6 +20,7 @@ #include <stdlib.h> #include <stdbool.h> +#include <errno.h> #include "internal.h" @@ -75,6 +76,39 @@ nbd_unlocked_opt_abort (struct nbd_handle *h) return wait_for_option (h); } +/* Issue NBD_OPT_LIST and wait for the reply. */ +int +nbd_unlocked_opt_list (struct nbd_handle *h) +{ + size_t i; + + if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) { + set_error (ENOTSUP, "server is not using fixed newstyle protocol"); + return -1; + } + + /* Overwrite any previous results */ + if (h->exports) { + for (i = 0; i < h->nr_exports; ++i) { + free (h->exports[i].name); + free (h->exports[i].description); + } + h->nr_exports = 0; + } + else { + h->exports = malloc (sizeof *h->exports); + if (h->exports == NULL) { + set_error (errno, "malloc"); + return -1; + } + } + + h->opt_current = NBD_OPT_LIST; + if (nbd_internal_run (h, cmd_issue) == -1) + debug (h, "option queued, ignoring state machine failure"); + return wait_for_option (h); +} + /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ int nbd_unlocked_aio_opt_go (struct nbd_handle *h) diff --git a/python/t/220-opt-list.py b/python/t/220-opt-list.py new file mode 100644 index 0000000..033eaa1 --- /dev/null +++ b/python/t/220-opt-list.py @@ -0,0 +1,59 @@ +# 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 + +# Require new-enough nbdkit +if os.system ("nbdkit sh --dump-plugin | grep -q has_list_exports=1"): + print ("skipping: nbdkit too old for this test") + exit (0) + +script = ("%s/../tests/opt-list.sh" % os.getenv ("srcdir", ".")) + +h = nbd.NBD () +h.set_opt_mode (True) +h.connect_command (["nbdkit", "-s", "--exit-with-parent", "-v", + "sh", script]) + +# First pass: server fails NBD_OPT_LIST +# XXX We can't tell the difference +h.opt_list () +assert h.get_nr_list_exports () == 0 + +# Second pass: server advertises 'a' and 'b' +h.opt_list () +assert h.get_nr_list_exports () == 2 +assert h.get_list_export_name (0) == "a" +assert h.get_list_export_name (1) == "b" + +# Third pass: server advertises empty list +h.opt_list () +assert h.get_nr_list_exports () == 0 +try: + h.get_list_export_name (0) + assert False +except nbd.Error as ex: + pass + +# Final pass: server advertises 'a' +h.opt_list () +assert h.get_nr_list_exports () == 1 +assert h.get_list_export_name (0) == "a" + +h.opt_abort () diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am index b2bc2f0..3af13a2 100644 --- a/ocaml/tests/Makefile.am +++ b/ocaml/tests/Makefile.am @@ -24,6 +24,7 @@ EXTRA_DIST = \ test_100_handle.ml \ test_200_connect_command.ml \ test_210_opt_abort.ml \ + test_220_opt_list.ml \ test_300_get_size.ml \ test_400_pread.ml \ test_405_pread_structured.ml \ @@ -45,6 +46,7 @@ tests_bc = \ test_100_handle.bc \ test_200_connect_command.bc \ test_210_opt_abort.bc \ + test_220_opt_list.bc \ test_300_get_size.bc \ test_400_pread.bc \ test_405_pread_structured.bc \ @@ -63,6 +65,7 @@ tests_opt = \ test_100_handle.opt \ test_200_connect_command.opt \ test_210_opt_abort.opt \ + test_220_opt_list.opt \ test_300_get_size.opt \ test_400_pread.opt \ test_405_pread_structured.opt \ diff --git a/ocaml/tests/test_220_opt_list.ml b/ocaml/tests/test_220_opt_list.ml new file mode 100644 index 0000000..4aa1980 --- /dev/null +++ b/ocaml/tests/test_220_opt_list.ml @@ -0,0 +1,76 @@ +(* 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-list.sh" srcdir + with + Not_found -> failwith "error: srcdir is not defined" + +let () = + (* Require new-enough nbdkit *) + let cmd = "nbdkit sh --dump-plugin | grep -q has_list_exports=1" in + if Sys.command cmd <> 0 then ( + exit 77 + ); + + let nbd = NBD.create () in + NBD.set_opt_mode nbd true; + NBD.connect_command nbd + ["nbdkit"; "-s"; "--exit-with-parent"; "-v"; + "sh"; script]; + + (* First pass: server fails NBD_OPT_LIST *) + (* XXX We can't tell the difference *) + NBD.opt_list nbd; + let count = NBD.get_nr_list_exports nbd in + assert (count = 0); + + (* Second pass: server advertises 'a' and 'b' *) + NBD.opt_list nbd; + let count = NBD.get_nr_list_exports nbd in + assert (count = 2); + let name = NBD.get_list_export_name nbd 0 in + assert (name = "a"); + let name = NBD.get_list_export_name nbd 1 in + assert (name = "b"); + + (* Third pass: server advertises empty list *) + NBD.opt_list nbd; + let count = NBD.get_nr_list_exports nbd in + assert (count = 0); + ( try + let _ = NBD.get_list_export_name nbd 0 in + assert (false) + with + NBD.Error (errstr, errno) -> () + ); + + (* Final pass: server advertises 'a' *) + NBD.opt_list nbd; + let count = NBD.get_nr_list_exports nbd in + assert (count = 1); + let name = NBD.get_list_export_name nbd 0 in + assert (name = "a"); + + NBD.opt_abort nbd + +let () = Gc.compact () diff --git a/tests/Makefile.am b/tests/Makefile.am index 76b370a..d02b3d9 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-list.sh synch-parallel.sh \ synch-parallel-tls.sh \ $(NULL) @@ -164,6 +165,7 @@ check_PROGRAMS += \ oldstyle \ newstyle-limited \ opt-abort \ + opt-list \ connect-unix \ connect-tcp \ aio-parallel \ @@ -200,6 +202,7 @@ TESTS += \ oldstyle \ newstyle-limited \ opt-abort \ + opt-list \ connect-unix \ connect-tcp \ aio-parallel.sh \ @@ -380,6 +383,14 @@ opt_abort_CPPFLAGS = -I$(top_srcdir)/include opt_abort_CFLAGS = $(WARNINGS_CFLAGS) opt_abort_LDADD = $(top_builddir)/lib/libnbd.la +opt_list_SOURCES = opt-list.c +opt_list_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -DSCRIPT='"$(abs_srcdir)/opt-list.sh"' \ + $(NULL) +opt_list_CFLAGS = $(WARNINGS_CFLAGS) +opt_list_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/meta-base-allocation.sh b/tests/meta-base-allocation.sh index ce79526..aee601d 100755 --- a/tests/meta-base-allocation.sh +++ b/tests/meta-base-allocation.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # nbd client library in userspace -# Copyright (C) 2019 Red Hat Inc. +# 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 @@ -17,7 +17,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA # This is used to test metadata context "base:allocation". -# See tests/meta-base-allocation.c. +# See tests/meta-base-allocation.c and test 460 in language bindings. case "$1" in thread_model) diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c index aa6f87b..f2f1cba 100644 --- a/tests/newstyle-limited.c +++ b/tests/newstyle-limited.c @@ -136,7 +136,14 @@ main (int argc, char *argv[]) fprintf (stderr, "unexpected state after negotiating\n"); exit (EXIT_FAILURE); } - /* XXX Test nbd_opt_list when it is implemented */ + if (nbd_opt_list (nbd) != -1) { + fprintf (stderr, "nbd_opt_list: expected failure\n"); + exit (EXIT_FAILURE); + } + if (nbd_get_errno () != ENOTSUP) { + fprintf (stderr, "%s: wrong errno value after failed opt_list\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-list.c b/tests/opt-list.c new file mode 100644 index 0000000..d8346d7 --- /dev/null +++ b/tests/opt-list.c @@ -0,0 +1,127 @@ +/* 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. */ + +#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 *name; + char *args[] = { "nbdkit", "-s", "--exit-with-parent", "-v", + "sh", SCRIPT, NULL }; + + /* Quick check that nbdkit is new enough */ + if (system ("nbdkit sh --dump-plugin | grep -q has_list_exports=1")) { + fprintf (stderr, "skipping: nbdkit too old for this test\n"); + exit (77); + } + + /* 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: server fails NBD_OPT_LIST. */ + /* XXX We can't tell the difference */ + if (nbd_opt_list (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_nr_list_exports (nbd)) != 0) { + fprintf (stderr, "wrong number of exports, got %" PRId64 " expecting 0\n", + r); + exit (EXIT_FAILURE); + } + + /* Second pass: server advertises 'a' and 'b'. */ + if (nbd_opt_list (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_nr_list_exports (nbd)) != 2) { + fprintf (stderr, "wrong number of exports, got %" PRId64 " expecting 2\n", + r); + exit (EXIT_FAILURE); + } + name = nbd_get_list_export_name (nbd, 0); + if (!name || strcmp (name, "a") != 0) { + fprintf (stderr, "wrong first export %s, expecting 'a'\n", name ?: "(nil)"); + exit (EXIT_FAILURE); + } + free (name); + name = nbd_get_list_export_name (nbd, 1); + if (!name || strcmp (name, "b") != 0) { + fprintf (stderr, "wrong first export %s, expecting 'b'\n", name ?: "(nil)"); + exit (EXIT_FAILURE); + } + free (name); + + /* Third pass: server advertises empty list. */ + if (nbd_opt_list (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_nr_list_exports (nbd)) != 0) { + fprintf (stderr, "wrong number of exports, got %" PRId64 " expecting 0\n", + r); + exit (EXIT_FAILURE); + } + name = nbd_get_list_export_name (nbd, 0); + if (name) { + fprintf (stderr, "expecting error for out of bounds request\n"); + exit (EXIT_FAILURE); + } + + /* Final pass: server advertises 'a'. */ + if (nbd_opt_list (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_nr_list_exports (nbd)) != 1) { + fprintf (stderr, "wrong number of exports, got %" PRId64 " expecting 1\n", + r); + exit (EXIT_FAILURE); + } + name = nbd_get_list_export_name (nbd, 0); + if (!name || strcmp (name, "a") != 0) { + fprintf (stderr, "wrong first export %s, expecting 'a'\n", name ?: "(nil)"); + exit (EXIT_FAILURE); + } + free (name); + + nbd_opt_abort (nbd); + nbd_close (nbd); + exit (EXIT_SUCCESS); +} diff --git a/tests/opt-list.sh b/tests/opt-list.sh new file mode 100755 index 0000000..2f9bf73 --- /dev/null +++ b/tests/opt-list.sh @@ -0,0 +1,44 @@ +#!/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_list in various language bindings. +# See tests/opt-list.c and test 220 in language bindings. + +if test ! -e "$tmpdir/count"; then + echo 0 > "$tmpdir/count" +fi +case "$1" in + list_exports) + read i < "$tmpdir/count" + # XXX nbkdit .list_exports interface not stable, this may need tweaking + if test "$2" = false; then + echo $((i+1)) > "$tmpdir/count" + fi + case $i in + 0) echo EINVAL listing not supported >&2; exit 1 ;; + 1) echo NAMES; echo a; echo b ;; + 2) echo NAMES ;; + *) echo NAMES; echo a ;; + esac ;; + get_size) + echo 512 ;; + pread) + dd bs=1 if=/dev/zero count=$3 ;; + *) + exit 2 ;; +esac diff --git a/examples/list-exports.c b/examples/list-exports.c index 3ec3649..99cef1b 100644 --- a/examples/list-exports.c +++ b/examples/list-exports.c @@ -54,29 +54,31 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* Set opt mode and request list exports. */ + /* Set opt mode. */ nbd_set_opt_mode (nbd, true); - nbd_set_list_exports (nbd, true); /* Connect to the NBD server over a - * Unix domain socket. A side effect of - * connecting is to list the exports. - * This operation can fail normally, so - * we need to check the return value and - * error code. + * Unix domain socket. If we did not + * end up in option mode, then a + * listing is not possible. */ r = nbd_connect_unix (nbd, argv[1]); - if (r == -1 && nbd_get_errno () == ENOTSUP) { + if (r == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } - if (!nbd_aio_is_negotiating (nbd) || - nbd_get_nr_list_exports (nbd) == 0) { + if (!nbd_aio_is_negotiating (nbd)) { fprintf (stderr, "Server does not support " "listing exports.\n"); exit (EXIT_FAILURE); } + /* Grab the export list. */ + if (nbd_opt_list (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + /* Display the list of exports. */ for (i = 0; i < nbd_get_nr_list_exports (nbd); diff --git a/interop/list-exports.c b/interop/list-exports.c index d003ce9..5af1234 100644 --- a/interop/list-exports.c +++ b/interop/list-exports.c @@ -52,8 +52,8 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* Set the list exports mode in the handle. */ - nbd_set_list_exports (nbd, true); + /* Set option mode in the handle. */ + nbd_set_opt_mode (nbd, true); /* Run qemu-nbd. */ char *args[] = { SERVER, SERVER_PARAMS, NULL }; @@ -62,9 +62,11 @@ main (int argc, char *argv[]) #else #define NBD_CONNECT nbd_connect_command #endif - if (NBD_CONNECT (nbd, args) == -1) - /* This is not an error so don't fail here. */ + if (NBD_CONNECT (nbd, args) == -1 || nbd_opt_list (nbd) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); + unlink (tmpfile); + exit (EXIT_FAILURE); + } /* We don't need the temporary file any longer. */ unlink (tmpfile); @@ -98,6 +100,7 @@ main (int argc, char *argv[]) free (desc); } + nbd_opt_abort (nbd); nbd_close (nbd); exit (EXIT_SUCCESS); } diff --git a/.gitignore b/.gitignore index 6fdb362..2352bc8 100644 --- a/.gitignore +++ b/.gitignore @@ -173,6 +173,7 @@ Makefile.in /tests/newstyle-limited /tests/oldstyle /tests/opt-abort +/tests/opt-list /tests/pki/ /tests/read-only-flag /tests/read-write-flag diff --git a/golang/src/libguestfs.org/libnbd/libnbd_220_opt_list_test.go b/golang/src/libguestfs.org/libnbd/libnbd_220_opt_list_test.go new file mode 100644 index 0000000..4bed131 --- /dev/null +++ b/golang/src/libguestfs.org/libnbd/libnbd_220_opt_list_test.go @@ -0,0 +1,114 @@ +/* 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" + "os/exec" + "testing" +) + +func Test220OptList(t *testing.T) { + /* Require new-enough nbdkit */ + srcdir := os.Getenv("srcdir") + script := srcdir + "/../../../../tests/opt-list.sh" + cmd := exec.Command("/bin/sh", "-c", + "nbdkit sh --dump-plugin | grep -q has_list_exports=1") + err := cmd.Run() + if err != nil { + t.Skip("Skipping: nbdkit too old for this test") + } + + 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) + } + + /* First pass: server fails NBD_OPT_LIST */ + /* XXX We can't tell the difference */ + err = h.OptList() + if err != nil { + t.Fatalf("could not request opt_list: %s", err) + } + count, err := h.GetNrListExports() + if err != nil || count != 0 { + t.Fatalf("unexpected count after opt_list: %s", err) + } + + /* Second pass: server advertises 'a' and 'b' */ + err = h.OptList() + if err != nil { + t.Fatalf("could not request opt_list: %s", err) + } + count, err = h.GetNrListExports() + if err != nil || count != 2 { + t.Fatalf("unexpected count after opt_list: %s", err) + } + name, err := h.GetListExportName(0) + if err != nil || *name != "a" { + t.Fatalf("unexpected name after opt_list: %s", err) + } + name, err = h.GetListExportName(1) + if err != nil || *name != "b" { + t.Fatalf("unexpected name after opt_list: %s", err) + } + + /* Third pass: server advertises empty list */ + err = h.OptList() + if err != nil { + t.Fatalf("could not request opt_list: %s", err) + } + count, err = h.GetNrListExports() + if err != nil || count != 0 { + t.Fatalf("unexpected count after opt_list: %s", err) + } + name, err = h.GetListExportName(0) + if err == nil { + t.Fatalf("expecting error after out-of-bounds request") + } + + /* Final pass: server advertises 'a' */ + err = h.OptList() + if err != nil { + t.Fatalf("could not request opt_list: %s", err) + } + count, err = h.GetNrListExports() + if err != nil || count != 1 { + t.Fatalf("unexpected count after opt_list: %s", err) + } + name, err = h.GetListExportName(0) + if err != nil || *name != "a" { + t.Fatalf("unexpected name after opt_list: %s", err) + } + + h.OptAbort() +} diff --git a/golang/src/libguestfs.org/libnbd/libnbd_460_block_status_test.go b/golang/src/libguestfs.org/libnbd/libnbd_460_block_status_test.go index 3bedcae..1abb731 100644 --- a/golang/src/libguestfs.org/libnbd/libnbd_460_block_status_test.go +++ b/golang/src/libguestfs.org/libnbd/libnbd_460_block_status_test.go @@ -25,9 +25,6 @@ import ( "testing" ) -var srcdir = os.Getenv("srcdir") -var script = srcdir + "/../../../../tests/meta-base-allocation.sh" - var entries []uint32 func mcf(metacontext string, offset uint64, e []uint32, error *int) int { @@ -63,6 +60,9 @@ func mc_to_string(a []uint32) string { } func Test460BlockStatus(t *testing.T) { + srcdir := os.Getenv("srcdir") + script := srcdir + "/../../../../tests/meta-base-allocation.sh" + h, err := Create() if err != nil { t.Fatalf("could not create handle: %s", err) diff --git a/info/nbdinfo.c b/info/nbdinfo.c index 394f5ac..cdc0db8 100644 --- a/info/nbdinfo.c +++ b/info/nbdinfo.c @@ -184,21 +184,27 @@ main (int argc, char *argv[]) } nbd_set_uri_allow_local_file (nbd, true); /* Allow ?tls-psk-file. */ - /* If using --list then we set the list_exports mode flag in the - * handle. In this case it can be OK for the connection to fail. - * In particular it can fail because the export in the URI is not - * recognized. - */ + /* If using --list then we need opt mode in the handle. */ if (list_all) - nbd_set_list_exports (nbd, true); + nbd_set_opt_mode (nbd, true); else if (!size_only) nbd_set_full_info (nbd, true); if (nbd_connect_uri (nbd, argv[optind]) == -1) { - if (!list_all || nbd_get_errno () == ENOTSUP) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + if (list_all) { + if (nbd_opt_list (nbd) == -1) { 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 (size_only) { @@ -211,21 +217,10 @@ main (int argc, char *argv[]) printf ("%" PRIi64 "\n", size); } else { - /* Print per-connection fields. - * - * XXX Not always displayed when using --list mode. This is - * because if the connection fails above (as we expect) then the - * handle state is dead and we cannot query these. - */ + /* Print per-connection fields. */ protocol = nbd_get_protocol (nbd); tls_negotiated = nbd_get_tls_negotiated (nbd); - /* Disconnect from the server to move the handle into a closed - * state, in case the server serializes further connections; but - * ignore errors as the connection may already be dead. - */ - nbd_shutdown (nbd, 0); - if (!json_output) { if (protocol) { printf ("protocol: %s", protocol); -- 2.28.0