Eric Blake
2020-Aug-18 02:09 UTC
[Libguestfs] [libnbd PATCH v3 0/2] Implementing NBD_OPT_LIST
This is a subset of my v2 posting, but limited to just the NBD_OPT_LIST handling. The biggest change since v2 is the addition of added unit testing in all four language bindings (C, python, ocaml, golang). The tests require nbdkit built from git on PATH, and may not be entirely idiomatic, but I at least validated that they catch issues (for example, adding an exit statement near the end of the test reliably caused failure). I'm also working on similar unit testing for opt_go/opt_list; I'm leaning more towards having opt_go have a completion handler, if only because opt_list has to have one (with opt_go, you can tell by whether the state is negotiating vs. ready whether it passed or failed, but with info, the state is always negotiating either way). That should get me to the point of figuring out where my work-in-progress posted in patches 12-13 of v2 was going wrong. Eric Blake (2): api: Add nbd_opt_list api: Add nbd_aio_opt_list lib/internal.h | 28 +-- generator/API.ml | 152 +++++++--------- generator/state_machine.ml | 4 +- generator/states-newstyle-opt-list.c | 82 ++++----- generator/states-newstyle-opt-starttls.c | 8 +- generator/states-newstyle.c | 3 + generator/states.c | 18 +- lib/handle.c | 78 +------- lib/opt.c | 74 ++++++++ python/t/220-opt-list.py | 64 +++++++ ocaml/tests/Makefile.am | 3 + ocaml/tests/test_220_opt_list.ml | 78 ++++++++ tests/Makefile.am | 11 ++ tests/meta-base-allocation.sh | 4 +- tests/newstyle-limited.c | 17 +- tests/opt-list.c | 171 ++++++++++++++++++ tests/opt-list.sh | 44 +++++ examples/list-exports.c | 85 ++++++--- interop/list-exports.c | 77 ++++---- .gitignore | 1 + .../libnbd/libnbd_220_opt_list_test.go | 110 +++++++++++ .../libnbd/libnbd_460_block_status_test.go | 6 +- info/nbdinfo.c | 106 +++++++---- 23 files changed, 881 insertions(+), 343 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 -- 2.28.0
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
Eric Blake
2020-Aug-18 02:09 UTC
[Libguestfs] [libnbd PATCH v3 2/2] api: Add nbd_aio_opt_list
This continues the changes for adding NBD_OPT_LIST support. Now,
instead of libnbd malloc'ing storage itself, the user passes a
callback that can handle name/description pairs however it likes, and
we get rid of the artificial cap at 10000 exports. However, the user
will probably end up malloc'ing a list themselves, as we can't call
nbd_set_export_name, or even request NBD_OPT_INFO (in a future patch),
from the context of the callback.
The choice to have a completion callback makes it possible to reflect
server errors back to the user. However, as this is during
negotiation phase, when nothing else is running in parallel, we don't
need the user to call additional APIs just to retire the results;
instead, return values in the callbacks are ignored, and omitting a
completion callback loses out on any server error.
In the future, we may want to add a synchronous wrapper (maybe just
for C?) that makes it easier to grab a malloc'd list of export names
(probably not descriptions), but that requires more work to the
generator to figure out whether we would return a char** list (with
NULL terminator) or return int with a char*** parameter.
---
lib/internal.h | 27 ++--
generator/API.ml | 100 ++++++---------
generator/states-newstyle-opt-list.c | 76 +++++------
generator/states.c | 18 ++-
lib/handle.c | 64 +---------
lib/opt.c | 88 +++++++++----
python/t/220-opt-list.py | 39 +++---
ocaml/tests/test_220_opt_list.ml | 48 +++----
tests/newstyle-limited.c | 10 +-
tests/opt-list.c | 120 ++++++++++++------
examples/list-exports.c | 69 +++++++---
interop/list-exports.c | 68 +++++-----
.../libnbd/libnbd_220_opt_list_test.go | 64 +++++-----
info/nbdinfo.c | 75 +++++++----
14 files changed, 477 insertions(+), 389 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index b418ccf..b2637bd 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -72,6 +72,15 @@ struct export {
char *description;
};
+struct command_cb {
+ union {
+ nbd_extent_callback extent;
+ nbd_chunk_callback chunk;
+ nbd_list_callback list;
+ } fn;
+ nbd_completion_callback completion;
+};
+
struct nbd_handle {
/* Unique name assigned to this handle for debug messages
* (to avoid having to print actual pointers).
@@ -102,10 +111,7 @@ struct nbd_handle {
/* Option negotiation mode. */
bool opt_mode;
uint8_t opt_current; /* 0 or one of NBD_OPT_* */
-
- /* Results of nbd_opt_list. */
- size_t nr_exports;
- struct export *exports;
+ struct command_cb opt_cb;
/* Full info mode. */
bool full_info;
@@ -186,7 +192,7 @@ struct nbd_handle {
union {
struct {
struct nbd_fixed_new_option_reply_server server;
- char str[NBD_MAX_STRING * 2]; /* name and description */
+ char str[NBD_MAX_STRING * 2 + 1]; /* name, description, NUL */
} __attribute__((packed)) server;
struct nbd_fixed_new_option_reply_info_export export;
struct nbd_fixed_new_option_reply_info_block_size block_size;
@@ -324,14 +330,6 @@ struct socket {
const struct socket_ops *ops;
};
-struct command_cb {
- union {
- nbd_extent_callback extent;
- nbd_chunk_callback chunk;
- } fn;
- nbd_completion_callback completion;
-};
-
struct command {
struct command *next;
uint16_t flags;
@@ -420,6 +418,9 @@ extern bool nbd_internal_is_state_processing (enum state
state);
extern bool nbd_internal_is_state_dead (enum state state);
extern bool nbd_internal_is_state_closed (enum state state);
+/* opt.c */
+extern void nbd_internal_free_option (struct nbd_handle *h);
+
/* protocol.c */
extern int nbd_internal_errno_of_nbd_error (uint32_t error);
extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type);
diff --git a/generator/API.ml b/generator/API.ml
index a5c253a..76e9793 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -127,8 +127,12 @@ let extent_closure = {
"nr_entries");
CBMutable (Int "error") ]
}
+let list_closure = {
+ cbname = "list";
+ cbargs = [ CBString "name"; CBString "description" ]
+}
let all_closures = [ chunk_closure; completion_closure;
- debug_closure; extent_closure ]
+ debug_closure; extent_closure; list_closure ]
(* Enums. *)
let tls_enum = {
@@ -758,23 +762,24 @@ enabled option mode.";
"opt_list", {
default_call with
- args = []; ret = RErr;
+ args = [ Closure list_closure ]; ret = RInt;
permitted_states = [ Negotiating ];
shortdesc = "request the server to list all exports during
negotiation";
longdesc = "\
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, set the export name
-(L<nbd_set_export_name(3)>), then finish connecting with
L<nbd_opt_go(3)>.
+The <list> function is called once per advertised export, with any
+C<user_data> passed to this function, and with C<name> and
C<description>
+supplied by the server. Many servers omit descriptions, in which
+case C<description> will be an empty string. Remember that it is not
+safe to call L<nbd_set_export_name(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.
-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.
+For convenience, when this function succeeds, it returns the number
+of exports that were advertised by the server.
Not all servers understand this request, and even when it is understood,
the server might intentionally send an empty list to avoid being an
@@ -783,56 +788,15 @@ 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.
+a server may send a lengthy list.
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>. For L<qemu-nbd(8)>, a
description is set with I<-D>.";
example = Some "examples/list-exports.c";
- see_also = [Link "set_opt_mode"; Link "opt_go";
- Link "get_nr_list_exports"; Link
"get_list_export_name"];
- };
-
- "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 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"];
- };
-
- "get_list_export_name", {
- default_call with
- args = [ Int "i" ]; ret = RString;
- permitted_states = [ Negotiating; Connected; Closed; Dead ];
- shortdesc = "return the i'th export name";
- longdesc = "\
-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 "opt_list"; Link
"get_list_export_description"];
- };
-
- "get_list_export_description", {
- default_call with
- args = [ Int "i" ]; ret = RString;
- permitted_states = [ Negotiating; Connected; Closed; Dead ];
- shortdesc = "return the i'th export description";
- longdesc = "\
-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_opt_mode"; Link "opt_go"];
+ see_also = [Link "set_opt_mode"; Link "aio_opt_list";
Link "opt_go";
+ Link "set_export_name"];
};
"add_meta_context", {
@@ -1942,6 +1906,28 @@ L<nbd_aio_is_connecting(3)> to return false.";
see_also = [Link "set_opt_mode"; Link "opt_abort"];
};
+ "aio_opt_list", {
+ default_call with
+ args = [ Closure list_closure ];
+ optargs = [ OClosure completion_closure ];
+ ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server to list all exports during
negotiation";
+ longdesc = "\
+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.
+
+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"];
+ };
+
"aio_pread", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
@@ -2550,9 +2536,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. *)
- "get_nr_list_exports", (1, 4);
- "get_list_export_name", (1, 4);
- "get_list_export_description", (1, 4);
"get_block_size", (1, 4);
"set_full_info", (1, 4);
"get_full_info", (1, 4);
@@ -2566,6 +2549,7 @@ let first_version = [
"opt_list", (1, 4);
"aio_opt_go", (1, 4);
"aio_opt_abort", (1, 4);
+ "aio_opt_list", (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-list.c
b/generator/states-newstyle-opt-list.c
index 19b353e..a549bdc 100644
--- a/generator/states-newstyle-opt-list.c
+++ b/generator/states-newstyle-opt-list.c
@@ -24,7 +24,8 @@
STATE_MACHINE {
NEWSTYLE.OPT_LIST.START:
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
- assert (h->opt_mode && h->exports &&
!h->nr_exports);
+ assert (h->opt_mode && h->opt_current == NBD_OPT_LIST);
+ assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.list));
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
h->sbuf.option.option = htobe32 (NBD_OPT_LIST);
h->sbuf.option.optlen = 0;
@@ -67,17 +68,21 @@ STATE_MACHINE {
uint32_t reply;
uint32_t len;
uint32_t elen;
- struct export exp;
- struct export *new_exports;
+ const char *name;
+ const char *desc;
+ char *tmp;
+ int err;
reply = be32toh (h->sbuf.or.option_reply.reply);
len = be32toh (h->sbuf.or.option_reply.replylen);
switch (reply) {
case NBD_REP_SERVER:
/* Got one export. */
- if (len > maxpayload)
+ if (len >= maxpayload)
debug (h, "skipping too large export name reply");
else {
+ /* server.str is oversized for trailing NUL byte convenience */
+ h->sbuf.or.payload.server.str[len - 4] = '\0';
elen = be32toh (h->sbuf.or.payload.server.server.export_name_len);
if (elen > len - 4 || elen > NBD_MAX_STRING ||
len - 4 - elen > NBD_MAX_STRING) {
@@ -85,42 +90,23 @@ STATE_MACHINE {
SET_NEXT_STATE (%.DEAD);
return 0;
}
- /* Copy the export name and description to the handle list. */
- exp.name = strndup (h->sbuf.or.payload.server.str, elen);
- if (exp.name == NULL) {
- set_error (errno, "strdup");
- SET_NEXT_STATE (%.DEAD);
- return 0;
+ if (elen == len + 4) {
+ tmp = NULL;
+ name = h->sbuf.or.payload.server.str;
+ desc = "";
}
- exp.description = strndup (h->sbuf.or.payload.server.str + elen,
- len - 4 - elen);
- if (exp.description == NULL) {
- set_error (errno, "strdup");
- free (exp.name);
- SET_NEXT_STATE (%.DEAD);
- return 0;
+ else {
+ tmp = strndup (h->sbuf.or.payload.server.str, elen);
+ if (tmp == NULL) {
+ set_error (errno, "strdup");
+ SET_NEXT_STATE (%.DEAD);
+ return 0;
+ }
+ name = tmp;
+ desc = h->sbuf.or.payload.server.str + elen;
}
- new_exports = realloc (h->exports,
- sizeof (*new_exports) * (h->nr_exports+1));
- if (new_exports == NULL) {
- set_error (errno, "strdup");
- SET_NEXT_STATE (%.DEAD);
- free (exp.name);
- free (exp.description);
- return 0;
- }
- h->exports = new_exports;
- h->exports[h->nr_exports++] = exp;
- }
-
- /* Just limit this so we don't receive unlimited amounts
- * of data from the server. Note each export name can be
- * up to 4K.
- */
- if (h->nr_exports > 10000) {
- set_error (0, "too many export names sent by the server");
- SET_NEXT_STATE (%.DEAD);
- return 0;
+ CALL_CALLBACK (h->opt_cb.fn.list, name, desc);
+ free (tmp);
}
/* Wait for more replies. */
@@ -131,19 +117,23 @@ STATE_MACHINE {
case NBD_REP_ACK:
/* Finished receiving the list. */
- SET_NEXT_STATE (%.NEGOTIATING);
- return 0;
+ err = 0;
+ break;
default:
if (handle_reply_error (h) == -1) {
SET_NEXT_STATE (%.DEAD);
return 0;
}
- set_error (ENOTSUP, "unexpected response, possibly the server does not
"
+ err = ENOTSUP;
+ set_error (err, "unexpected response, possibly the server does not
"
"support listing exports");
- SET_NEXT_STATE (%.NEGOTIATING);
- return 0;
+ break;
}
+
+ CALL_CALLBACK (h->opt_cb.completion, &err);
+ nbd_internal_free_option (h);
+ SET_NEXT_STATE (%.NEGOTIATING);
return 0;
} /* END STATE MACHINE */
diff --git a/generator/states.c b/generator/states.c
index d2bb9b5..aa40467 100644
--- a/generator/states.c
+++ b/generator/states.c
@@ -1,5 +1,5 @@
/* nbd client library in userspace: state machine
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * 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
@@ -111,9 +111,19 @@ send_from_wbuf (struct nbd_handle *h)
return 0; /* move to next state */
}
+/* Forcefully fail any in-flight option */
+static void
+abort_option (struct nbd_handle *h)
+{
+ int err = nbd_get_errno () ? : ENOTCONN;
+
+ CALL_CALLBACK (h->opt_cb.completion, &err);
+ nbd_internal_free_option (h);
+}
+
/* Forcefully fail any remaining in-flight commands in list */
-void abort_commands (struct nbd_handle *h,
- struct command **list)
+static void
+abort_commands (struct nbd_handle *h, struct command **list)
{
struct command *next, *cmd;
@@ -168,6 +178,7 @@ STATE_MACHINE {
DEAD:
/* The caller should have used set_error() before reaching here */
assert (nbd_get_error ());
+ abort_option (h);
abort_commands (h, &h->cmds_to_issue);
abort_commands (h, &h->cmds_in_flight);
h->in_flight = 0;
@@ -178,6 +189,7 @@ STATE_MACHINE {
return -1;
CLOSED:
+ abort_option (h);
abort_commands (h, &h->cmds_to_issue);
abort_commands (h, &h->cmds_in_flight);
h->in_flight = 0;
diff --git a/lib/handle.c b/lib/handle.c
index c308461..7987d59 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -112,8 +112,6 @@ nbd_create (void)
void
nbd_close (struct nbd_handle *h)
{
- size_t i;
-
nbd_internal_set_error_context ("nbd_close");
if (h == NULL)
@@ -126,13 +124,7 @@ nbd_close (struct nbd_handle *h)
free (h->bs_entries);
nbd_internal_reset_size_and_flags (h);
- for (i = 0; i < h->nr_exports; ++i) {
- free (h->exports[i].name);
- free (h->exports[i].description);
- }
- free (h->exports);
- free (h->canonical_name);
- free (h->description);
+ nbd_internal_free_option (h);
free_cmd_list (h->cmds_to_issue);
free_cmd_list (h->cmds_in_flight);
free_cmd_list (h->cmds_done);
@@ -288,60 +280,6 @@ nbd_unlocked_get_export_description (struct nbd_handle *h)
return r;
}
-int
-nbd_unlocked_get_nr_list_exports (struct nbd_handle *h)
-{
- if (!h->exports) {
- set_error (EINVAL, "nbd_opt_list not yet run on this handle");
- return -1;
- }
- return (int) h->nr_exports;
-}
-
-char *
-nbd_unlocked_get_list_export_name (struct nbd_handle *h,
- int i)
-{
- char *name;
-
- 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) {
- set_error (EINVAL, "invalid index");
- return NULL;
- }
- name = strdup (h->exports[i].name);
- if (!name) {
- set_error (errno, "strdup");
- return NULL;
- }
- return name;
-}
-
-char *
-nbd_unlocked_get_list_export_description (struct nbd_handle *h,
- int i)
-{
- char *desc;
-
- 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) {
- set_error (EINVAL, "invalid index");
- return NULL;
- }
- desc = strdup (h->exports[i].description);
- if (!desc) {
- set_error (errno, "strdup");
- return NULL;
- }
- return desc;
-}
-
int
nbd_unlocked_add_meta_context (struct nbd_handle *h, const char *name)
{
diff --git a/lib/opt.c b/lib/opt.c
index 72e7ac3..f8055d1 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -20,10 +20,21 @@
#include <stdlib.h>
#include <stdbool.h>
+#include <limits.h>
#include <errno.h>
+#include <assert.h>
#include "internal.h"
+/* Internal function which frees an option with callback. */
+void
+nbd_internal_free_option (struct nbd_handle *h)
+{
+ if (h->opt_current == NBD_OPT_LIST)
+ FREE_CALLBACK (h->opt_cb.fn.list);
+ FREE_CALLBACK (h->opt_cb.completion);
+}
+
int
nbd_unlocked_set_opt_mode (struct nbd_handle *h, bool value)
{
@@ -76,37 +87,47 @@ nbd_unlocked_opt_abort (struct nbd_handle *h)
return wait_for_option (h);
}
+struct list_helper {
+ int count;
+ nbd_list_callback list;
+ int err;
+};
+static int
+list_visitor (void *opaque, const char *name, const char *description)
+{
+ struct list_helper *h = opaque;
+ if (h->count < INT_MAX)
+ h->count++;
+ CALL_CALLBACK (h->list, name, description);
+ return 0;
+}
+static int
+list_complete (void *opaque, int *err)
+{
+ struct list_helper *h = opaque;
+ h->err = *err;
+ FREE_CALLBACK (h->list);
+ return 0;
+}
+
/* Issue NBD_OPT_LIST and wait for the reply. */
int
-nbd_unlocked_opt_list (struct nbd_handle *h)
+nbd_unlocked_opt_list (struct nbd_handle *h, nbd_list_callback list)
{
- size_t i;
+ struct list_helper s = { .list = list };
+ nbd_list_callback l = { .callback = list_visitor, .user_data = &s };
+ nbd_completion_callback c = { .callback = list_complete, .user_data = &s
};
- if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
- set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
+ if (nbd_unlocked_aio_opt_list (h, l, c) == -1)
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;
- }
+ if (wait_for_option (h) == -1)
+ return -1;
+ if (s.err) {
+ set_error (s.err, "server replied with error to list request");
+ 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);
+ return s.count;
}
/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */
@@ -130,3 +151,22 @@ nbd_unlocked_aio_opt_abort (struct nbd_handle *h)
debug (h, "option queued, ignoring state machine failure");
return 0;
}
+
+/* Issue NBD_OPT_LIST without waiting. */
+int
+nbd_unlocked_aio_opt_list (struct nbd_handle *h, nbd_list_callback list,
+ nbd_completion_callback complete)
+{
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
+ return -1;
+ }
+
+ assert (CALLBACK_IS_NULL (h->opt_cb.fn.list));
+ h->opt_cb.fn.list = list;
+ h->opt_cb.completion = complete;
+ h->opt_current = NBD_OPT_LIST;
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
diff --git a/python/t/220-opt-list.py b/python/t/220-opt-list.py
index 033eaa1..d97edfb 100644
--- a/python/t/220-opt-list.py
+++ b/python/t/220-opt-list.py
@@ -31,29 +31,34 @@ h.set_opt_mode (True)
h.connect_command (["nbdkit", "-s",
"--exit-with-parent", "-v",
"sh", script])
+exports = []
+def f (user_data, name, desc):
+ global exports
+ assert user_data == 42
+ assert desc == ""
+ exports.append(name)
+
# 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)
+ h.opt_list (lambda *args: f (42, *args))
assert False
except nbd.Error as ex:
pass
+assert exports == []
+
+# Second pass: server advertises 'a' and 'b'
+exports = []
+assert h.opt_list (lambda *args: f (42, *args)) == 2
+assert exports == [ "a", "b" ]
+
+# Third pass: server advertises empty list
+exports = []
+assert h.opt_list (lambda *args: f (42, *args)) == 0
+assert exports == []
# Final pass: server advertises 'a'
-h.opt_list ()
-assert h.get_nr_list_exports () == 1
-assert h.get_list_export_name (0) == "a"
+exports = []
+assert h.opt_list (lambda *args: f (42, *args)) == 1
+assert exports == [ "a" ]
h.opt_abort ()
diff --git a/ocaml/tests/test_220_opt_list.ml b/ocaml/tests/test_220_opt_list.ml
index 4aa1980..aa7e7a9 100644
--- a/ocaml/tests/test_220_opt_list.ml
+++ b/ocaml/tests/test_220_opt_list.ml
@@ -25,6 +25,13 @@ let script with
Not_found -> failwith "error: srcdir is not defined"
+let exports = ref []
+let f user_data name desc + assert (user_data = 42);
+ assert (desc = "");
+ exports := !exports @ [name];
+ 0
+
let () (* Require new-enough nbdkit *)
let cmd = "nbdkit sh --dump-plugin | grep -q has_list_exports=1" in
@@ -39,37 +46,32 @@ let () "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);
+ exports := [];
( try
- let _ = NBD.get_list_export_name nbd 0 in
+ let _ = NBD.opt_list nbd (f 42) in
assert (false)
with
NBD.Error (errstr, errno) -> ()
);
+ assert (!exports = []);
+
+ (* Second pass: server advertises 'a' and 'b' *)
+ exports := [];
+ let count = NBD.opt_list nbd (f 42) in
+ assert (count = 2);
+ assert (!exports = [ "a"; "b" ]);
+
+ (* Third pass: server advertises empty list *)
+ exports := [];
+ let count = NBD.opt_list nbd (f 42) in
+ assert (count = 0);
+ assert (!exports = []);
(* Final pass: server advertises 'a' *)
- NBD.opt_list nbd;
- let count = NBD.get_nr_list_exports nbd in
+ exports := [];
+ let count = NBD.opt_list nbd (f 42) in
assert (count = 1);
- let name = NBD.get_list_export_name nbd 0 in
- assert (name = "a");
+ assert (!exports = [ "a" ]);
NBD.opt_abort nbd
diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c
index f2f1cba..ab6c1c2 100644
--- a/tests/newstyle-limited.c
+++ b/tests/newstyle-limited.c
@@ -76,6 +76,14 @@ pread_cb (void *data,
return 0;
}
+static int
+list_cb (void *opaque, const char *name, const char *descriptiong)
+{
+ /* This callback is unreachable; plain newstyle can't do OPT_LIST */
+ fprintf (stderr, "%s: list callback mistakenly reached", progname);
+ exit (EXIT_FAILURE);
+}
+
int
main (int argc, char *argv[])
{
@@ -136,7 +144,7 @@ main (int argc, char *argv[])
fprintf (stderr, "unexpected state after negotiating\n");
exit (EXIT_FAILURE);
}
- if (nbd_opt_list (nbd) != -1) {
+ if (nbd_opt_list (nbd, (nbd_list_callback) { .callback = list_cb }) != -1) {
fprintf (stderr, "nbd_opt_list: expected failure\n");
exit (EXIT_FAILURE);
}
diff --git a/tests/opt-list.c b/tests/opt-list.c
index d8346d7..7a730a9 100644
--- a/tests/opt-list.c
+++ b/tests/opt-list.c
@@ -29,14 +29,75 @@
#include <libnbd.h>
+struct progress {
+ int id;
+ int visit;
+};
+
+static int
+check (void *user_data, const char *name, const char *description)
+{
+ struct progress *p = user_data;
+
+ if (*description) {
+ fprintf (stderr, "unexpected description for id %d visit %d:
%s\n",
+ p->id, p->visit, description);
+ exit (EXIT_FAILURE);
+ }
+
+ switch (p->id) {
+ case 0:
+ fprintf (stderr, "callback shouldn't be reached when server has
error\n");
+ exit (EXIT_FAILURE);
+ case 1:
+ switch (p->visit) {
+ case 0:
+ if (strcmp (name, "a") != 0) {
+ fprintf (stderr, "unexpected name '%s', expecting
'a'\n", name);
+ exit (EXIT_FAILURE);
+ }
+ break;
+ case 1:
+ if (strcmp (name, "b") != 0) {
+ fprintf (stderr, "unexpected name '%s', expecting
'b'\n", name);
+ exit (EXIT_FAILURE);
+ }
+ break;
+ default:
+ fprintf (stderr, "callback reached too many times\n");
+ exit (EXIT_FAILURE);
+ }
+ break;
+ case 2:
+ fprintf (stderr, "callback shouldn't be reached when list is
empty\n");
+ exit (EXIT_FAILURE);
+ case 3:
+ if (p->visit != 0) {
+ fprintf (stderr, "callback reached too many times\n");
+ exit (EXIT_FAILURE);
+ }
+ if (strcmp (name, "a") != 0) {
+ fprintf (stderr, "unexpected name '%s', expecting
'a'\n", name);
+ exit (EXIT_FAILURE);
+ }
+ break;
+ default:
+ fprintf (stderr, "callback reached with unexpected id %d\n",
p->id);
+ exit (EXIT_FAILURE);
+ }
+
+ p->visit++;
+ return 0;
+}
+
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 };
+ struct progress p;
/* Quick check that nbdkit is new enough */
if (system ("nbdkit sh --dump-plugin | grep -q
has_list_exports=1")) {
@@ -54,72 +115,55 @@ main (int argc, char *argv[])
}
/* 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);
+ p = (struct progress) { .id = 0 };
+ r = nbd_opt_list (nbd, (nbd_list_callback) { .callback = check,
+ .user_data = &p});
+ if (r != -1) {
+ fprintf (stderr, "expected error after opt_list\n");
exit (EXIT_FAILURE);
}
/* Second pass: server advertises 'a' and 'b'. */
- if (nbd_opt_list (nbd) == -1) {
+ p = (struct progress) { .id = 1 };
+ r = nbd_opt_list (nbd, (nbd_list_callback) { .callback = check,
+ .user_data = &p});
+ if (r == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
- if ((r = nbd_get_nr_list_exports (nbd)) != 2) {
+ else if (r != 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) {
+ p = (struct progress) { .id = 2 };
+ r = nbd_opt_list (nbd, (nbd_list_callback) { .callback = check,
+ .user_data = &p});
+ if (r == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
- if ((r = nbd_get_nr_list_exports (nbd)) != 0) {
+ else if (r != 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) {
+ p = (struct progress) { .id = 3 };
+ r = nbd_opt_list (nbd, (nbd_list_callback) { .callback = check,
+ .user_data = &p});
+ if (r == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
- if ((r = nbd_get_nr_list_exports (nbd)) != 1) {
+ else if (r != 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);
diff --git a/examples/list-exports.c b/examples/list-exports.c
index 99cef1b..f6200f4 100644
--- a/examples/list-exports.c
+++ b/examples/list-exports.c
@@ -29,18 +29,52 @@
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
+#include <string.h>
#include <inttypes.h>
#include <errno.h>
#include <libnbd.h>
+struct export_list {
+ int i;
+ char **names;
+};
+
+/* Callback function for nbd_opt_list */
+static int
+list_one (void *opaque, const char *name,
+ const char *description)
+{
+ struct export_list *l = opaque;
+ char **names;
+
+ printf ("[%d] %s\n", l->i, name);
+ if (*description)
+ printf(" (%s)\n", description);
+ names = realloc (l->names,
+ (l->i + 1) * sizeof *names);
+ if (!names) {
+ perror ("realloc");
+ exit (EXIT_FAILURE);
+ }
+ names[l->i] = strdup (name);
+ if (!names[l->i]) {
+ perror ("strdup");
+ exit (EXIT_FAILURE);
+ }
+ l->names = names;
+ l->i++;
+ return 0;
+}
+
int
main (int argc, char *argv[])
{
struct nbd_handle *nbd;
- int r, i;
- char *name, *desc;
+ int i;
+ const char *name;
int64_t size;
+ struct export_list list = { 0 };
if (argc != 2) {
fprintf (stderr, "%s socket\n", argv[0]);
@@ -62,8 +96,7 @@ main (int argc, char *argv[])
* end up in option mode, then a
* listing is not possible.
*/
- r = nbd_connect_unix (nbd, argv[1]);
- if (r == -1) {
+ if (nbd_connect_unix (nbd, argv[1]) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
@@ -73,24 +106,16 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- /* Grab the export list. */
- if (nbd_opt_list (nbd) == -1) {
+ /* Print the export list. */
+ if (nbd_opt_list (nbd,
+ (nbd_list_callback) {
+ .callback = list_one,
+ .user_data = &list, }) == -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);
- i++) {
- name = nbd_get_list_export_name (nbd, i);
- printf ("[%d] %s\n", i, name);
- desc = nbd_get_list_export_description (nbd, i);
- if (desc && *desc)
- printf(" (%s)\n", desc);
- free (name);
- free (desc);
- }
printf ("Which export to connect to? ");
if (scanf ("%d", &i) != 1) exit (EXIT_FAILURE);
if (i == -1) {
@@ -101,11 +126,11 @@ main (int argc, char *argv[])
nbd_close (nbd);
exit (EXIT_SUCCESS);
}
- name = nbd_get_list_export_name (nbd, i);
- if (name == NULL) {
- fprintf (stderr, "%s\n", nbd_get_error ());
+ if (i < 0 || i >= list.i) {
+ fprintf (stderr, "index %d out of range", i);
exit (EXIT_FAILURE);
}
+ name = list.names[i];
printf ("Connecting to %s ...\n", name);
/* Resume connecting to the chosen export. */
@@ -131,7 +156,9 @@ main (int argc, char *argv[])
/* Close the libnbd handle. */
nbd_close (nbd);
- free (name);
+ for (i = 0; i < list.i; i++)
+ free (list.names[i]);
+ free (list.names);
exit (EXIT_SUCCESS);
}
diff --git a/interop/list-exports.c b/interop/list-exports.c
index 5af1234..44d5664 100644
--- a/interop/list-exports.c
+++ b/interop/list-exports.c
@@ -27,14 +27,44 @@
#include <libnbd.h>
+static const char *exports[] = { EXPORTS };
+static const char *descriptions[] = { DESCRIPTIONS };
+static const size_t nr_exports = sizeof exports / sizeof exports[0];
+
+static char *progname;
+
+static int
+check (void *opaque, const char *name, const char *description)
+{
+ size_t *i = opaque;
+ if (*i == nr_exports) {
+ fprintf (stderr, "%s: server returned more exports than
expected",
+ progname);
+ exit (EXIT_FAILURE);
+ }
+ if (strcmp (exports[*i], name) != 0) {
+ fprintf (stderr, "%s: expected export \"%s\", but got
\"%s\"\n",
+ progname, exports[*i], name);
+ exit (EXIT_FAILURE);
+ }
+ if (strcmp (descriptions[*i], description) != 0) {
+ fprintf (stderr, "%s: expected description \"%s\", but got
\"%s\"\n",
+ progname, descriptions[*i], description);
+ exit (EXIT_FAILURE);
+ }
+ (*i)++;
+ return 0;
+}
+
int
main (int argc, char *argv[])
{
struct nbd_handle *nbd;
char tmpfile[] = "/tmp/nbdXXXXXX";
- int fd, r;
- size_t i;
- char *name, *desc;
+ int fd;
+ size_t i = 0;
+
+ progname = argv[0];
/* Create a sparse temporary file. */
fd = mkstemp (tmpfile);
@@ -62,7 +92,9 @@ main (int argc, char *argv[])
#else
#define NBD_CONNECT nbd_connect_command
#endif
- if (NBD_CONNECT (nbd, args) == -1 || nbd_opt_list (nbd) == -1) {
+ if (NBD_CONNECT (nbd, args) == -1 ||
+ nbd_opt_list (nbd, (nbd_list_callback) {
+ .callback = check, .user_data = &i}) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
unlink (tmpfile);
exit (EXIT_FAILURE);
@@ -72,34 +104,12 @@ main (int argc, char *argv[])
unlink (tmpfile);
/* Check for expected number of exports. */
- const char *exports[] = { EXPORTS };
- const char *descriptions[] = { DESCRIPTIONS };
- const size_t nr_exports = sizeof exports / sizeof exports[0];
- r = nbd_get_nr_list_exports (nbd);
- if (r != nr_exports) {
- fprintf (stderr, "%s: expected %zu export, but got %d\n",
- argv[0], nr_exports, r);
+ if (i != nr_exports) {
+ fprintf (stderr, "%s: expected %zu export, but got %zu\n",
+ argv[0], nr_exports, i);
exit (EXIT_FAILURE);
}
- /* Check the export names and descriptions. */
- for (i = 0; i < nr_exports; ++i) {
- name = nbd_get_list_export_name (nbd, (int) i);
- if (strcmp (name, exports[i]) != 0) {
- fprintf (stderr, "%s: expected export \"%s\", but got
\"%s\"\n",
- argv[0], exports[i], name);
- exit (EXIT_FAILURE);
- }
- free (name);
- desc = nbd_get_list_export_description (nbd, (int) i);
- if (strcmp (desc, descriptions[i]) != 0) {
- fprintf (stderr, "%s: expected description \"%s\", but got
\"%s\"\n",
- argv[0], descriptions[i], desc);
- exit (EXIT_FAILURE);
- }
- free (desc);
- }
-
nbd_opt_abort (nbd);
nbd_close (nbd);
exit (EXIT_SUCCESS);
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
index 4bed131..b390ba5 100644
--- a/golang/src/libguestfs.org/libnbd/libnbd_220_opt_list_test.go
+++ b/golang/src/libguestfs.org/libnbd/libnbd_220_opt_list_test.go
@@ -24,6 +24,16 @@ import (
"testing"
)
+var exports []string
+
+func listf(name string, desc string) int {
+ if desc != "" {
+ panic("expected empty description")
+ }
+ exports = append(exports, name)
+ return 0
+}
+
func Test220OptList(t *testing.T) {
/* Require new-enough nbdkit */
srcdir := os.Getenv("srcdir")
@@ -54,60 +64,46 @@ func Test220OptList(t *testing.T) {
}
/* 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)
+ exports = make([]string, 0, 4)
+ _, err = h.OptList(listf)
+ if err == nil {
+ t.Fatalf("expected error")
}
/* Second pass: server advertises 'a' and 'b' */
- err = h.OptList()
+ exports = make([]string, 0, 4)
+ count, err := h.OptList(listf)
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)
+ if count != 2 {
+ t.Fatalf("unexpected count after opt_list")
}
- 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)
+ if len(exports) != 2 || exports[0] != "a" || exports[1] !=
"b" {
+ t.Fatalf("unexpected exports contents after opt_list")
}
/* Third pass: server advertises empty list */
- err = h.OptList()
+ exports = make([]string, 0, 4)
+ count, err = h.OptList(listf)
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")
+ if count != 0 {
+ t.Fatalf("unexpected count after opt_list")
}
/* Final pass: server advertises 'a' */
- err = h.OptList()
+ exports = make([]string, 0, 4)
+ count, err = h.OptList(listf)
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)
+ if count != 1 {
+ t.Fatalf("unexpected count after opt_list")
}
- name, err = h.GetListExportName(0)
- if err != nil || *name != "a" {
- t.Fatalf("unexpected name after opt_list: %s", err)
+ if len(exports) != 1 || exports[0] != "a" {
+ t.Fatalf("unexpected exports contents after opt_list")
}
h.OptAbort()
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index cdc0db8..81913dd 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -36,6 +36,14 @@ static bool probe_content, content_flag, no_content_flag;
static bool json_output = false;
static bool size_only = false;
+static struct export_list {
+ size_t len;
+ char **names;
+ char **descs;
+} export_list;
+
+static int collect_export (void *opaque, const char *name,
+ const char *desc);
static void list_one_export (struct nbd_handle *nbd, const char *desc,
bool first, bool last);
static void list_all_exports (struct nbd_handle *nbd1, const char *uri);
@@ -196,7 +204,8 @@ main (int argc, char *argv[])
}
if (list_all) {
- if (nbd_opt_list (nbd) == -1) {
+ if (nbd_opt_list (nbd, (nbd_list_callback) {
+ .callback = collect_export, .user_data = &export_list}) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
@@ -253,10 +262,46 @@ main (int argc, char *argv[])
printf ("}\n");
}
+ for (i = 0; i < export_list.len; i++) {
+ free (export_list.names[i]);
+ free (export_list.descs[i]);
+ }
+ free (export_list.names);
+ free (export_list.descs);
nbd_close (nbd);
exit (EXIT_SUCCESS);
}
+static int
+collect_export (void *opaque, const char *name, const char *desc)
+{
+ struct export_list *l = opaque;
+ char **names, **descs;
+
+ names = realloc (l->names, (l->len + 1) * sizeof name);
+ descs = realloc (l->descs, (l->len + 1) * sizeof desc);
+ if (!names || !descs) {
+ perror ("realloc");
+ exit (EXIT_FAILURE);
+ }
+ l->names = names;
+ l->descs = descs;
+ l->names[l->len] = strdup (name);
+ if (!l->names[l->len]) {
+ perror ("strdup");
+ exit (EXIT_FAILURE);
+ }
+ if (*desc) {
+ l->descs[l->len] = strdup (desc);
+ if (!l->descs[l->len]) {
+ perror ("strdup");
+ exit (EXIT_FAILURE);
+ }
+ }
+ l->len++;
+ return 0;
+}
+
static void
list_one_export (struct nbd_handle *nbd, const char *desc,
bool first, bool last)
@@ -415,29 +460,17 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
static void
list_all_exports (struct nbd_handle *nbd1, const char *uri)
{
- int i;
- int count = nbd_get_nr_list_exports (nbd1);
+ size_t i;
- if (count == -1) {
- fprintf (stderr, "unable to obtain list of exports: %s\n",
- nbd_get_error ());
- exit (EXIT_FAILURE);
- }
- if (count == 0 && json_output)
+ if (export_list.len == 0 && json_output)
printf ("\t\"exports\": []\n");
- for (i = 0; i < count; ++i) {
- char *name, *desc;
+ for (i = 0; i < export_list.len; ++i) {
+ const char *name;
struct nbd_handle *nbd2;
- name = nbd_get_list_export_name (nbd1, i);
- if (!name) {
- fprintf (stderr, "unable to obtain export name: %s\n",
- nbd_get_error ());
- exit (EXIT_FAILURE);
- }
-
/* 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 ());
@@ -454,12 +487,10 @@ list_all_exports (struct nbd_handle *nbd1, const char
*uri)
}
/* List the metadata of this export. */
- desc = nbd_get_list_export_description (nbd1, i);
- list_one_export (nbd2, desc, i == 0, i + 1 == count);
+ list_one_export (nbd2, export_list.descs[i], i == 0,
+ i + 1 == export_list.len);
nbd_close (nbd2);
- free (desc);
- free (name);
}
}
--
2.28.0
Richard W.M. Jones
2020-Aug-18 15:01 UTC
Re: [Libguestfs] [libnbd PATCH v3 2/2] api: Add nbd_aio_opt_list
It all looks good to me. I don't know if you were thinking about further changes, but as it stands, ACK. 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 11/13] api: Add nbd_aio_opt_list
- [PATCH libnbd PROPOSAL] Add APIs for listing exports from an NBD server.
- Re: [PATCH libnbd PROPOSAL] Add APIs for listing exports from an NBD server.
- [libnbd PATCH v3 0/2] Implementing NBD_OPT_LIST
- [libnbd PATCH 2/2] generator: Free closures on failure