[Libguestfs] [libnbd PATCH v3 2/2] api: Add nbd_aio_opt_list
Eric Blake
eblake at redhat.com
Tue Aug 18 02:09:22 UTC 2020
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
More information about the Libguestfs
mailing list