Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 00/13] Adding nbd_set_opt_mode to improve nbdinfo
Well, I'm not quite done (I still want to get nbdinfo to work on a single nbd connection for all cases when reading the heads of the file is not required), but I'm happy with patches 1-11, and 12-13 show where I'm headed for getting NBD_OPT_INFO to work. Posting now to see if some of the earlier patches are ready to commit while I continue working on the latter half. Eric Blake (13): api: Add nbd_set_full_info and friends info: Improve info without --list api: Add nbd_is_state_negotiating for new state api: Permit several existing API in Negotiating state api: Add nbd_set_opt_mode api: Add nbd_opt_abort and nbd_aio_opt_abort api: Add nbd_opt_go and nbd_aio_opt_go examples: Update list-exports to demonstrate nbd_opt_go info: Simplify by using nbd_opt_go api: Add nbd_opt_list api: Add nbd_aio_opt_list wip: api: Give aio_opt_go a completion callback wip: api: add nbd_opt_info, nbd_aio_opt_info docs/libnbd.pod | 42 +- lib/internal.h | 45 +- lib/nbd-protocol.h | 10 +- generator/API.ml | 478 +++++++++++++----- generator/API.mli | 1 + generator/C.ml | 4 +- generator/state_machine.ml | 37 +- generator/states-magic.c | 6 +- generator/states-newstyle-opt-go.c | 88 +++- generator/states-newstyle-opt-list.c | 83 ++- .../states-newstyle-opt-set-meta-context.c | 4 +- generator/states-newstyle-opt-starttls.c | 13 +- .../states-newstyle-opt-structured-reply.c | 8 +- generator/states-newstyle.c | 80 ++- generator/states.c | 18 +- lib/connect.c | 5 +- lib/flags.c | 28 +- lib/handle.c | 76 +-- lib/is-state.c | 14 +- lib/opt.c | 221 ++++++++ lib/Makefile.am | 3 +- tests/Makefile.am | 14 + tests/errors.c | 11 +- tests/newstyle-limited.c | 58 ++- tests/oldstyle.c | 29 +- tests/opt-abort.c | 95 ++++ tests/opt-info.c | 107 ++++ examples/list-exports.c | 139 +++-- examples/server-flags.c | 23 + interop/list-exports.c | 77 +-- .gitignore | 2 + info/Makefile.am | 7 +- info/info-description.sh | 56 ++ info/nbdinfo.c | 167 +++--- 34 files changed, 1613 insertions(+), 436 deletions(-) create mode 100644 lib/opt.c create mode 100644 tests/opt-abort.c create mode 100644 tests/opt-info.c create mode 100755 info/info-description.sh -- 2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 01/13] api: Add nbd_set_full_info and friends
Add APIs to make it possible to request and display NBD_INFO_NAME and
NBD_INFO_DESCRIPTION, which will enable fixing one of the todo items
in nbdinfo when not used in --list mode. Note that at least qemu-nbd
does not advertise name and description during NBD_OPT_GO unless the
client requests it first; at the same time, always requesting it only
slows down handshaking, so exposing a knob seems better than always
doing this check (contrast that with NBD_INFO_BLOCK_SIZE, which
affects correctness of data access).
---
lib/internal.h | 13 ++++-
lib/nbd-protocol.h | 10 +++-
generator/API.ml | 81 +++++++++++++++++++++++++++++-
generator/states-newstyle-opt-go.c | 49 +++++++++++++++---
lib/handle.c | 61 ++++++++++++++++++++++
5 files changed, 204 insertions(+), 10 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 6106af3..aa7e9a2 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -104,6 +104,9 @@ struct nbd_handle {
size_t nr_exports;
struct export *exports;
+ /* Full info mode. */
+ bool full_info;
+
/* Global flags from the server. */
uint16_t gflags;
@@ -120,6 +123,10 @@ struct nbd_handle {
uint32_t block_preferred;
uint32_t block_maximum;
+ /* Server canonical name and description, or NULL if not advertised. */
+ char *canonical_name;
+ char *description;
+
/* Flags set by the state machine to tell what protocol and whether
* TLS was negotiated.
*/
@@ -180,6 +187,10 @@ struct nbd_handle {
} __attribute__((packed)) server;
struct nbd_fixed_new_option_reply_info_export export;
struct nbd_fixed_new_option_reply_info_block_size block_size;
+ struct {
+ struct nbd_fixed_new_option_reply_info_name_or_desc info;
+ char str[NBD_MAX_STRING];
+ } __attribute__((packed)) name_desc;
struct {
struct nbd_fixed_new_option_reply_meta_context context;
char str[NBD_MAX_STRING];
@@ -205,7 +216,7 @@ struct nbd_handle {
uint32_t cflags;
uint32_t len;
uint16_t nrinfos;
- uint16_t info;
+ uint16_t info[3];
uint32_t nrqueries;
} sbuf;
diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h
index 627bc6e..e5d6404 100644
--- a/lib/nbd-protocol.h
+++ b/lib/nbd-protocol.h
@@ -154,6 +154,14 @@ struct nbd_fixed_new_option_reply_info_export {
uint16_t eflags; /* per-export flags */
} NBD_ATTRIBUTE_PACKED;
+/* NBD_INFO_NAME or NBD_INFO_DESCRIPTION reply (follows
+ * fixed_new_option_reply).
+ */
+struct nbd_fixed_new_option_reply_info_name_or_desc {
+ uint16_t info; /* NBD_INFO_NAME, NBD_INFO_DESCRIPTION */
+ /* followed by a string name or description */
+} NBD_ATTRIBUTE_PACKED;
+
/* NBD_INFO_BLOCK_SIZE reply (follows fixed_new_option_reply). */
struct nbd_fixed_new_option_reply_info_block_size {
uint16_t info; /* NBD_INFO_BLOCK_SIZE */
@@ -165,7 +173,7 @@ struct nbd_fixed_new_option_reply_info_block_size {
/* NBD_REP_SERVER reply (follows fixed_new_option_reply). */
struct nbd_fixed_new_option_reply_server {
uint32_t export_name_len; /* length of export name */
- /* followed by a string export name and description*/
+ /* followed by a string export name and description */
} NBD_ATTRIBUTE_PACKED;
/* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */
diff --git a/generator/API.ml b/generator/API.ml
index 2f3b57d..eff0ac4 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -294,8 +294,81 @@ to a URI that includes an export name.";
args = []; ret = RString;
shortdesc = "get the export name";
longdesc = "\
-Get the export name associated with the handle.";
- see_also = [Link "set_export_name"; Link "connect_uri"];
+Get the export name associated with the handle. This is the name
+that libnbd requests; see L<nbd_get_canonical_export_name(3)> for
+determining if the server has a different canonical name for the
+given export (most common when requesting the default export name
+of an empty string C<\"\">)";
+ see_also = [Link "set_export_name"; Link "connect_uri";
+ Link "get_canonical_export_name"];
+ };
+
+ "set_full_info", {
+ default_call with
+ args = [Bool "request"]; ret = RErr;
+ permitted_states = [ Created ];
+ shortdesc = "control whether NBD_OPT_GO requests extra details";
+ longdesc = "\
+By default, when connecting to an export, libnbd only requests the
+details it needs to service data operations. The NBD server says
+that a server can supply optional information, such as a canonical
+name of the export (see L<nbd_get_canonical_export_name(3)>) or
+a description of the export (see L<nbd_get_export_description(3)>),
+but that a hint from the client makes it more likely for this
+extra information to be provided. This function controls whether
+libnbd will provide that hint.
+
+Note that even when full info is requested, the server is not
+obligated to reply with all information that libnbd requested.
+Similarly, libnbd will ignore any optional server information that
+libnbd has not yet been taught to recognize.";
+ see_also = [Link "get_full_info"; Link
"get_canonical_export_name";
+ Link "get_export_description"];
+ };
+
+ "get_full_info", {
+ default_call with
+ args = []; ret = RBool;
+ permitted_states = [];
+ shortdesc = "see if NBD_OPT_GO requests extra details";
+ longdesc = "\
+Return the state of the full info request flag on this handle.";
+ see_also = [Link "set_full_info"];
+ };
+
+ "get_canonical_export_name", {
+ default_call with
+ args = []; ret = RString;
+ permitted_states = [ Connected; Closed ];
+ shortdesc = "return the canonical export name, if the server has
one";
+ longdesc = "\
+The NBD protocol permits a server to report an optional canonical
+export name which may differ from the client's request (as set by
+L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>. This function
+accesses any name returned by the server; it may be the same as
+the client request, but is more likely to differ when the client
+requested a connection to the default export name (an empty string
+C<\"\">).
+
+Some servers are unlikely to report a canonical name unless the
+client specifically hinted about wanting it, via
L<nbd_set_full_info(3)>.";
+ see_also = [Link "set_full_info"; Link
"get_export_name"];
+ };
+
+ "get_export_description", {
+ default_call with
+ args = []; ret = RString;
+ permitted_states = [ Connected; Closed ];
+ shortdesc = "return the export description, if the server has
one";
+ longdesc = "\
+The NBD protocol permits a server to report an optional export
+description. This function reports any description reported by
+the server.
+
+Some servers are unlikely to report description unless the
+client specifically hinted about wanting it, via L<nbd_set_full_info(3)>.
+For L<qemu-nbd(8)>, a description is set with I<-D>.";
+ see_also = [Link "set_full_info"];
};
"set_tls", {
@@ -2364,6 +2437,10 @@ let first_version = [
"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);
+ "get_canonical_export_name", (1, 4);
+ "get_export_description", (1, 4);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index 99a1dcd..74f092e 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -20,11 +20,13 @@
STATE_MACHINE {
NEWSTYLE.OPT_GO.START:
+ uint16_t nrinfos = h->full_info ? 3 : 1;
+
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
h->sbuf.option.option = htobe32 (NBD_OPT_GO);
h->sbuf.option.optlen htobe32 (/* exportnamelen */ 4 + strlen
(h->export_name)
- + /* nrinfos */ 2 + /* INFO_BLOCK_SIZE */ 2);
+ + sizeof nrinfos + 2 * nrinfos);
h->wbuf = &h->sbuf;
h->wlen = sizeof h->sbuf.option;
h->wflags = MSG_MORE;
@@ -57,23 +59,29 @@ STATE_MACHINE {
return 0;
NEWSTYLE.OPT_GO.SEND_EXPORT:
+ uint16_t nrinfos = h->full_info ? 3 : 1;
+
switch (send_from_wbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
- h->sbuf.nrinfos = htobe16 (1);
+ h->sbuf.nrinfos = htobe16 (nrinfos);
h->wbuf = &h->sbuf;
- h->wlen = 2;
+ h->wlen = sizeof h->sbuf.nrinfos;
SET_NEXT_STATE (%SEND_NRINFOS);
}
return 0;
NEWSTYLE.OPT_GO.SEND_NRINFOS:
+ uint16_t nrinfos = h->full_info ? 3 : 1;
+
switch (send_from_wbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
- h->sbuf.info = htobe16 (NBD_INFO_BLOCK_SIZE);
+ h->sbuf.info[0] = htobe16 (NBD_INFO_BLOCK_SIZE);
+ h->sbuf.info[1] = htobe16 (NBD_INFO_NAME);
+ h->sbuf.info[2] = htobe16 (NBD_INFO_DESCRIPTION);
h->wbuf = &h->sbuf;
- h->wlen = 2;
+ h->wlen = sizeof h->sbuf.info[0] * nrinfos;
SET_NEXT_STATE (%SEND_INFO);
}
return 0;
@@ -154,8 +162,37 @@ STATE_MACHINE {
return 0;
}
break;
+ case NBD_INFO_NAME:
+ if (len > sizeof h->sbuf.or.payload.name_desc.info +
NBD_MAX_STRING ||
+ len < sizeof h->sbuf.or.payload.name_desc.info) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (0, "handshake: incorrect NBD_INFO_NAME option reply
length");
+ return 0;
+ }
+ free (h->canonical_name);
+ h->canonical_name = strndup (h->sbuf.or.payload.name_desc.str,
len - 2);
+ if (h->canonical_name == NULL) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (errno, "strndup");
+ return 0;
+ }
+ break;
+ case NBD_INFO_DESCRIPTION:
+ if (len > sizeof h->sbuf.or.payload.name_desc.info +
NBD_MAX_STRING ||
+ len < sizeof h->sbuf.or.payload.name_desc.info) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (0, "handshake: incorrect NBD_INFO_DESCRIPTION option
reply length");
+ return 0;
+ }
+ free (h->description);
+ h->description = strndup (h->sbuf.or.payload.name_desc.str, len -
2);
+ if (h->description == NULL) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (errno, "strndup");
+ return 0;
+ }
+ break;
default:
- /* XXX Handle other info types, like NBD_INFO_DESCRIPTION */
debug (h, "skipping unknown NBD_REP_INFO type %d",
be16toh (h->sbuf.or.payload.export.info));
break;
diff --git a/lib/handle.c b/lib/handle.c
index 210ac7d..28c30dd 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -136,6 +136,8 @@ nbd_close (struct nbd_handle *h)
free (h->exports[i].description);
}
free (h->exports);
+ free (h->canonical_name);
+ free (h->description);
free_cmd_list (h->cmds_to_issue);
free_cmd_list (h->cmds_in_flight);
free_cmd_list (h->cmds_done);
@@ -232,6 +234,65 @@ nbd_unlocked_get_export_name (struct nbd_handle *h)
return copy;
}
+int
+nbd_unlocked_set_full_info (struct nbd_handle *h, bool request)
+{
+ h->full_info = request;
+ return 0;
+}
+
+int
+nbd_unlocked_get_full_info (struct nbd_handle *h)
+{
+ return h->full_info;
+}
+
+char *
+nbd_unlocked_get_canonical_export_name (struct nbd_handle *h)
+{
+ char *r;
+
+ if (h->eflags == 0) {
+ set_error (EINVAL, "server has not returned export flags, "
+ "you need to connect to the server first");
+ return NULL;
+ }
+ if (h->canonical_name == NULL) {
+ set_error (ENOTSUP, "server did not advertise a canonical name");
+ return NULL;
+ }
+
+ r = strdup (h->canonical_name);
+ if (r == NULL) {
+ set_error (errno, "strdup");
+ return NULL;
+ }
+ return r;
+}
+
+char *
+nbd_unlocked_get_export_description (struct nbd_handle *h)
+{
+ char *r;
+
+ if (h->eflags == 0) {
+ set_error (EINVAL, "server has not returned export flags, "
+ "you need to connect to the server first");
+ return NULL;
+ }
+ if (h->description == NULL) {
+ set_error (ENOTSUP, "server did not advertise a description");
+ return NULL;
+ }
+
+ r = strdup (h->description);
+ if (r == NULL) {
+ set_error (errno, "strdup");
+ return NULL;
+ }
+ return r;
+}
+
int
nbd_unlocked_set_list_exports (struct nbd_handle *h, bool list)
{
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 02/13] info: Improve info without --list
Make use of the new nbd_get_export_description to list a description
when connecting to a single export.
At present, only qemu-nbd offers the needed information; it would
require a patch to nbdkit to do likewise (although such a patch is not
difficult, given that we recently added .list_exports).
---
generator/API.ml | 3 +++
examples/server-flags.c | 23 +++++++++++++++++
info/Makefile.am | 1 +
info/info-description.sh | 56 ++++++++++++++++++++++++++++++++++++++++
info/nbdinfo.c | 16 +++++++++---
5 files changed, 95 insertions(+), 4 deletions(-)
create mode 100755 info/info-description.sh
diff --git a/generator/API.ml b/generator/API.ml
index eff0ac4..0c8ef30 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -322,6 +322,7 @@ Note that even when full info is requested, the server is
not
obligated to reply with all information that libnbd requested.
Similarly, libnbd will ignore any optional server information that
libnbd has not yet been taught to recognize.";
+ example = Some "examples/server-flags.c";
see_also = [Link "get_full_info"; Link
"get_canonical_export_name";
Link "get_export_description"];
};
@@ -352,6 +353,7 @@ C<\"\">).
Some servers are unlikely to report a canonical name unless the
client specifically hinted about wanting it, via
L<nbd_set_full_info(3)>.";
+ example = Some "examples/server-flags.c";
see_also = [Link "set_full_info"; Link
"get_export_name"];
};
@@ -368,6 +370,7 @@ the server.
Some servers are unlikely to report description unless the
client specifically hinted about wanting it, via L<nbd_set_full_info(3)>.
For L<qemu-nbd(8)>, a description is set with I<-D>.";
+ example = Some "examples/server-flags.c";
see_also = [Link "set_full_info"];
};
diff --git a/examples/server-flags.c b/examples/server-flags.c
index d06f02f..d156ace 100644
--- a/examples/server-flags.c
+++ b/examples/server-flags.c
@@ -16,6 +16,7 @@ int
main (int argc, char *argv[])
{
struct nbd_handle *nbd;
+ char *str;
int flag;
if (argc != 2) {
@@ -30,6 +31,14 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
+ /* Request full information. */
+#if LIBNBD_HAVE_NBD_SET_FULL_INFO /* Added in 1.4 */
+ if (nbd_set_full_info (nbd, true) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+#endif
+
/* Connect to the NBD server over a
* Unix domain socket.
*/
@@ -38,6 +47,20 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
+ /* See if the server provided extra details,
+ * using functions added in 1.4
+ */
+#if LIBNBD_HAVE_NBD_GET_EXPORT_DESCRIPTION
+ str = nbd_get_canonical_export_name (nbd);
+ if (str)
+ printf ("canonical_name = %s\n", str);
+ free (str);
+ str = nbd_get_export_description (nbd);
+ if (str)
+ printf ("description = %s\n", str);
+ free (str);
+#endif
+
/* Read and print the flags. */
#define PRINT_FLAG(flag_fn) \
flag = flag_fn (nbd); \
diff --git a/info/Makefile.am b/info/Makefile.am
index 22c0c28..05b8137 100644
--- a/info/Makefile.am
+++ b/info/Makefile.am
@@ -66,6 +66,7 @@ TESTS += \
info-oldstyle.sh \
info-size.sh \
info-text.sh \
+ info-description.sh \
$(NULL)
check-valgrind:
diff --git a/info/info-description.sh b/info/info-description.sh
new file mode 100755
index 0000000..d367a25
--- /dev/null
+++ b/info/info-description.sh
@@ -0,0 +1,56 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright (C) 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
+
+. ../tests/functions.sh
+
+set -e
+set -x
+
+# XXX Use nbdkit once it supports NBD_INFO_DESCRIPTION
+requires qemu-nbd --version
+requires truncate --version
+
+img=info-description.img
+out=info-description.out
+pid=info-description.pid
+sock=`mktemp -u`
+cleanup_fn rm -f $img $out $pid $sock
+rm -f $img $out $pid $sock
+
+truncate -s 1M $img
+qemu-nbd -f raw -t --socket=$sock --pid-file=$pid -x "hello" -D
"world" $img &
+cleanup_fn kill $!
+
+# Wait for qemu-nbd to start up.
+for i in {1..60}; do
+ if test -f $pid; then
+ break
+ fi
+ sleep 1
+done
+if ! test -f $pid; then
+ echo "$0: qemu-nbd did not start up"
+ exit 1
+fi
+
+$VG nbdinfo "nbd+unix:///hello?socket=$sock" > $out
+cat $out
+
+grep 'export="hello":' $out
+grep 'description: world' $out
+grep 'export-size: 1048576' $out
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 454e5c5..b54dfd4 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -99,6 +99,7 @@ main (int argc, char *argv[])
int64_t size;
const char *protocol;
int tls_negotiated;
+ char *desc;
for (;;) {
c = getopt_long (argc, argv, short_options, long_options, NULL);
@@ -191,6 +192,8 @@ main (int argc, char *argv[])
*/
if (list_all)
nbd_set_list_exports (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) {
@@ -244,9 +247,11 @@ main (int argc, char *argv[])
printf ("\"TLS\": %s,\n", tls_negotiated ?
"true" : "false");
}
- if (!list_all)
- /* XXX We would need libnbd to request NBD_INFO_DESCRIPTION */
- list_one_export (nbd, NULL, true, true);
+ if (!list_all) {
+ desc = nbd_get_export_description (nbd);
+ list_one_export (nbd, desc, true, true);
+ free (desc);
+ }
else
list_all_exports (nbd, argv[optind]);
@@ -277,7 +282,10 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
exit (EXIT_FAILURE);
}
- export_name = nbd_get_export_name (nbd);
+ /* Prefer the server's version of the name, if available */
+ export_name = nbd_get_canonical_export_name (nbd);
+ if (export_name == NULL)
+ export_name = nbd_get_export_name (nbd);
if (export_name == NULL) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 03/13] api: Add nbd_is_state_negotiating for new state
This patch prepares for a change to export list extraction by adding a
new state in the state machine, NEGOTIATING. At present, it is not
actually possible to enter this state. But upcoming patches will
allow the user to opt-in to fine-grained control over negotiation
during the connecting phase. Since the handshake phase is synchronous
(the client sends a full command, then waits for the full server
response, with no interleaving or out-of-order traffic), the idea is
that we can drop out of connecting over to Negotiating any time we are
waiting for the user to determine the next client command to send, and
then back to connecting until that option is handled; where unlike the
data phase, we won't need a cookie. Other upcoming patches will then
adjust existing API that make sense to call while in this state, as
well as adding new API to give the user control over when to actually
use this state.
---
lib/internal.h | 1 +
generator/API.ml | 17 +++++++++++++++++
generator/API.mli | 1 +
generator/C.ml | 4 +++-
generator/state_machine.ml | 7 +++++++
lib/connect.c | 5 +++--
lib/is-state.c | 14 +++++++++++++-
7 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index aa7e9a2..186d677 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -410,6 +410,7 @@ extern int nbd_internal_set_block_size (struct nbd_handle
*h, uint32_t min,
/* is-state.c */
extern bool nbd_internal_is_state_created (enum state state);
extern bool nbd_internal_is_state_connecting (enum state state);
+extern bool nbd_internal_is_state_negotiating (enum state state);
extern bool nbd_internal_is_state_ready (enum state state);
extern bool nbd_internal_is_state_processing (enum state state);
extern bool nbd_internal_is_state_dead (enum state state);
diff --git a/generator/API.ml b/generator/API.ml
index 0c8ef30..0990d48 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -90,6 +90,7 @@ and flags = {
and permitted_state | Created
| Connecting
+| Negotiating
| Connected
| Closed | Dead
and link @@ -2136,6 +2137,21 @@ issue commands (see
L<nbd_aio_is_ready(3)>).";
see_also = [Link "aio_is_ready"];
};
+ "aio_is_negotiating", {
+ default_call with
+ args = []; ret = RBool; is_locked = false; may_set_error = false;
+ shortdesc = "check if connection is ready to send handshake
option";
+ longdesc = "\
+Return true if this connection is ready to start another option
+negotiation command while handshaking with the server. An option
+command will move back to the connecting state (see
+L<nbd_aio_is_connecting(3)>). Note that this state cannot be
+reached unless requested by nbd_set_opt_mode, and even then
+it only works with newstyle servers; an oldstyle server will skip
+straight to L<nbd_aio_is_ready(3)>.";
+ see_also = [Link "aio_is_connecting"; Link
"aio_is_ready"];
+ };
+
"aio_is_ready", {
default_call with
args = []; ret = RBool; is_locked = false; may_set_error = false;
@@ -2444,6 +2460,7 @@ let first_version = [
"get_full_info", (1, 4);
"get_canonical_export_name", (1, 4);
"get_export_description", (1, 4);
+ "aio_is_negotiating", (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/API.mli b/generator/API.mli
index 3dbd1ff..712e837 100644
--- a/generator/API.mli
+++ b/generator/API.mli
@@ -102,6 +102,7 @@ and flags = {
and permitted_state | Created (** can be called in the START
state *)
| Connecting (** can be called when connecting/handshaking *)
+| Negotiating (** can be called when handshaking in opt_mode *)
| Connected (** when connected and READY or processing, but
not including CLOSED or DEAD *)
| Closed | Dead (** can be called when the handle is
diff --git a/generator/C.ml b/generator/C.ml
index afb8142..6b65f6e 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -397,7 +397,8 @@ let permitted_state_text permitted_states function
| Created -> "newly created"
| Connecting -> "connecting"
- | Connected -> "connected and finished handshaking with the
server"
+ | Negotiating -> "negotiating"
+ | Connected -> "connected with the server"
| Closed -> "shut down"
| Dead -> "dead"
) permitted_states
@@ -421,6 +422,7 @@ let generate_lib_api_c () function
| Created -> "nbd_internal_is_state_created (state)"
| Connecting -> "nbd_internal_is_state_connecting
(state)"
+ | Negotiating -> "nbd_internal_is_state_negotiating
(state)"
| Connected -> "nbd_internal_is_state_ready (state) ||
nbd_internal_is_state_processing (state)"
| Closed -> "nbd_internal_is_state_closed (state)"
| Dead -> "nbd_internal_is_state_dead (state)"
diff --git a/generator/state_machine.ml b/generator/state_machine.ml
index 144a5f8..8525520 100644
--- a/generator/state_machine.ml
+++ b/generator/state_machine.ml
@@ -100,6 +100,13 @@ let rec state_machine = [
Group ("OLDSTYLE", oldstyle_state_machine);
Group ("NEWSTYLE", newstyle_state_machine);
+ State {
+ default_state with
+ name = "NEGOTIATING";
+ comment = "Connection is ready to negotiate an NBD option";
+ external_events = [ CmdIssue, "NEWSTYLE.START" ];
+ };
+
State {
default_state with
name = "READY";
diff --git a/lib/connect.c b/lib/connect.c
index 92ed6b1..7e42b14 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * 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
@@ -40,7 +40,8 @@
static int
error_unless_ready (struct nbd_handle *h)
{
- if (nbd_internal_is_state_ready (get_next_state (h)))
+ if (nbd_internal_is_state_ready (get_next_state (h)) ||
+ nbd_internal_is_state_negotiating (get_next_state (h)))
return 0;
/* Why did it fail? */
diff --git a/lib/is-state.c b/lib/is-state.c
index 1a85c7a..e019e53 100644
--- a/lib/is-state.c
+++ b/lib/is-state.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * 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
@@ -58,6 +58,12 @@ nbd_internal_is_state_connecting (enum state state)
return is_connecting_group (group);
}
+bool
+nbd_internal_is_state_negotiating (enum state state)
+{
+ return state == STATE_NEGOTIATING;
+}
+
bool
nbd_internal_is_state_ready (enum state state)
{
@@ -120,6 +126,12 @@ nbd_unlocked_aio_is_connecting (struct nbd_handle *h)
return nbd_internal_is_state_connecting (get_public_state (h));
}
+int
+nbd_unlocked_aio_is_negotiating (struct nbd_handle *h)
+{
+ return nbd_internal_is_state_negotiating (get_public_state (h));
+}
+
int
nbd_unlocked_aio_is_ready (struct nbd_handle *h)
{
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 04/13] api: Permit several existing API in Negotiating state
In particular, any query that can be determined after getting gflags
from the server or after a response to NBD_OPT* can be now done
earlier than Connected, and any configuration that affects behavior
of NBD_OPT_* can be set later than rather just during Created.
In order to support this change: nbd_get_protocol also needs to hoist
the setting of h->protocol earlier in the state machine. Also,
whereas nbd_connect_uri used to unconditionally take precedence over
nbd_set_export_name, now nbd_set_export_name can be used during
negotiation to override the export name portion of a URI.
Of course, until a later patch actually lets us enter the NEGOTIATED
state, you can't test the differences yet.
---
generator/API.ml | 33 +++++++++++++++++++--------------
generator/states-newstyle.c | 12 ++++++------
2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/generator/API.ml b/generator/API.ml
index 0990d48..a363117 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -272,7 +272,7 @@ C<\"nbd2\">, etc.";
"set_export_name", {
default_call with
args = [ String "export_name" ]; ret = RErr;
- permitted_states = [ Created ];
+ permitted_states = [ Created; Negotiating ];
shortdesc = "set the export name";
longdesc = "\
For servers which require an export name or can serve different
@@ -287,7 +287,7 @@ The encoding of export names is always UTF-8.
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"];
+ see_also = [Link "get_export_name"; Link
"connect_uri"];
};
"get_export_name", {
@@ -307,7 +307,7 @@ of an empty string C<\"\">)";
"set_full_info", {
default_call with
args = [Bool "request"]; ret = RErr;
- permitted_states = [ Created ];
+ permitted_states = [ Created; Negotiating ];
shortdesc = "control whether NBD_OPT_GO requests extra details";
longdesc = "\
By default, when connecting to an export, libnbd only requests the
@@ -441,7 +441,7 @@ on a particular connection use
L<nbd_get_tls_negotiated(3)> instead.";
"get_tls_negotiated", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "find out if TLS was negotiated on a connection";
longdesc = "\
After connecting you may call this to find out if the
@@ -610,7 +610,7 @@ L<nbd_get_structured_replies_negotiated(3)>
instead.";
"get_structured_replies_negotiated", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "see if structured replies are in use";
longdesc = "\
After connecting you may call this to find out if the connection is
@@ -735,7 +735,7 @@ Return true if list exports mode was enabled on this
handle.";
"get_nr_list_exports", {
default_call with
args = []; ret = RInt;
- permitted_states = [ Connected; Closed; Dead ];
+ 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
@@ -749,7 +749,7 @@ L<nbd_set_list_exports(3)>.";
"get_list_export_name", {
default_call with
args = [ Int "i" ]; ret = RString;
- permitted_states = [ Connected; Closed; Dead ];
+ 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
@@ -761,7 +761,7 @@ from the list returned by the server.";
"get_list_export_description", {
default_call with
args = [ Int "i" ]; ret = RString;
- permitted_states = [ Connected; Closed; Dead ];
+ 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
@@ -776,7 +776,7 @@ is set with I<-D>.";
"add_meta_context", {
default_call with
args = [ String "name" ]; ret = RErr;
- permitted_states = [ Created ];
+ permitted_states = [ Created; Negotiating ];
shortdesc = "ask server to negotiate metadata context";
longdesc = "\
During connection libnbd can negotiate zero or more metadata
@@ -885,10 +885,14 @@ parameter in NBD URIs is allowed.";
shortdesc = "connect to NBD URI";
longdesc = "\
Connect (synchronously) to an NBD server and export by specifying
-the NBD URI. This call parses the URI and may call
+the NBD URI. This call parses the URI and calls
L<nbd_set_export_name(3)> and L<nbd_set_tls(3)> and other
-calls as needed, followed by
-L<nbd_connect_tcp(3)> or L<nbd_connect_unix(3)>.
+calls as needed, followed by L<nbd_connect_tcp(3)> or
+L<nbd_connect_unix(3)>. However, it is possible to override the
+export name portion of a URI by using C<nbd_set_opt_mode> to
+enable option mode, then using L<nbd_set_export_name(3)> as part
+of later negotiation.
+
This call returns when the connection has been made.
=head2 Example URIs supported
@@ -1027,7 +1031,8 @@ test whether this is the case with
L<nbd_supports_uri(3)>.
Support for URIs that require TLS will fail if libnbd was not
compiled with gnutls; you can test whether this is the case
with L<nbd_supports_tls(3)>.";
- see_also = [URLLink
"https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md"];
+ see_also = [URLLink
"https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md";
+ Link "set_export_name"; Link "set_tls"];
};
"connect_unix", {
@@ -1326,7 +1331,7 @@ are free to pass in other contexts."
"get_protocol", {
default_call with
args = []; ret = RStaticString;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "return the NBD protocol variant";
longdesc = "\
Return the NBD protocol variant in use on the connection. At
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 573f724..8da4617 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.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
@@ -136,6 +136,11 @@ STATE_MACHINE {
return 0;
}
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
+ h->protocol = "newstyle";
+ else
+ h->protocol = "newstyle-fixed";
+
cflags = h->gflags;
h->sbuf.cflags = htobe32 (cflags);
h->wbuf = &h->sbuf;
@@ -156,11 +161,6 @@ STATE_MACHINE {
return 0;
NEWSTYLE.FINISHED:
- if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
- h->protocol = "newstyle";
- else
- h->protocol = "newstyle-fixed";
-
SET_NEXT_STATE (%.READY);
return 0;
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 05/13] api: Add nbd_set_opt_mode
Now that we have a new NEGOTIATING state, we need a knob for the user
to request moving into that state. This patch does not actually alter
the state machine, but one thing at a time. However, the libnbd docs
now mention several commands that will be added in future commits.
(Hmm - our generator refuses to compile in-tree links in API.ml that
don't resolve, but not links in libnbd.pod.)
One thing we can test at this point: an oldstyle server will never
get into opt mode; that will only be possible for a newstyle server.
---
docs/libnbd.pod | 38 ++++++++++++++-
lib/internal.h | 3 ++
generator/API.ml | 46 +++++++++++++++++--
generator/states-newstyle-opt-go.c | 1 +
generator/states-newstyle-opt-list.c | 1 +
.../states-newstyle-opt-set-meta-context.c | 3 +-
generator/states-newstyle-opt-starttls.c | 1 +
.../states-newstyle-opt-structured-reply.c | 1 +
lib/opt.c | 41 +++++++++++++++++
lib/Makefile.am | 3 +-
tests/oldstyle.c | 29 ++++++++----
11 files changed, 151 insertions(+), 16 deletions(-)
create mode 100644 lib/opt.c
diff --git a/docs/libnbd.pod b/docs/libnbd.pod
index fc66888..4ee0815 100644
--- a/docs/libnbd.pod
+++ b/docs/libnbd.pod
@@ -447,6 +447,40 @@ subprocess. This works very similarly to
L<nbd_connect_command(3)>
described above, but you must use
L<nbd_connect_systemd_socket_activation(3)> instead.
+=head1 CONTROLLING NEGOTIATION
+
+By default, when beginning a connection, libnbd will handle all
+negotiation with the server, using only the configuration
+(eg. L<nbd_set_export_name(3)> or L<nbd_add_meta_context(3)>) that
was
+requested before the connection attempt; this phase continues until
+L<nbd_aio_is_connecting(3)> no longer returns true, at which point,
+either data commands are ready to use or else the connection has
+failed with an error.
+
+But there are scenarios in which it is useful to also control the
+handshaking commands sent during negotiation, such as asking the
+server for a list of available exports prior to selecting which one to
+use. This is done by calling L<nbd_set_opt_mode(3)> before
+connecting; then after requesting a connection, the state machine will
+pause at L<nbd_aio_is_negotiating(3)> at any point that the user can
+decide which handshake command to send next. Note that the
+negotiation state is only reachable from newstyle servers; older
+servers cannot negotiate and will progress all the way to the ready
+state.
+
+When the negotiating state is reached, you can initiate option
+commands such as L<nbd_opt_list(3)> or their asynchronous equivalents,
+as well as alter configuration such as export name that previously had
+to be set before connection. Since the NBD protocol does not allow
+parallel negotiating commands, no cookie is involved, and you can
+track completion of each command when the state is no longer
+L<nbd_aio_is_connecting(3)>. If L<nbd_opt_go(3)> fails but the
+connection is still live, you will be back in negotiation state, where
+you can request a different export name and try again. Exiting the
+negotiation state is only possible with a successful L<nbd_opt_go(3)>
+which moves to the data phase, or L<nbd_opt_abort(3)> which performs a
+clean shutdown of the connection by skipping the data phase.
+
=head1 EXPORTS AND FLAGS
It is possible for NBD servers to serve different content on different
@@ -562,8 +596,8 @@
L<https://github.com/libguestfs/libnbd/blob/master/interop/dirty-bitmap.c>
=head2 Issuing multiple in-flight requests
NBD servers which properly implement the specification can handle
-multiple requests in flight over the same connection at the same time.
-Libnbd supports this when using the low level API.
+multiple data requests in flight over the same connection at the same
+time. Libnbd supports this when using the low level API.
To use it you simply issue more requests as needed (eg. using calls
like L<nbd_aio_pread(3)>, L<nbd_aio_pwrite(3)>) without waiting for
previous
diff --git a/lib/internal.h b/lib/internal.h
index 186d677..5f495fb 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -99,6 +99,9 @@ struct nbd_handle {
int uri_allow_tls;
bool uri_allow_local_file;
+ /* Option negotiation mode. */
+ bool opt_mode;
+
/* List exports mode. */
bool list_exports;
size_t nr_exports;
diff --git a/generator/API.ml b/generator/API.ml
index a363117..3dd94f6 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -681,6 +681,40 @@ the time of compilation.";
Link "aio_is_created"; Link
"aio_is_ready"];
};
+ "set_opt_mode", {
+ default_call with
+ args = [Bool "enable"]; ret = RErr;
+ permitted_states = [ Created ];
+ shortdesc = "control option mode, for pausing during option
negotiation";
+ longdesc = "\
+Set this flag to true in order to request that a connection command
+C<nbd_connect_*> will pause for negotiation options rather than
+proceeding all the way to the ready state, when communicating with a
+newstyle server. This setting has no effect when connecting to an
+oldstyle server.
+
+When option mode is enabled, you have fine-grained control over which
+options are negotiated, compared to the default of the server
+negotiating everything on your behalf using settings made before
+starting the connection. To leave the mode and proceed on to the
+ready state, you must use nbd_opt_go successfully; a failed
+C<nbd_opt_go> returns to the negotiating state to allow a change of
+export name before trying again. You may also use nbd_opt_abort
+to end the connection without finishing negotiation.";
+ example = Some "examples/list-exports.c";
+ see_also = [Link "get_opt_mode"; Link
"aio_is_negotiating"];
+ };
+
+ "get_opt_mode", {
+ default_call with
+ args = []; ret = RBool;
+ may_set_error = false;
+ shortdesc = "return whether option mode was enabled";
+ longdesc = "\
+Return true if option negotiation mode was enabled on this handle.";
+ see_also = [Link "set_opt_mode"];
+ };
+
"set_list_exports", {
default_call with
args = [Bool "list"]; ret = RErr;
@@ -889,7 +923,7 @@ the NBD URI. This call parses the URI and calls
L<nbd_set_export_name(3)> and L<nbd_set_tls(3)> and other
calls as needed, followed by L<nbd_connect_tcp(3)> or
L<nbd_connect_unix(3)>. However, it is possible to override the
-export name portion of a URI by using C<nbd_set_opt_mode> to
+export name portion of a URI by using L<nbd_set_opt_mode(3)> to
enable option mode, then using L<nbd_set_export_name(3)> as part
of later negotiation.
@@ -1032,7 +1066,8 @@ Support for URIs that require TLS will fail if libnbd was
not
compiled with gnutls; you can test whether this is the case
with L<nbd_supports_tls(3)>.";
see_also = [URLLink
"https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md";
- Link "set_export_name"; Link "set_tls"];
+ Link "set_export_name"; Link "set_tls";
+ Link "set_opt_mode"];
};
"connect_unix", {
@@ -2151,10 +2186,11 @@ Return true if this connection is ready to start another
option
negotiation command while handshaking with the server. An option
command will move back to the connecting state (see
L<nbd_aio_is_connecting(3)>). Note that this state cannot be
-reached unless requested by nbd_set_opt_mode, and even then
+reached unless requested by L<nbd_set_opt_mode(3)>, and even then
it only works with newstyle servers; an oldstyle server will skip
straight to L<nbd_aio_is_ready(3)>.";
- see_also = [Link "aio_is_connecting"; Link
"aio_is_ready"];
+ see_also = [Link "aio_is_connecting"; Link
"aio_is_ready";
+ Link "set_opt_mode"];
};
"aio_is_ready", {
@@ -2465,6 +2501,8 @@ let first_version = [
"get_full_info", (1, 4);
"get_canonical_export_name", (1, 4);
"get_export_description", (1, 4);
+ "set_opt_mode", (1, 4);
+ "get_opt_mode", (1, 4);
"aio_is_negotiating", (1, 4);
(* These calls are proposed for a future version of libnbd, but
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index 74f092e..d696cae 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -22,6 +22,7 @@ STATE_MACHINE {
NEWSTYLE.OPT_GO.START:
uint16_t nrinfos = h->full_info ? 3 : 1;
+ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
h->sbuf.option.option = htobe32 (NBD_OPT_GO);
h->sbuf.option.optlen diff --git a/generator/states-newstyle-opt-list.c
b/generator/states-newstyle-opt-list.c
index 80fcb1b..f2846b6 100644
--- a/generator/states-newstyle-opt-list.c
+++ b/generator/states-newstyle-opt-list.c
@@ -23,6 +23,7 @@
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;
diff --git a/generator/states-newstyle-opt-set-meta-context.c
b/generator/states-newstyle-opt-set-meta-context.c
index 92fe26c..77fd022 100644
--- a/generator/states-newstyle-opt-set-meta-context.c
+++ b/generator/states-newstyle-opt-set-meta-context.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
@@ -27,6 +27,7 @@ STATE_MACHINE {
* Also we skip the group if the client didn't request any metadata
* contexts.
*/
+ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
if (!h->structured_replies ||
h->request_meta_contexts == NULL ||
nbd_internal_string_list_length (h->request_meta_contexts) == 0) {
diff --git a/generator/states-newstyle-opt-starttls.c
b/generator/states-newstyle-opt-starttls.c
index 2d74e5f..7162c7a 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -20,6 +20,7 @@
STATE_MACHINE {
NEWSTYLE.OPT_STARTTLS.START:
+ 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);
diff --git a/generator/states-newstyle-opt-structured-reply.c
b/generator/states-newstyle-opt-structured-reply.c
index cfe6e0d..58a76db 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -20,6 +20,7 @@
STATE_MACHINE {
NEWSTYLE.OPT_STRUCTURED_REPLY.START:
+ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
if (!h->request_sr) {
SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
return 0;
diff --git a/lib/opt.c b/lib/opt.c
new file mode 100644
index 0000000..306a2e9
--- /dev/null
+++ b/lib/opt.c
@@ -0,0 +1,41 @@
+/* NBD client library in userspace
+ * Copyright (C) 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
+ */
+
+#include <config.h>
+
+#include <stdlib.h>
+#include <stdbool.h>
+#include <errno.h>
+#include <assert.h>
+#include <limits.h>
+
+#include "internal.h"
+
+int
+nbd_unlocked_set_opt_mode (struct nbd_handle *h, bool value)
+{
+ h->opt_mode = value;
+ return 0;
+}
+
+/* NB: may_set_error = false. */
+int
+nbd_unlocked_get_opt_mode (struct nbd_handle *h)
+{
+ return h->opt_mode;
+}
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 1c46c54..9fd6331 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -1,5 +1,5 @@
# nbd client library in userspace
-# 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
@@ -47,6 +47,7 @@ libnbd_la_SOURCES = \
internal.h \
is-state.c \
nbd-protocol.h \
+ opt.c \
poll.c \
protocol.c \
rw.c \
diff --git a/tests/oldstyle.c b/tests/oldstyle.c
index dc58d94..fd8bdc5 100644
--- a/tests/oldstyle.c
+++ b/tests/oldstyle.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * 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
@@ -106,18 +106,26 @@ main (int argc, char *argv[])
}
nbd_close (nbd);
- /* Now for a working connection */
+ /* Now for a working connection. Requesting an export name and opt_mode
+ * have no effects.
+ */
nbd = nbd_create ();
- if (nbd == NULL) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- exit (EXIT_FAILURE);
- }
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd == NULL ||
+ nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1 ||
+ nbd_set_export_name (nbd, "ignored") == -1 ||
+ nbd_connect_command (nbd, args) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
- /* Protocol should be "oldstyle", with no structured replies. */
+ /* Protocol should be "oldstyle", with no structured replies or
meta
+ * contexts.
+ */
+ if (nbd_aio_is_ready (nbd) != true) {
+ fprintf (stderr, "unexpected state after connection\n");
+ exit (EXIT_FAILURE);
+ }
s = nbd_get_protocol (nbd);
if (strcmp (s, "oldstyle") != 0) {
fprintf (stderr,
@@ -129,6 +137,11 @@ main (int argc, char *argv[])
"incorrect structured replies %" PRId64 ", expected
0\n", r);
exit (EXIT_FAILURE);
}
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr,
+ "incorrect meta context %" PRId64 ", expected
0\n", r);
+ exit (EXIT_FAILURE);
+ }
if ((r = nbd_get_size (nbd)) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 06/13] api: Add nbd_opt_abort and nbd_aio_opt_abort
It is finally time to introduce our first negotiating option command.
With this change, we can now enter NEWSTYLE.START more than once; as
such, it needs to know whether it is the first entry (proceed with
gflags/cflags, TLS, and structured reply) or a later entry (all
nbd_opt_* will cause an IssueCommand event to kick the state machine
out of NEGOTIATING, at which point we want to jump to the sub-state
appropriate for that option). This is done by setting h->current_opt
prior to entering NEWSTYLE.
We also need a couple of new states to implement NBD_OPT_ABORT; this
was not deemed worth a separate sub-group and new
states-newstyle-opt-abort.c file, so I just inlined it in NEWSTYLE
instead. One nice benefit of these new states: when NBD_OPT_GO fails
and we are not in opt mode, or when we require TLS but the server does
not support it, we can now hang up gracefully instead of abruptly
disconnecting from a newstyle-fixed server. This is observable when
using qemu-nbd, in that we now silence the former noise from qemu:
$ truncate --size=512 /tmp/f
$ ./run nbdsh -c '
import errno
try:
h.connect_systemd_socket_activation(["qemu-nbd", "-f",
"raw", "-x", "b",
"/tmp/f"])
except nbd.Error as e:
assert e.errnum == errno.ENOENT
'
qemu-nbd: option negotiation failed: Failed to read opts magic: Unexpected
end-of-file before all bytes were read
The aio_opt_abort counterpart does not need a completion closure, for
the same reason that aio_disconnect does not have one - the whole
point of this command is a clean shutdown, so you aren't going to rely
on a server reply, and there are no parameters that need cleanup.
Yes, it's a bit awkward that with just this patch, enabling opt_mode
means you HAVE to kill the connection without a data phase; that will
be rectified in the next patch adding nbd_opt_go. But breaking the
patches up makes review easier.
---
lib/internal.h | 1 +
generator/API.ml | 35 ++++++-
generator/state_machine.ml | 26 +++++
generator/states-magic.c | 6 +-
generator/states-newstyle-opt-go.c | 2 +-
generator/states-newstyle-opt-starttls.c | 4 +-
.../states-newstyle-opt-structured-reply.c | 7 +-
generator/states-newstyle.c | 59 +++++++++++-
lib/opt.c | 34 +++++++
tests/Makefile.am | 7 ++
tests/errors.c | 11 ++-
tests/newstyle-limited.c | 26 +++++
tests/opt-abort.c | 95 +++++++++++++++++++
.gitignore | 1 +
14 files changed, 302 insertions(+), 12 deletions(-)
create mode 100644 tests/opt-abort.c
diff --git a/lib/internal.h b/lib/internal.h
index 5f495fb..03baacd 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -101,6 +101,7 @@ struct nbd_handle {
/* Option negotiation mode. */
bool opt_mode;
+ uint8_t current_opt;
/* List exports mode. */
bool list_exports;
diff --git a/generator/API.ml b/generator/API.ml
index 3dd94f6..cbc1c33 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -699,10 +699,11 @@ negotiating everything on your behalf using settings made
before
starting the connection. To leave the mode and proceed on to the
ready state, you must use nbd_opt_go successfully; a failed
C<nbd_opt_go> returns to the negotiating state to allow a change of
-export name before trying again. You may also use nbd_opt_abort
+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"];
+ see_also = [Link "get_opt_mode"; Link
"aio_is_negotiating";
+ Link "opt_abort"];
};
"get_opt_mode", {
@@ -715,6 +716,19 @@ Return true if option negotiation mode was enabled on this
handle.";
see_also = [Link "set_opt_mode"];
};
+ "opt_abort", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and close the connection";
+ longdesc = "\
+Request that the server finish negotiation, gracefully if possible, then
+close the connection. This can only be used if L<nbd_set_opt_mode(3)>
+enabled option mode.";
+ example = Some "examples/list-exports.c";
+ see_also = [Link "set_opt_mode"; Link "aio_opt_abort"];
+ };
+
"set_list_exports", {
default_call with
args = [Bool "list"]; ret = RErr;
@@ -1883,6 +1897,21 @@ and completed the NBD handshake by calling
L<nbd_aio_is_ready(3)>,
on the connection.";
};
+ "aio_opt_abort", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and close the connection";
+ longdesc = "\
+Request that the server finish negotiation, gracefully if possible, then
+close the connection. 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.";
+ see_also = [Link "set_opt_mode"; Link "opt_abort"];
+ };
+
"aio_pread", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
@@ -2504,6 +2533,8 @@ let first_version = [
"set_opt_mode", (1, 4);
"get_opt_mode", (1, 4);
"aio_is_negotiating", (1, 4);
+ "opt_abort", (1, 4);
+ "aio_opt_abort", (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/state_machine.ml b/generator/state_machine.ml
index 8525520..2d28c2e 100644
--- a/generator/state_machine.ml
+++ b/generator/state_machine.ml
@@ -278,6 +278,8 @@ and newstyle_state_machine = [
(* Options. These state groups are always entered unconditionally,
* in this order. The START state in each group will check if the
* state needs to run and skip to the next state in the list if not.
+ * When opt_mode is set, control is returned to the user in state
+ * 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);
@@ -286,6 +288,30 @@ and newstyle_state_machine = [
Group ("OPT_GO", newstyle_opt_go_state_machine);
Group ("OPT_EXPORT_NAME", newstyle_opt_export_name_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.
+ *)
+ State {
+ default_state with
+ name = "PREPARE_OPT_ABORT";
+ comment = "Prepare to send NBD_OPT_ABORT";
+ external_events = [];
+ };
+
+ State {
+ default_state with
+ name = "SEND_OPT_ABORT";
+ comment = "Send NBD_OPT_ABORT to end negotiation";
+ external_events = [ NotifyWrite, "" ];
+ };
+
+ State {
+ default_state with
+ name = "SEND_OPTION_SHUTDOWN";
+ comment = "Sending write shutdown notification to the remote
server";
+ external_events = [ NotifyWrite, "" ];
+ };
+
(* When option parsing has successfully finished negotiation
* it will jump to this state for final steps before moving to
* the %READY state.
diff --git a/generator/states-magic.c b/generator/states-magic.c
index 944728d..644bf73 100644
--- a/generator/states-magic.c
+++ b/generator/states-magic.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
@@ -42,8 +42,10 @@ STATE_MACHINE {
}
version = be64toh (h->sbuf.new_handshake.version);
- if (version == NBD_NEW_VERSION)
+ if (version == NBD_NEW_VERSION) {
+ assert (h->current_opt == 0);
SET_NEXT_STATE (%.NEWSTYLE.START);
+ }
else if (version == NBD_OLD_VERSION)
SET_NEXT_STATE (%.OLDSTYLE.START);
else {
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index d696cae..1a59ae7 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -240,7 +240,7 @@ STATE_MACHINE {
set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%"
PRIx32,
reply);
}
- SET_NEXT_STATE (%.DEAD);
+ SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
break;
case NBD_REP_ACK:
SET_NEXT_STATE (%^FINISHED);
diff --git a/generator/states-newstyle-opt-starttls.c
b/generator/states-newstyle-opt-starttls.c
index 7162c7a..d107b74 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.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
@@ -93,7 +93,7 @@ STATE_MACHINE {
* then we can continue unencrypted.
*/
if (h->tls == LIBNBD_TLS_REQUIRE) {
- SET_NEXT_STATE (%.DEAD);
+ SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
set_error (ENOTSUP, "handshake: server refused TLS, "
"but handle TLS setting is 'require' (2)");
return 0;
diff --git a/generator/states-newstyle-opt-structured-reply.c
b/generator/states-newstyle-opt-structured-reply.c
index 58a76db..71a4952 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.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
@@ -84,7 +84,10 @@ STATE_MACHINE {
}
/* Next option. */
- SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
return 0;
} /* END STATE MACHINE */
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 8da4617..6613f9f 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -112,6 +112,26 @@ handle_reply_error (struct nbd_handle *h)
STATE_MACHINE {
NEWSTYLE.START:
+ if (h->opt_mode) {
+ /* NEWSTYLE can be entered multiple times, from MAGIC.CHECK_MAGIC and
+ * during various nbd_opt_* calls during NEGOTIATION. Each previous
+ * state has informed us what we still need to do.
+ */
+ switch (h->current_opt) {
+ case NBD_OPT_ABORT:
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (ENOTSUP, "handshake: server is not using fixed
newstyle");
+ return 0;
+ }
+ SET_NEXT_STATE (%PREPARE_OPT_ABORT);
+ return 0;
+ case 0:
+ break;
+ default:
+ abort ();
+ }
+ }
h->rbuf = &h->sbuf;
h->rlen = sizeof h->sbuf.gflags;
SET_NEXT_STATE (%RECV_GFLAGS);
@@ -153,13 +173,48 @@ STATE_MACHINE {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
/* Start sending options. */
- if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
- SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
+ }
else
SET_NEXT_STATE (%OPT_STARTTLS.START);
}
return 0;
+ NEWSTYLE.PREPARE_OPT_ABORT:
+ assert ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) != 0);
+ h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
+ h->sbuf.option.option = htobe32 (NBD_OPT_ABORT);
+ h->sbuf.option.optlen = htobe32 (0);
+ h->wbuf = &h->sbuf;
+ h->wlen = sizeof h->sbuf.option;
+ SET_NEXT_STATE (%SEND_OPT_ABORT);
+ return 0;
+
+ NEWSTYLE.SEND_OPT_ABORT:
+ switch (send_from_wbuf (h)) {
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
+ case 0:
+ SET_NEXT_STATE (%SEND_OPTION_SHUTDOWN);
+ }
+ return 0;
+
+ NEWSTYLE.SEND_OPTION_SHUTDOWN:
+ /* We don't care if the server replies to NBD_OPT_ABORT. However,
+ * unless we are in opt mode, we want to preserve the error message
+ * from a failed OPT_GO by moving to DEAD instead.
+ */
+ if (h->sock->ops->shut_writes (h, h->sock)) {
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.CLOSED);
+ else
+ SET_NEXT_STATE (%.DEAD);
+ }
+ return 0;
+
NEWSTYLE.FINISHED:
SET_NEXT_STATE (%.READY);
return 0;
diff --git a/lib/opt.c b/lib/opt.c
index 306a2e9..6243553 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -39,3 +39,37 @@ nbd_unlocked_get_opt_mode (struct nbd_handle *h)
{
return h->opt_mode;
}
+
+static int
+wait_for_option (struct nbd_handle *h)
+{
+ while (nbd_internal_is_state_connecting (get_next_state (h))) {
+ if (nbd_unlocked_poll (h, -1) == -1)
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Issue NBD_OPT_ABORT and wait for the state change. */
+int
+nbd_unlocked_opt_abort (struct nbd_handle *h)
+{
+ int r = nbd_unlocked_aio_opt_abort (h);
+
+ if (r == -1)
+ return r;
+
+ return wait_for_option (h);
+}
+
+/* Issue NBD_OPT_ABORT without waiting. */
+int
+nbd_unlocked_aio_opt_abort (struct nbd_handle *h)
+{
+ h->current_opt = NBD_OPT_ABORT;
+
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 46c819a..76b370a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -163,6 +163,7 @@ check_PROGRAMS += \
can-not-cache-flag \
oldstyle \
newstyle-limited \
+ opt-abort \
connect-unix \
connect-tcp \
aio-parallel \
@@ -198,6 +199,7 @@ TESTS += \
can-not-cache-flag \
oldstyle \
newstyle-limited \
+ opt-abort \
connect-unix \
connect-tcp \
aio-parallel.sh \
@@ -373,6 +375,11 @@ newstyle_limited_CPPFLAGS = -I$(top_srcdir)/include
newstyle_limited_CFLAGS = $(WARNINGS_CFLAGS)
newstyle_limited_LDADD = $(top_builddir)/lib/libnbd.la
+opt_abort_SOURCES = opt-abort.c
+opt_abort_CPPFLAGS = -I$(top_srcdir)/include
+opt_abort_CFLAGS = $(WARNINGS_CFLAGS)
+opt_abort_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/errors.c b/tests/errors.c
index cde0f48..1ae6555 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * 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
@@ -178,6 +178,15 @@ main (int argc, char *argv[])
}
check (EINVAL, "nbd_connect_command: ");
+ /* nbd_opt_abort can only be called during negotiation. */
+ if (nbd_opt_abort (nbd) != -1) {
+ fprintf (stderr, "%s: test failed: "
+ "nbd_opt_abort did not reject attempt in wrong state\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ check (EINVAL, "nbd_opt_abort: ");
+
/* Try to notify that writes are ready when we aren't blocked on POLLOUT
*/
if (nbd_aio_notify_write (nbd) != -1) {
fprintf (stderr, "%s: test failed: "
diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c
index 424da19..cdda474 100644
--- a/tests/newstyle-limited.c
+++ b/tests/newstyle-limited.c
@@ -121,6 +121,32 @@ main (int argc, char *argv[])
}
nbd_close (nbd);
+ /* Next try. Requesting opt_mode works, but opt_go is the only
+ * option that can succeed (via NBD_OPT_EXPORT_NAME); opt_abort is
+ * special-cased but moves to DEAD rather than CLOSED.
+ */
+ 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);
+ }
+ if (!nbd_aio_is_negotiating (nbd)) {
+ fprintf (stderr, "unexpected state after negotiating\n");
+ exit (EXIT_FAILURE);
+ }
+ /* XXX Test nbd_opt_list when it is implemented */
+ if (nbd_opt_abort (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (!nbd_aio_is_dead (nbd)) {
+ fprintf (stderr, "unexpected state after opt_abort\n");
+ exit (EXIT_FAILURE);
+ }
+ nbd_close (nbd);
+
/* And another try: an incorrect export name kills the connection,
* rather than allowing a second try.
*/
diff --git a/tests/opt-abort.c b/tests/opt-abort.c
new file mode 100644
index 0000000..3acc1fb
--- /dev/null
+++ b/tests/opt-abort.c
@@ -0,0 +1,95 @@
+/* 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_abort. */
+
+#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;
+ const char *s;
+ char *args[] = { "nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=512", NULL };
+
+ /* 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);
+ }
+
+ /* Protocol should be "newstyle-fixed", with structured replies
already
+ * negotiated.
+ */
+ if (nbd_aio_is_negotiating (nbd) != true) {
+ fprintf (stderr, "unexpected state after connection\n");
+ exit (EXIT_FAILURE);
+ }
+ s = nbd_get_protocol (nbd);
+ if (strcmp (s, "newstyle-fixed") != 0) {
+ fprintf (stderr,
+ "incorrect protocol \"%s\", expected
\"newstyle-fixed\"\n", s);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_get_structured_replies_negotiated (nbd)) != 1) {
+ fprintf (stderr,
+ "incorrect structured replies %" PRId64 ", expected
1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Quitting negotiation should be graceful. */
+ if (nbd_opt_abort (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_aio_is_closed (nbd) != true) {
+ fprintf (stderr, "unexpected state after abort\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* As negotiation never finished, we have no size. */
+ if ((r = nbd_get_size (nbd)) != -1) {
+ fprintf (stderr, "%s: test failed: incorrect size, "
+ "actual %" PRIi64 ", expected -1",
+ argv[0], r);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_get_errno ()) != EINVAL) {
+ fprintf (stderr, "%s: test failed: unexpected errno, "
+ "actual %" PRIi64 ", expected %d EINVAL",
+ argv[0], r, EINVAL);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index 44b4655..6fdb362 100644
--- a/.gitignore
+++ b/.gitignore
@@ -172,6 +172,7 @@ Makefile.in
/tests/meta-base-allocation
/tests/newstyle-limited
/tests/oldstyle
+/tests/opt-abort
/tests/pki/
/tests/read-only-flag
/tests/read-write-flag
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 07/13] api: Add nbd_opt_go and nbd_aio_opt_go
Time to make option negotiation actually useful, by now allowing the
user to request an export name before (re-)trying NBD_OPT_GO. This
also means that we have to reset state before each attempt at
NBD_OPT_GO (any meta contexts from the previous try must not be
inherited into the next try).
For now, I chose not to have a completion closure on nbd_aio_opt_go;
you can detect success or failure based on the resulting state, and
being able to set *err would be awkward (you can't magically get back
to negotiation state if the server has moved on to data phase). But
we have until 1.4 to decide if this decision needs to be revisited.
---
lib/internal.h | 1 +
generator/API.ml | 55 ++++++++++++++++---
generator/states-newstyle-opt-go.c | 6 +-
.../states-newstyle-opt-set-meta-context.c | 1 +
generator/states-newstyle.c | 6 ++
lib/flags.c | 28 +++++++++-
lib/handle.c | 7 +--
lib/opt.c | 26 +++++++++
tests/newstyle-limited.c | 9 ++-
9 files changed, 122 insertions(+), 17 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 03baacd..3672538 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -405,6 +405,7 @@ extern void nbd_internal_set_last_error (int errnum, char
*error);
} while (0)
/* flags.c */
+extern void nbd_internal_reset_size_and_flags (struct nbd_handle *h);
extern int nbd_internal_set_size_and_flags (struct nbd_handle *h,
uint64_t exportsize,
uint16_t eflags);
diff --git a/generator/API.ml b/generator/API.ml
index cbc1c33..21485ea 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -285,9 +285,15 @@ export names. The NBD protocol limits export names to
4096 bytes, but servers may not support the full length.
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)>.
+
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"];
+ see_also = [Link "get_export_name"; Link "connect_uri";
+ Link "set_opt_mode"; Link "opt_go"];
};
"get_export_name", {
@@ -697,13 +703,13 @@ When option mode is enabled, you have fine-grained control
over which
options are negotiated, compared to the default of the server
negotiating everything on your behalf using settings made before
starting the connection. To leave the mode and proceed on to the
-ready state, you must use nbd_opt_go successfully; a failed
-C<nbd_opt_go> returns to the negotiating state to allow a change of
+ready state, you must use L<nbd_opt_go(3)> successfully; a failed
+L<nbd_opt_go(3)> returns to the negotiating state to allow a change of
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_abort"; Link "opt_go"];
};
"get_opt_mode", {
@@ -716,6 +722,24 @@ Return true if option negotiation mode was enabled on this
handle.";
see_also = [Link "set_opt_mode"];
};
+ "opt_go", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and move on to using an export";
+ longdesc = "\
+Request that the server finish negotiation and move on to serving the
+export previously specified by L<nbd_set_export_name(3)>. This can only
+be used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+If this fails, the server may still be in negotiation, where it is
+possible to attempt another option such as a different export name;
+although older servers will instead have killed the connection.";
+ example = Some "examples/list-exports.c";
+ see_also = [Link "set_opt_mode"; Link "aio_opt_go";
Link "opt_abort";
+ Link "set_export_name"];
+ };
+
"opt_abort", {
default_call with
args = []; ret = RErr;
@@ -726,7 +750,7 @@ Request that the server finish negotiation, gracefully if
possible, then
close the connection. This can only be used if L<nbd_set_opt_mode(3)>
enabled option mode.";
example = Some "examples/list-exports.c";
- see_also = [Link "set_opt_mode"; Link "aio_opt_abort"];
+ see_also = [Link "set_opt_mode"; Link "aio_opt_abort";
Link "opt_go"];
};
"set_list_exports", {
@@ -938,8 +962,8 @@ L<nbd_set_export_name(3)> and L<nbd_set_tls(3)>
and other
calls as needed, followed by L<nbd_connect_tcp(3)> or
L<nbd_connect_unix(3)>. However, it is possible to override the
export name portion of a URI by using L<nbd_set_opt_mode(3)> to
-enable option mode, then using L<nbd_set_export_name(3)> as part
-of later negotiation.
+enable option mode, then using L<nbd_set_export_name(3)> and
+L<nbd_opt_go(3)> as part of subsequent negotiation.
This call returns when the connection has been made.
@@ -1897,6 +1921,21 @@ and completed the NBD handshake by calling
L<nbd_aio_is_ready(3)>,
on the connection.";
};
+ "aio_opt_go", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "end negotiation and move on to using an export";
+ longdesc = "\
+Request that the server finish negotiation and move on to serving the
+export previously specified by L<nbd_set_export_name(3)>. This can only
+be used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+To determine when the request completes, wait for
+L<nbd_aio_is_connecting(3)> to return false.";
+ see_also = [Link "set_opt_mode"; Link "opt_go"];
+ };
+
"aio_opt_abort", {
default_call with
args = []; ret = RErr;
@@ -2533,7 +2572,9 @@ let first_version = [
"set_opt_mode", (1, 4);
"get_opt_mode", (1, 4);
"aio_is_negotiating", (1, 4);
+ "opt_go", (1, 4);
"opt_abort", (1, 4);
+ "aio_opt_go", (1, 4);
"aio_opt_abort", (1, 4);
(* These calls are proposed for a future version of libnbd, but
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index 1a59ae7..0568695 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -240,7 +240,11 @@ STATE_MACHINE {
set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%"
PRIx32,
reply);
}
- SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
+ nbd_internal_reset_size_and_flags (h);
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
break;
case NBD_REP_ACK:
SET_NEXT_STATE (%^FINISHED);
diff --git a/generator/states-newstyle-opt-set-meta-context.c
b/generator/states-newstyle-opt-set-meta-context.c
index 77fd022..2dc8db8 100644
--- a/generator/states-newstyle-opt-set-meta-context.c
+++ b/generator/states-newstyle-opt-set-meta-context.c
@@ -28,6 +28,7 @@ STATE_MACHINE {
* contexts.
*/
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
+ nbd_internal_reset_size_and_flags (h);
if (!h->structured_replies ||
h->request_meta_contexts == NULL ||
nbd_internal_string_list_length (h->request_meta_contexts) == 0) {
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 6613f9f..d771097 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -118,6 +118,12 @@ STATE_MACHINE {
* state has informed us what we still need to do.
*/
switch (h->current_opt) {
+ case NBD_OPT_GO:
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
+ SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
+ else
+ SET_NEXT_STATE (%OPT_SET_META_CONTEXT.START);
+ return 0;
case NBD_OPT_ABORT:
if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
SET_NEXT_STATE (%.DEAD);
diff --git a/lib/flags.c b/lib/flags.c
index 223b77d..09eac72 100644
--- a/lib/flags.c
+++ b/lib/flags.c
@@ -27,6 +27,31 @@
#include "internal.h"
+/* Reset connection data. Called after swapping export name, after
+ * failed OPT_GO/OPT_INFO, or when starting a fresh OPT_SET_META_CONTEXT.
+ */
+void
+nbd_internal_reset_size_and_flags (struct nbd_handle *h)
+{
+ struct meta_context *m, *m_next;
+
+ h->exportsize = 0;
+ h->eflags = 0;
+ for (m = h->meta_contexts; m != NULL; m = m_next) {
+ m_next = m->next;
+ free (m->name);
+ free (m);
+ }
+ h->meta_contexts = NULL;
+ h->block_minimum = 0;
+ h->block_preferred = 0;
+ h->block_maximum = 0;
+ free (h->canonical_name);
+ h->canonical_name = NULL;
+ free (h->description);
+ h->description = NULL;
+}
+
/* Set the export size and eflags on the handle, validating them.
* This is called from the state machine when either the newstyle or
* oldstyle negotiation reaches the point that these are available.
@@ -194,7 +219,8 @@ nbd_unlocked_get_size (struct nbd_handle *h)
return h->exportsize;
}
-int64_t nbd_unlocked_get_block_size (struct nbd_handle *h, int type)
+int64_t
+nbd_unlocked_get_block_size (struct nbd_handle *h, int type)
{
switch (type) {
case LIBNBD_SIZE_MINIMUM:
diff --git a/lib/handle.c b/lib/handle.c
index 28c30dd..a51b923 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -112,7 +112,6 @@ nbd_create (void)
void
nbd_close (struct nbd_handle *h)
{
- struct meta_context *m, *m_next;
size_t i;
nbd_internal_set_error_context ("nbd_close");
@@ -126,11 +125,7 @@ nbd_close (struct nbd_handle *h)
nbd_unlocked_clear_debug_callback (h);
free (h->bs_entries);
- for (m = h->meta_contexts; m != NULL; m = m_next) {
- m_next = m->next;
- free (m->name);
- free (m);
- }
+ 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);
diff --git a/lib/opt.c b/lib/opt.c
index 6243553..6ad7f1a 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -51,6 +51,21 @@ wait_for_option (struct nbd_handle *h)
return 0;
}
+/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) and wait for the reply. */
+int
+nbd_unlocked_opt_go (struct nbd_handle *h)
+{
+ int r = nbd_unlocked_aio_opt_go (h);
+
+ if (r == -1)
+ return r;
+
+ r = wait_for_option (h);
+ if (r == 0 && nbd_internal_is_state_negotiating (get_next_state (h)))
+ return -1; /* NBD_OPT_GO failed, but can be attempted again */
+ return r;
+}
+
/* Issue NBD_OPT_ABORT and wait for the state change. */
int
nbd_unlocked_opt_abort (struct nbd_handle *h)
@@ -63,6 +78,17 @@ nbd_unlocked_opt_abort (struct nbd_handle *h)
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)
+{
+ h->current_opt = NBD_OPT_GO;
+
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
+
/* Issue NBD_OPT_ABORT without waiting. */
int
nbd_unlocked_aio_opt_abort (struct nbd_handle *h)
diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c
index cdda474..aa6f87b 100644
--- a/tests/newstyle-limited.c
+++ b/tests/newstyle-limited.c
@@ -152,11 +152,16 @@ main (int argc, char *argv[])
*/
nbd = nbd_create ();
if (nbd == NULL ||
- nbd_set_export_name (nbd, "b") == -1 ||
- nbd_connect_command (nbd, args) == 1) {
+ nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_connect_command (nbd, args) == -1 ||
+ nbd_set_export_name (nbd, "b") == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
+ if (nbd_opt_go (nbd) != -1) {
+ fprintf (stderr, "%s\n", "expected failure");
+ exit (EXIT_FAILURE);
+ }
if (!nbd_aio_is_dead (nbd)) {
fprintf (stderr, "unexpected state after failed export\n");
exit (EXIT_FAILURE);
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 08/13] examples: Update list-exports to demonstrate nbd_opt_go
There's more improvements coming, but this is enough to prove that
we've got a working option mode. We can now select an export name
from the same connection requesting the list.
---
examples/list-exports.c | 64 ++++++++++++++++++++++++++++-------------
1 file changed, 44 insertions(+), 20 deletions(-)
diff --git a/examples/list-exports.c b/examples/list-exports.c
index 643e611..daea2ab 100644
--- a/examples/list-exports.c
+++ b/examples/list-exports.c
@@ -4,9 +4,27 @@
* $ qemu-nbd -x "hello" -t -k /tmp/sock disk.img
* $ ./run examples/list-exports /tmp/sock
* [0] hello
- * Which export to connect to? 0
+ * Which export to connect to (-1 to quit)? 0
* Connecting to hello ...
* /tmp/sock: hello: size = 2048 bytes
+ *
+ * To test this with nbdkit (requires 1.22):
+ * $ nbdkit -U /tmp/sock sh - <<\EOF
+ * case $1 in
+ * list_exports) echo NAMES; echo foo; echo foobar ;;
+ * open) echo "$3" ;;
+ * get_size) echo "$2" | wc -c ;;
+ * pread) echo "$2" | dd skip=$4 count=$3 \
+ * iflag=skip_bytes,count_bytes ;;
+ * *) exit 2 ;;
+ * esac
+ * EOF
+ * $ ./run examples/list-exports /tmp/sock
+ * [0] foo
+ * [1] foobar
+ * Which export to connect to (-1 to quit)? 1
+ * Connecting to foobar ...
+ * /tmp/sock: foobar: size = 7 bytes
*/
#include <stdio.h>
@@ -20,7 +38,7 @@
int
main (int argc, char *argv[])
{
- struct nbd_handle *nbd, *nbd2;
+ struct nbd_handle *nbd;
int r, i;
char *name, *desc;
int64_t size;
@@ -30,14 +48,15 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- /* Create the libnbd handle for querying exports. */
+ /* Create the libnbd handle. */
nbd = nbd_create ();
if (nbd == NULL) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
- /* Set the list exports mode in the handle. */
+ /* Set opt mode and request list exports. */
+ nbd_set_opt_mode (nbd, true);
nbd_set_list_exports (nbd, true);
/* Connect to the NBD server over a
@@ -52,8 +71,8 @@ main (int argc, char *argv[])
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
-
- if (nbd_get_nr_list_exports (nbd) == 0) {
+ if (!nbd_aio_is_negotiating (nbd) ||
+ nbd_get_nr_list_exports (nbd) == 0) {
fprintf (stderr, "Server does not support "
"listing exports.\n");
exit (EXIT_FAILURE);
@@ -73,29 +92,34 @@ main (int argc, char *argv[])
}
printf ("Which export to connect to? ");
if (scanf ("%d", &i) != 1) exit (EXIT_FAILURE);
+ if (i == -1) {
+ if (nbd_opt_abort (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+ }
name = nbd_get_list_export_name (nbd, i);
+ if (name == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
printf ("Connecting to %s ...\n", name);
- nbd_close (nbd);
- /* Connect again to the chosen export. */
- nbd2 = nbd_create ();
- if (nbd2 == NULL) {
+ /* Resume connecting to the chosen export. */
+ if (nbd_set_export_name (nbd, name) == -1 ||
+ nbd_opt_go (nbd) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
-
- if (nbd_set_export_name (nbd2, name) == -1) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- exit (EXIT_FAILURE);
- }
-
- if (nbd_connect_unix (nbd2, argv[1]) == -1) {
- fprintf (stderr, "%s\n", nbd_get_error ());
+ if (!nbd_aio_is_ready (nbd)) {
+ fprintf (stderr, "server closed early\n");
exit (EXIT_FAILURE);
}
/* Read the size in bytes and print it. */
- size = nbd_get_size (nbd2);
+ size = nbd_get_size (nbd);
if (size == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
@@ -104,7 +128,7 @@ main (int argc, char *argv[])
argv[1], name, size);
/* Close the libnbd handle. */
- nbd_close (nbd2);
+ nbd_close (nbd);
free (name);
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 09/13] info: Simplify by using nbd_opt_go
Instead of having to munge a URI to supply a different export name, we
can exploit the fact that nbd_opt_mode lets us change the preferred
export name after nbd_connect_uri has established its socket.
In fact, by doing this, nbdinfo no longer needs to directly link
against libxml2, so it can now be built regardless of whether the
underlying libnbd.so supports URIs (although use of the program will
fail if libnbd.so was not built against libxml2).
---
info/Makefile.am | 6 ++---
info/nbdinfo.c | 63 +++++++++++++++++++-----------------------------
2 files changed, 27 insertions(+), 42 deletions(-)
diff --git a/info/Makefile.am b/info/Makefile.am
index 05b8137..d049d16 100644
--- a/info/Makefile.am
+++ b/info/Makefile.am
@@ -27,8 +27,6 @@ EXTRA_DIST = \
nbdinfo.pod \
$(NULL)
-if HAVE_LIBXML2
-
TESTS_ENVIRONMENT = LIBNBD_DEBUG=1
LOG_COMPILER = $(top_builddir)/run
TESTS @@ -39,11 +37,9 @@ nbdinfo_SOURCES = nbdinfo.c
nbdinfo_CPPFLAGS = -I$(top_srcdir)/include
nbdinfo_CFLAGS = \
$(WARNINGS_CFLAGS) \
- $(LIBXML2_CFLAGS) \
$(NULL)
nbdinfo_LDADD = \
$(top_builddir)/lib/libnbd.la \
- $(LIBXML2_LIBS) \
$(NULL)
if HAVE_POD
@@ -59,6 +55,8 @@ nbdinfo.1: nbdinfo.pod $(top_builddir)/podwrapper.pl
endif HAVE_POD
+if HAVE_LIBXML2
+
TESTS += \
info-list.sh \
info-list-json.sh \
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index b54dfd4..394f5ac 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -29,7 +29,6 @@
#include <limits.h>
#include <errno.h>
-#include <libxml/uri.h>
#include <libnbd.h>
static bool list_all = false;
@@ -422,7 +421,6 @@ static void
list_all_exports (struct nbd_handle *nbd1, const char *uri)
{
int i;
- xmlURIPtr xmluri = NULL;
int count = nbd_get_nr_list_exports (nbd1);
if (count == -1) {
@@ -434,49 +432,38 @@ list_all_exports (struct nbd_handle *nbd1, const char
*uri)
printf ("\t\"exports\": []\n");
for (i = 0; i < count; ++i) {
- char *name, *desc, *new_path, *new_uri;
+ char *name, *desc;
struct nbd_handle *nbd2;
name = nbd_get_list_export_name (nbd1, i);
- if (name) {
- /* We have to modify the original URI to change the export name.
- * In the URI spec, paths always start with '/' (which is
ignored).
- */
- xmluri = xmlParseURI (uri);
- if (!xmluri) {
- fprintf (stderr, "unable to parse original URI: %s\n", uri);
- exit (EXIT_FAILURE);
- }
- if (asprintf (&new_path, "/%s", name) == -1) {
- perror ("asprintf");
- exit (EXIT_FAILURE);
- }
- free (xmluri->path);
- xmluri->path = new_path;
- new_uri = (char *) xmlSaveUri (xmluri);
+ if (!name) {
+ fprintf (stderr, "unable to obtain export name: %s\n",
+ nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
- /* Connect to the new URI. */
- nbd2 = nbd_create ();
- if (nbd2 == NULL) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- exit (EXIT_FAILURE);
- }
- nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */
+ /* Connect to the original URI, but using opt mode to alter the export. */
+ nbd2 = nbd_create ();
+ if (nbd2 == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */
+ nbd_set_opt_mode (nbd2, true);
- if (nbd_connect_uri (nbd2, new_uri) == -1) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- exit (EXIT_FAILURE);
- }
+ if (nbd_connect_uri (nbd2, uri) == -1 ||
+ nbd_set_export_name (nbd2, name) == -1 ||
+ nbd_opt_go (nbd2) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
- /* 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 the metadata of this export. */
+ desc = nbd_get_list_export_description (nbd1, i);
+ list_one_export (nbd2, desc, i == 0, i + 1 == count);
- nbd_close (nbd2);
- free (new_uri);
- free (desc);
- xmlFreeURI (xmluri); /* this also frees xmluri->path == new_path */
- }
+ nbd_close (nbd2);
+ free (desc);
free (name);
}
}
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 10/13] api: Add nbd_opt_list
Adding NBD_OPT_LIST support will occur across two commits. This patch
switches the existing nbd_set_list_exports() (called at a distance
compared to where its results are used) to instead be the synchronous
nbd_opt_list() command; the next patch will then focus on making the
API friendlier to async usage by shifting from libnbd malloc'ing the
results to instead having the caller supply a callback function.
Thus, the API signature in this patch is not what we intend to be
final for 1.4, but rather a division to make review of the
transformation easier.
---
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 | 33 ++++++++++
tests/newstyle-limited.c | 9 ++-
examples/list-exports.c | 22 ++++---
interop/list-exports.c | 11 ++--
info/nbdinfo.c | 33 ++++------
12 files changed, 131 insertions(+), 117 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 3672538..3d059cc 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -103,8 +103,7 @@ struct nbd_handle {
bool opt_mode;
uint8_t current_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 21485ea..c660960 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", {
@@ -753,68 +755,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"];
};
@@ -824,10 +813,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", {
@@ -836,13 +825,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", {
@@ -2559,8 +2548,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);
@@ -2574,6 +2561,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 d771097..25ef351 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 6ad7f1a..7458b4b 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -78,6 +78,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->current_opt = 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/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/examples/list-exports.c b/examples/list-exports.c
index daea2ab..fcd474b 100644
--- a/examples/list-exports.c
+++ b/examples/list-exports.c
@@ -55,29 +55,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/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-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 11/13] 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 | 86 ++++++++++++++++-------
tests/newstyle-limited.c | 10 ++-
examples/list-exports.c | 69 ++++++++++++------
interop/list-exports.c | 68 ++++++++++--------
info/nbdinfo.c | 75 ++++++++++++++------
10 files changed, 316 insertions(+), 277 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 3d059cc..b723622 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 current_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 c660960..52970d3 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 = {
@@ -757,23 +761,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
@@ -782,56 +787,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", {
@@ -1940,6 +1904,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" ];
@@ -2548,9 +2534,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);
@@ -2564,6 +2547,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..b8d8025 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->current_opt == 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 7458b4b..1a5f645 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -26,6 +26,15 @@
#include "internal.h"
+/* Internal function which frees an option with callback. */
+void
+nbd_internal_free_option (struct nbd_handle *h)
+{
+ if (h->current_opt == 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)
{
@@ -78,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->current_opt = 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. */
@@ -132,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->current_opt = NBD_OPT_LIST;
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
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/examples/list-exports.c b/examples/list-exports.c
index fcd474b..9204c11 100644
--- a/examples/list-exports.c
+++ b/examples/list-exports.c
@@ -30,18 +30,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]);
@@ -63,8 +97,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);
}
@@ -74,24 +107,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) {
@@ -102,11 +127,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. */
@@ -132,7 +157,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/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
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 12/13] wip: api: Give aio_opt_go a completion callback
Squash this into opt_go? into opt_info? Standalone?
Testing: why does python not throw an exception:
$ ./run nbdsh
Welcome to nbdsh, the shell for interacting with
Network Block Device (NBD) servers.
The ‘nbd’ module has already been imported and there
is an open NBD handle called ‘h’.
h.connect_tcp ("remote", "10809") # Connect to a remote
server.
h.get_size () # Get size of the remote disk.
buf = h.pread (512, 0, 0) # Read the first sector.
exit() or Ctrl-D # Quit the shell
help (nbd) # Display documentation
nbd> h.set_opt_mode(True)
nbd>
h.connect_command(["nbdkit","-s","--exit-","eval",
"open=echo ENOENT >&2; exit 1"])
nbd> h.aio_is_negotiating()
True
nbd> h.set_export_name('a')
nbd> h.opt_go()
nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENT
nbd> h.set_export_name('b')
nbd> h.opt_go()
nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENT
nbd> h.set_opt_mode(True)
Traceback (most recent call last):
File "/usr/lib64/python3.8/code.py", line 90, in runcode
exec(code, self.locals)
File "<console>", line 1, in <module>
File "/home/eblake/libnbd/python/nbd.py", line 570, in set_opt_mode
return libnbdmod.set_opt_mode (self._o, enable)
nbd.Error: nbd_set_opt_mode: invalid state: NEGOTIATING: the handle must be
newly created: Invalid argument (EINVAL)
nbd> quit()
---
generator/API.ml | 12 ++++++++++--
generator/states-newstyle-opt-go.c | 19 +++++++++++++------
lib/opt.c | 18 ++++++++++++++++--
3 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/generator/API.ml b/generator/API.ml
index 52970d3..bf50030 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -1876,7 +1876,9 @@ on the connection.";
"aio_opt_go", {
default_call with
- args = []; ret = RErr;
+ args = [];
+ optargs = [ OClosure completion_closure ];
+ ret = RErr;
permitted_states = [ Negotiating ];
shortdesc = "end negotiation and move on to using an export";
longdesc = "\
@@ -1885,7 +1887,13 @@ export previously specified by
L<nbd_set_export_name(3)>. This can only
be used if L<nbd_set_opt_mode(3)> enabled option mode.
To determine when the request completes, wait for
-L<nbd_aio_is_connecting(3)> to return false.";
+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_go"];
};
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index 0568695..95cc041 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -124,6 +124,7 @@ STATE_MACHINE {
uint64_t exportsize;
uint16_t eflags;
uint32_t min, pref, max;
+ int err;
reply = be32toh (h->sbuf.or.option_reply.reply);
len = be32toh (h->sbuf.or.option_reply.replylen);
@@ -241,15 +242,21 @@ STATE_MACHINE {
reply);
}
nbd_internal_reset_size_and_flags (h);
- if (h->opt_mode)
- SET_NEXT_STATE (%.NEGOTIATING);
- else
- SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
+ err = nbd_get_errno ();
break;
case NBD_REP_ACK:
+ err = 0;
+ break;
+ }
+
+ CALL_CALLBACK (h->opt_cb.completion, &err);
+ nbd_internal_free_option (h);
+ if (err == 0)
SET_NEXT_STATE (%^FINISHED);
- break;
- }
+ else if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
return 0;
} /* END STATE MACHINE */
diff --git a/lib/opt.c b/lib/opt.c
index 1a5f645..17f2508 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -60,16 +60,28 @@ wait_for_option (struct nbd_handle *h)
return 0;
}
+static int
+go_complete (void *opaque, int *err)
+{
+ int *i = opaque;
+ *i = *err;
+ return 0;
+}
+
/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) and wait for the reply. */
int
nbd_unlocked_opt_go (struct nbd_handle *h)
{
- int r = nbd_unlocked_aio_opt_go (h);
+ int err;
+ nbd_completion_callback c = { .callback = go_complete, .user_data = &err
};
+ int r = nbd_unlocked_aio_opt_go (h, c);
if (r == -1)
return r;
r = wait_for_option (h);
+ if (r == 0)
+ r = err;
if (r == 0 && nbd_internal_is_state_negotiating (get_next_state (h)))
return -1; /* NBD_OPT_GO failed, but can be attempted again */
return r;
@@ -132,9 +144,11 @@ nbd_unlocked_opt_list (struct nbd_handle *h,
nbd_list_callback list)
/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */
int
-nbd_unlocked_aio_opt_go (struct nbd_handle *h)
+nbd_unlocked_aio_opt_go (struct nbd_handle *h,
+ nbd_completion_callback complete)
{
h->current_opt = NBD_OPT_GO;
+ h->opt_cb.completion = complete;
if (nbd_internal_run (h, cmd_issue) == -1)
debug (h, "option queued, ignoring state machine failure");
--
2.28.0
Eric Blake
2020-Aug-14 22:00 UTC
[Libguestfs] [libnbd PATCH v2 13/13] wip: api: add nbd_opt_info, nbd_aio_opt_info
Make it possible to query details about a potential export without
actually connecting. This resues the NBD_OPT_GO state machine, with
the main difference being that we can now return to negotiating state
even on success; thus checking for progression to Ready state is no
longer sufficient for detecting failure, and we have to rely on the
completion callback.
And now that we can get to Negotiation after a successful query, we
have to enable a lot more functions for use in that state to read the
results of the info query.
The next patch will then update nbdinfo to take advantage of this.
[TODO: fix tests/opt-info to actually test what is needed...]
---
docs/libnbd.pod | 4 ++
generator/API.ml | 108 +++++++++++++++++++++--------
generator/states-newstyle-opt-go.c | 29 +++++---
lib/opt.c | 35 ++++++++++
tests/Makefile.am | 7 ++
tests/newstyle-limited.c | 8 +++
tests/opt-info.c | 107 ++++++++++++++++++++++++++++
.gitignore | 1 +
8 files changed, 262 insertions(+), 37 deletions(-)
create mode 100644 tests/opt-info.c
diff --git a/docs/libnbd.pod b/docs/libnbd.pod
index 4ee0815..9cdf152 100644
--- a/docs/libnbd.pod
+++ b/docs/libnbd.pod
@@ -493,6 +493,10 @@ Note that there are some servers (like L<nbdkit(1)>
E<le> 1.14) which
ignore this, and other servers (like L<qemu-nbd(8)>) which require it
to be set correctly but cannot serve different content.
+These APIs are also available after a successful L<nbd_opt_info(3)>
+during the negotiation phase, if you used L<nbd_set_opt_mode(3)> prior
+to connecting.
+
=head2 Flag calls
After connecting the server will send back a set of flags describing
diff --git a/generator/API.ml b/generator/API.ml
index bf50030..e703de7 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -294,12 +294,14 @@ before beginning a connection. However, when
L<nbd_set_opt_mode(3)>
has enabled option mode, it is possible to change the export
name prior to L<nbd_opt_go(3)>. In particular, the use of
L<nbd_opt_list(3)> during negotiation can be used to determine
-a name the server is likely to accept.
+a name the server is likely to accept, and L<nbd_opt_info(3)> can
+be used to learn details about an export before connecting.
This call may be skipped if using L<nbd_connect_uri(3)> to connect
to a URI that includes an export name.";
see_also = [Link "get_export_name"; Link "connect_uri";
- Link "set_opt_mode"; Link "opt_go"; Link
"opt_list"];
+ Link "set_opt_mode"; Link "opt_go"; Link
"opt_list";
+ Link "opt_info"];
};
"get_export_name", {
@@ -353,7 +355,7 @@ Return the state of the full info request flag on this
handle.";
"get_canonical_export_name", {
default_call with
args = []; ret = RString;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "return the canonical export name, if the server has
one";
longdesc = "\
The NBD protocol permits a server to report an optional canonical
@@ -373,7 +375,7 @@ client specifically hinted about wanting it, via
L<nbd_set_full_info(3)>.";
"get_export_description", {
default_call with
args = []; ret = RString;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "return the export description, if the server has
one";
longdesc = "\
The NBD protocol permits a server to report an optional export
@@ -715,7 +717,8 @@ export name before trying again. You may also use
L<nbd_opt_abort(3)>
to end the connection without finishing negotiation.";
example = Some "examples/list-exports.c";
see_also = [Link "get_opt_mode"; Link
"aio_is_negotiating";
- Link "opt_abort"; Link "opt_go"; Link
"opt_list"];
+ Link "opt_abort"; Link "opt_go"; Link
"opt_list";
+ Link "opt_info"];
};
"get_opt_mode", {
@@ -743,7 +746,7 @@ possible to attempt another option such as a different
export name;
although older servers will instead have killed the connection.";
example = Some "examples/list-exports.c";
see_also = [Link "set_opt_mode"; Link "aio_opt_go";
Link "opt_abort";
- Link "set_export_name"];
+ Link "set_export_name"; Link "opt_info"];
};
"opt_abort", {
@@ -798,6 +801,29 @@ description is set with I<-D>.";
Link "set_export_name"];
};
+ "opt_info", {
+ default_call with
+ args = []; ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server for information about an export";
+ longdesc = "\
+Request that the server supply information about the most recent
+export name set by L<nbd_set_export_name(3)>. This can
+only be used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+If successful, functions like L<nbd_is_read_only(3)> and
+L<nbd_get_size(3)> will report details about that export. In
+general, if L<nbd_opt_go(3)> is called next, that call will
+likely succeed with the details remaining the same, although this
+is not guaranteed by all servers.
+
+Not all servers understand this request, and even when it is
+understood, the server might fail the request even when a
+corresponding L<nbd_opt_go(3)> would succeed.";
+ see_also = [Link "set_opt_mode"; Link "aio_opt_info";
Link "opt_go";
+ Link "set_export_name"];
+ };
+
"add_meta_context", {
default_call with
args = [ String "name" ]; ret = RErr;
@@ -1180,27 +1206,27 @@ is killed.";
"is_read_only", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "is the NBD export read-only?";
longdesc = "\
Returns true if the NBD export is read-only; writes and
write-like operations will fail."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls"];
+ see_also = [SectionLink "Flag calls"; Link "opt_info"];
example = Some "examples/server-flags.c";
};
"can_flush", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support the flush command?";
longdesc = "\
Returns true if the server supports the flush command
(see L<nbd_flush(3)>, L<nbd_aio_flush(3)>). Returns false if
the server does not."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "flush"; Link "aio_flush"];
example = Some "examples/server-flags.c";
};
@@ -1208,13 +1234,13 @@ the server does not."
"can_fua", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support the FUA flag?";
longdesc = "\
Returns true if the server supports the FUA flag on
certain commands (see L<nbd_pwrite(3)>)."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls"; Link "pwrite";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "pwrite";
Link "zero"; Link "trim"];
example = Some "examples/server-flags.c";
};
@@ -1222,7 +1248,7 @@ certain commands (see L<nbd_pwrite(3)>)."
"is_rotational", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "is the NBD disk rotational (like a disk)?";
longdesc = "\
Returns true if the disk exposed over NBD is rotational
@@ -1230,21 +1256,21 @@ Returns true if the disk exposed over NBD is rotational
the disk has no penalty for random access (like an SSD or
RAM disk)."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls"];
+ see_also = [SectionLink "Flag calls"; Link "opt_info"];
example = Some "examples/server-flags.c";
};
"can_trim", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support the trim command?";
longdesc = "\
Returns true if the server supports the trim command
(see L<nbd_trim(3)>, L<nbd_aio_trim(3)>). Returns false if
the server does not."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "trim"; Link "aio_trim"];
example = Some "examples/server-flags.c";
};
@@ -1252,14 +1278,14 @@ the server does not."
"can_zero", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support the zero command?";
longdesc = "\
Returns true if the server supports the zero command
(see L<nbd_zero(3)>, L<nbd_aio_zero(3)>). Returns false if
the server does not."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "zero"; Link "aio_zero";
Link "can_fast_zero"];
example = Some "examples/server-flags.c";
@@ -1268,7 +1294,7 @@ the server does not."
"can_fast_zero", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support the fast zero flag?";
longdesc = "\
Returns true if the server supports the use of the
@@ -1276,7 +1302,7 @@ C<LIBNBD_CMD_FLAG_FAST_ZERO> flag to the zero
command
(see L<nbd_zero(3)>, L<nbd_aio_zero(3)>). Returns false if
the server does not."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "zero"; Link "aio_zero"; Link
"can_zero"];
example = Some "examples/server-flags.c";
};
@@ -1284,7 +1310,7 @@ the server does not."
"can_df", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support the don't fragment flag to
pread?";
longdesc = "\
Returns true if the server supports structured reads with an
@@ -1292,7 +1318,7 @@ ability to request a non-fragmented read (see
L<nbd_pread_structured(3)>,
L<nbd_aio_pread_structured(3)>). Returns false if the server either
lacks
structured reads or if it does not support a non-fragmented read request."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "pread_structured";
Link "aio_pread_structured"];
example = Some "examples/server-flags.c";
@@ -1301,7 +1327,7 @@ structured reads or if it does not support a
non-fragmented read request."
"can_multi_conn", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support multi-conn?";
longdesc = "\
Returns true if the server supports multi-conn. Returns
@@ -1314,21 +1340,22 @@ way to check for this is to open one connection, check
this flag is true, then open further connections as
required."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Multi-conn"];
+ see_also = [SectionLink "Flag calls"; SectionLink
"Multi-conn";
+ Link "opt_info"];
example = Some "examples/server-flags.c";
};
"can_cache", {
default_call with
args = []; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support the cache command?";
longdesc = "\
Returns true if the server supports the cache command
(see L<nbd_cache(3)>, L<nbd_aio_cache(3)>). Returns false if
the server does not."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "cache"; Link "aio_cache"];
example = Some "examples/server-flags.c";
};
@@ -1336,7 +1363,7 @@ the server does not."
"can_meta_context", {
default_call with
args = [String "metacontext"]; ret = RBool;
- permitted_states = [ Connected; Closed ];
+ permitted_states = [ Negotiating; Connected; Closed ];
shortdesc = "does the server support a specific meta context?";
longdesc = "\
Returns true if the server supports the given meta context
@@ -1349,7 +1376,7 @@ B<E<lt>libnbd.hE<gt>> includes defined
constants for well-known
namespace contexts beginning with C<LIBNBD_CONTEXT_>, but you
are free to pass in other contexts."
^ non_blocking_test_call_description;
- see_also = [SectionLink "Flag calls";
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "add_meta_context";
Link "block_status"; Link
"aio_block_status"];
};
@@ -1934,6 +1961,29 @@ callback.";
see_also = [Link "set_opt_mode"; Link "opt_list"];
};
+ "aio_opt_info", {
+ default_call with
+ args = [];
+ optargs = [ OClosure completion_closure ];
+ ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server for information about an export";
+ longdesc = "\
+Request that the server supply information about the most recent
+export name set by L<nbd_set_export_name(3)>. This can
+only be used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+To determine when the request completes, wait for
+L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
+C<completion_callback> which will be invoked as described in
+L<libnbd(3)/Completion callbacks>. Note that direct detection of
+whether the server returns an error (as is done by the return value
+of the synchronous counterpart) is only possible with a completion
+callback; however, indirect detection of failure is possible by
+checking whether L<nbd_is_read_only(3)> and similar also fail.";
+ see_also = [Link "set_opt_mode"; Link "opt_info"; Link
"is_read_only"];
+ };
+
"aio_pread", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
@@ -2553,9 +2603,11 @@ let first_version = [
"opt_go", (1, 4);
"opt_abort", (1, 4);
"opt_list", (1, 4);
+ "opt_info", (1, 4);
"aio_opt_go", (1, 4);
"aio_opt_abort", (1, 4);
"aio_opt_list", (1, 4);
+ "aio_opt_info", (1, 4);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index 95cc041..54e83ab 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -23,8 +23,12 @@ STATE_MACHINE {
uint16_t nrinfos = h->full_info ? 3 : 1;
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
+ if (h->current_opt == NBD_OPT_INFO)
+ assert (h->opt_mode);
+ else
+ assert (h->current_opt == NBD_OPT_GO);
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
- h->sbuf.option.option = htobe32 (NBD_OPT_GO);
+ h->sbuf.option.option = htobe32 (h->current_opt);
h->sbuf.option.optlen htobe32 (/* exportnamelen */ 4 + strlen
(h->export_name)
+ sizeof nrinfos + 2 * nrinfos);
@@ -101,7 +105,7 @@ STATE_MACHINE {
switch (recv_into_rbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
- if (prepare_for_reply_payload (h, NBD_OPT_GO) == -1) {
+ if (prepare_for_reply_payload (h, h->current_opt) == -1) {
SET_NEXT_STATE (%.DEAD);
return 0;
}
@@ -206,9 +210,12 @@ STATE_MACHINE {
SET_NEXT_STATE (%RECV_REPLY);
return 0;
case NBD_REP_ERR_UNSUP:
- debug (h, "server is confused by NBD_OPT_GO, continuing anyway");
- SET_NEXT_STATE (%^OPT_EXPORT_NAME.START);
- return 0;
+ if (h->current_opt == NBD_OPT_GO) {
+ debug (h, "server is confused by NBD_OPT_GO, continuing
anyway");
+ SET_NEXT_STATE (%^OPT_EXPORT_NAME.START);
+ return 0;
+ }
+ /* fallthrough */
default:
if (handle_reply_error (h) == -1) {
SET_NEXT_STATE (%.DEAD);
@@ -216,6 +223,10 @@ STATE_MACHINE {
}
/* Decode expected known errors into a nicer string */
switch (reply) {
+ case NBD_REP_ERR_UNSUP:
+ assert (h->current_opt == NBD_OPT_INFO);
+ set_error (ENOTSUP, "handshake: server lacks NBD_OPT_INFO
support");
+ break;
case NBD_REP_ERR_POLICY:
case NBD_REP_ERR_PLATFORM:
set_error (0, "handshake: server policy prevents NBD_OPT_GO");
@@ -242,21 +253,21 @@ STATE_MACHINE {
reply);
}
nbd_internal_reset_size_and_flags (h);
- err = nbd_get_errno ();
+ err = nbd_get_errno () ? : ENOTSUP;
break;
case NBD_REP_ACK:
err = 0;
break;
}
- CALL_CALLBACK (h->opt_cb.completion, &err);
- nbd_internal_free_option (h);
- if (err == 0)
+ if (err == 0 && h->current_opt == NBD_OPT_GO)
SET_NEXT_STATE (%^FINISHED);
else if (h->opt_mode)
SET_NEXT_STATE (%.NEGOTIATING);
else
SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
+ CALL_CALLBACK (h->opt_cb.completion, &err);
+ nbd_internal_free_option (h);
return 0;
} /* END STATE MACHINE */
diff --git a/lib/opt.c b/lib/opt.c
index 17f2508..d0b9f78 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -87,6 +87,23 @@ nbd_unlocked_opt_go (struct nbd_handle *h)
return r;
}
+/* Issue NBD_OPT_INFO and wait for the reply. */
+int
+nbd_unlocked_opt_info (struct nbd_handle *h)
+{
+ int err;
+ nbd_completion_callback c = { .callback = go_complete, .user_data = &err
};
+ int r = nbd_unlocked_aio_opt_info (h, c);
+
+ if (r == -1)
+ return r;
+
+ r = wait_for_option (h);
+ if (r == 0)
+ r = err;
+ return r;
+}
+
/* Issue NBD_OPT_ABORT and wait for the state change. */
int
nbd_unlocked_opt_abort (struct nbd_handle *h)
@@ -155,6 +172,24 @@ nbd_unlocked_aio_opt_go (struct nbd_handle *h,
return 0;
}
+/* Issue NBD_OPT_INFO without waiting. */
+int
+nbd_unlocked_aio_opt_info (struct nbd_handle *h,
+ nbd_completion_callback complete)
+{
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
+ return -1;
+ }
+
+ h->current_opt = NBD_OPT_INFO;
+ h->opt_cb.completion = complete;
+
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
+
/* Issue NBD_OPT_ABORT without waiting. */
int
nbd_unlocked_aio_opt_abort (struct nbd_handle *h)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 76b370a..81cd30e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -164,6 +164,7 @@ check_PROGRAMS += \
oldstyle \
newstyle-limited \
opt-abort \
+ opt-info \
connect-unix \
connect-tcp \
aio-parallel \
@@ -200,6 +201,7 @@ TESTS += \
oldstyle \
newstyle-limited \
opt-abort \
+ opt-info \
connect-unix \
connect-tcp \
aio-parallel.sh \
@@ -380,6 +382,11 @@ opt_abort_CPPFLAGS = -I$(top_srcdir)/include
opt_abort_CFLAGS = $(WARNINGS_CFLAGS)
opt_abort_LDADD = $(top_builddir)/lib/libnbd.la
+opt_info_SOURCES = opt-info.c
+opt_info_CPPFLAGS = -I$(top_srcdir)/include
+opt_info_CFLAGS = $(WARNINGS_CFLAGS)
+opt_info_LDADD = $(top_builddir)/lib/libnbd.la
+
connect_unix_SOURCES = connect-unix.c
connect_unix_CPPFLAGS = -I$(top_srcdir)/include
connect_unix_CFLAGS = $(WARNINGS_CFLAGS)
diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c
index ab6c1c2..ce62bdb 100644
--- a/tests/newstyle-limited.c
+++ b/tests/newstyle-limited.c
@@ -152,6 +152,14 @@ main (int argc, char *argv[])
fprintf (stderr, "%s: wrong errno value after failed opt_list\n",
argv[0]);
exit (EXIT_FAILURE);
}
+ if (nbd_opt_info (nbd) != -1) {
+ fprintf (stderr, "nbd_opt_info: expected failure\n");
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_get_errno () != ENOTSUP) {
+ fprintf (stderr, "%s: wrong errno value after failed opt_info\n",
argv[0]);
+ exit (EXIT_FAILURE);
+ }
if (nbd_opt_abort (nbd) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
diff --git a/tests/opt-info.c b/tests/opt-info.c
new file mode 100644
index 0000000..d237351
--- /dev/null
+++ b/tests/opt-info.c
@@ -0,0 +1,107 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2020 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Test behavior of nbd_opt_info. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <string.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int64_t r;
+ const char *s;
+ /* A rather convoluted plugin: export '' is advertised but
+ * unavailable, export 'a' is unadvertised but available,
+ * export 'b' is unadvertised and unavailable, and all
+ * other exports are unadvertised but available. Available
+ * exports have a size matching the name length.
+ */
+ char *args[] = { "nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=512", NULL };
+
+ /* Quick check that nbdkit is new enough */
+ if (system ("nbdkit eval --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);
+ }
+
+ /* Protocol should be "newstyle-fixed", with structured replies
already
+ * negotiated.
+ */
+ if (nbd_aio_is_negotiating (nbd) != true) {
+ fprintf (stderr, "unexpected state after connection\n");
+ exit (EXIT_FAILURE);
+ }
+ s = nbd_get_protocol (nbd);
+ if (strcmp (s, "newstyle-fixed") != 0) {
+ fprintf (stderr,
+ "incorrect protocol \"%s\", expected
\"newstyle-fixed\"\n", s);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_get_structured_replies_negotiated (nbd)) != 1) {
+ fprintf (stderr,
+ "incorrect structured replies %" PRId64 ", expected
1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Quitting negotiation should be graceful. */
+ if (nbd_opt_abort (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_aio_is_closed (nbd) != true) {
+ fprintf (stderr, "unexpected state after abort\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* As negotiation never finished, we have no size. */
+ if ((r = nbd_get_size (nbd)) != -1) {
+ fprintf (stderr, "%s: test failed: incorrect size, "
+ "actual %" PRIi64 ", expected -1",
+ argv[0], r);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_get_errno ()) != EINVAL) {
+ fprintf (stderr, "%s: test failed: unexpected errno, "
+ "actual %" PRIi64 ", expected %d EINVAL",
+ argv[0], r, EINVAL);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index 6fdb362..1e49413 100644
--- a/.gitignore
+++ b/.gitignore
@@ -173,6 +173,7 @@ Makefile.in
/tests/newstyle-limited
/tests/oldstyle
/tests/opt-abort
+/tests/opt-info
/tests/pki/
/tests/read-only-flag
/tests/read-write-flag
--
2.28.0
Richard W.M. Jones
2020-Aug-17 11:06 UTC
Re: [Libguestfs] [libnbd PATCH v2 06/13] api: Add nbd_opt_abort and nbd_aio_opt_abort
On Fri, Aug 14, 2020 at 05:00:25PM -0500, Eric Blake wrote:> diff --git a/lib/internal.h b/lib/internal.h > index 5f495fb..03baacd 100644 > --- a/lib/internal.h > +++ b/lib/internal.h > @@ -101,6 +101,7 @@ struct nbd_handle { > > /* Option negotiation mode. */ > bool opt_mode; > + uint8_t current_opt;Be nice to add a comment here about what current_opt can contain, which would also explain why it has this somewhat unexpected type. Something like: + uint8_t current_opt; /* 0 or NBD_OPT_* */ (Can it only contain NBD_OPT_ABORT or are other options added in later patches?) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Richard W.M. Jones
2020-Aug-17 11:10 UTC
Re: [Libguestfs] [libnbd PATCH v2 09/13] info: Simplify by using nbd_opt_go
On Fri, Aug 14, 2020 at 05:00:28PM -0500, Eric Blake wrote:> Instead of having to munge a URI to supply a different export name, we > can exploit the fact that nbd_opt_mode lets us change the preferred > export name after nbd_connect_uri has established its socket. > > In fact, by doing this, nbdinfo no longer needs to directly link > against libxml2, so it can now be built regardless of whether the > underlying libnbd.so supports URIs (although use of the program will > fail if libnbd.so was not built against libxml2). > --- > info/Makefile.am | 6 ++--- > info/nbdinfo.c | 63 +++++++++++++++++++----------------------------- > 2 files changed, 27 insertions(+), 42 deletions(-) > > diff --git a/info/Makefile.am b/info/Makefile.am > index 05b8137..d049d16 100644 > --- a/info/Makefile.am > +++ b/info/Makefile.am > @@ -27,8 +27,6 @@ EXTRA_DIST = \ > nbdinfo.pod \ > $(NULL) > > -if HAVE_LIBXML2 > - > TESTS_ENVIRONMENT = LIBNBD_DEBUG=1 > LOG_COMPILER = $(top_builddir)/run > TESTS > @@ -39,11 +37,9 @@ nbdinfo_SOURCES = nbdinfo.c > nbdinfo_CPPFLAGS = -I$(top_srcdir)/include > nbdinfo_CFLAGS = \ > $(WARNINGS_CFLAGS) \ > - $(LIBXML2_CFLAGS) \ > $(NULL) > nbdinfo_LDADD = \ > $(top_builddir)/lib/libnbd.la \ > - $(LIBXML2_LIBS) \ > $(NULL) > > if HAVE_POD > @@ -59,6 +55,8 @@ nbdinfo.1: nbdinfo.pod $(top_builddir)/podwrapper.pl > > endif HAVE_POD > > +if HAVE_LIBXML2 > + > TESTS += \ > info-list.sh \ > info-list-json.sh \ > diff --git a/info/nbdinfo.c b/info/nbdinfo.c > index b54dfd4..394f5ac 100644 > --- a/info/nbdinfo.c > +++ b/info/nbdinfo.c > @@ -29,7 +29,6 @@ > #include <limits.h> > #include <errno.h> > > -#include <libxml/uri.h> > #include <libnbd.h> > > static bool list_all = false; > @@ -422,7 +421,6 @@ static void > list_all_exports (struct nbd_handle *nbd1, const char *uri) > { > int i; > - xmlURIPtr xmluri = NULL; > int count = nbd_get_nr_list_exports (nbd1); > > if (count == -1) { > @@ -434,49 +432,38 @@ list_all_exports (struct nbd_handle *nbd1, const char *uri) > printf ("\t\"exports\": []\n"); > > for (i = 0; i < count; ++i) { > - char *name, *desc, *new_path, *new_uri; > + char *name, *desc; > struct nbd_handle *nbd2; > > name = nbd_get_list_export_name (nbd1, i); > - if (name) { > - /* We have to modify the original URI to change the export name. > - * In the URI spec, paths always start with '/' (which is ignored). > - */ > - xmluri = xmlParseURI (uri); > - if (!xmluri) { > - fprintf (stderr, "unable to parse original URI: %s\n", uri); > - exit (EXIT_FAILURE); > - } > - if (asprintf (&new_path, "/%s", name) == -1) { > - perror ("asprintf"); > - exit (EXIT_FAILURE); > - } > - free (xmluri->path); > - xmluri->path = new_path; > - new_uri = (char *) xmlSaveUri (xmluri);Yes, good to get rid of this horrible bit of code! 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
Richard W.M. Jones
2020-Aug-17 11:19 UTC
Re: [Libguestfs] [libnbd PATCH v2 11/13] api: Add nbd_aio_opt_list
On Fri, Aug 14, 2020 at 05:00:30PM -0500, Eric Blake wrote:> +let list_closure = { > + cbname = "list"; > + cbargs = [ CBString "name"; CBString "description" ] > +}This is fine assuming we're certain that NBD_OPT_LIST / NBD_REP_SERVER (in the protocol) cannot be extended with more fields. Otherwise I'd suggesting adding some kind id/cookie here which the caller could use to get "further information", but would be useless at the moment. NBD_OPT_LIST is documented as returning NBD_REP_SERVER replies, and not anything else. The NBD protocol documentation tries to give wiggle room for NBD_REP_SERVER but is also clear that the extra field is a description field for user display, and I guess we're all using it for that now. It seems therefore that NBD_OPT_LIST cannot be extended, and so another option would have to be introduced in future, in which case we would support that option either by needing to extend this API or by a new API (aio_list2 or something). On balance therefore, this seems fine. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Richard W.M. Jones
2020-Aug-17 11:41 UTC
Re: [Libguestfs] [libnbd PATCH v2 11/13] api: Add nbd_aio_opt_list
ACK up to this patch (1-11). Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Richard W.M. Jones
2020-Aug-17 11:47 UTC
Re: [Libguestfs] [libnbd PATCH v2 12/13] wip: api: Give aio_opt_go a completion callback
On Fri, Aug 14, 2020 at 05:00:31PM -0500, Eric Blake wrote:> Squash this into opt_go? into opt_info? Standalone? > > Testing: why does python not throw an exception: > $ ./run nbdsh > > Welcome to nbdsh, the shell for interacting with > Network Block Device (NBD) servers. > > The ‘nbd’ module has already been imported and there > is an open NBD handle called ‘h’. > > h.connect_tcp ("remote", "10809") # Connect to a remote server. > h.get_size () # Get size of the remote disk. > buf = h.pread (512, 0, 0) # Read the first sector. > exit() or Ctrl-D # Quit the shell > help (nbd) # Display documentation > > nbd> h.set_opt_mode(True) > nbd> h.connect_command(["nbdkit","-s","--exit-","eval", > "open=echo ENOENT >&2; exit 1"]) > nbd> h.aio_is_negotiating() > True > nbd> h.set_export_name('a') > nbd> h.opt_go() > nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENT > nbd> h.set_export_name('b') > nbd> h.opt_go() > nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENTI don't know, but I would suggest first of all calling h.set_debug(True) which will display what the C API nbd_opt_go is actually returning. If it's not returning an error then Python bindings won't throw an exception (perhaps it just skips through the states if h->opt_mode is false?). I'm fairly sure the Python bindings themselves should be robust and always throw an exception if the underlying C returns an error, but if you suspect a problem them have a look at the generated code in python/methods.c for your new function. The patch itself looks like a good idea. Rich.> nbd> h.set_opt_mode(True) > Traceback (most recent call last): > File "/usr/lib64/python3.8/code.py", line 90, in runcode > exec(code, self.locals) > File "<console>", line 1, in <module> > File "/home/eblake/libnbd/python/nbd.py", line 570, in set_opt_mode > return libnbdmod.set_opt_mode (self._o, enable) > nbd.Error: nbd_set_opt_mode: invalid state: NEGOTIATING: the handle must be newly created: Invalid argument (EINVAL) > nbd> quit() > --- > generator/API.ml | 12 ++++++++++-- > generator/states-newstyle-opt-go.c | 19 +++++++++++++------ > lib/opt.c | 18 ++++++++++++++++-- > 3 files changed, 39 insertions(+), 10 deletions(-) > > diff --git a/generator/API.ml b/generator/API.ml > index 52970d3..bf50030 100644 > --- a/generator/API.ml > +++ b/generator/API.ml > @@ -1876,7 +1876,9 @@ on the connection."; > > "aio_opt_go", { > default_call with > - args = []; ret = RErr; > + args = []; > + optargs = [ OClosure completion_closure ]; > + ret = RErr; > permitted_states = [ Negotiating ]; > shortdesc = "end negotiation and move on to using an export"; > longdesc = "\ > @@ -1885,7 +1887,13 @@ export previously specified by L<nbd_set_export_name(3)>. This can only > be used if L<nbd_set_opt_mode(3)> enabled option mode. > > To determine when the request completes, wait for > -L<nbd_aio_is_connecting(3)> to return false."; > +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_go"]; > }; > > diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c > index 0568695..95cc041 100644 > --- a/generator/states-newstyle-opt-go.c > +++ b/generator/states-newstyle-opt-go.c > @@ -124,6 +124,7 @@ STATE_MACHINE { > uint64_t exportsize; > uint16_t eflags; > uint32_t min, pref, max; > + int err; > > reply = be32toh (h->sbuf.or.option_reply.reply); > len = be32toh (h->sbuf.or.option_reply.replylen); > @@ -241,15 +242,21 @@ STATE_MACHINE { > reply); > } > nbd_internal_reset_size_and_flags (h); > - if (h->opt_mode) > - SET_NEXT_STATE (%.NEGOTIATING); > - else > - SET_NEXT_STATE (%^PREPARE_OPT_ABORT); > + err = nbd_get_errno (); > break; > case NBD_REP_ACK: > + err = 0; > + break; > + } > + > + CALL_CALLBACK (h->opt_cb.completion, &err); > + nbd_internal_free_option (h); > + if (err == 0) > SET_NEXT_STATE (%^FINISHED); > - break; > - } > + else if (h->opt_mode) > + SET_NEXT_STATE (%.NEGOTIATING); > + else > + SET_NEXT_STATE (%^PREPARE_OPT_ABORT); > return 0; > > } /* END STATE MACHINE */ > diff --git a/lib/opt.c b/lib/opt.c > index 1a5f645..17f2508 100644 > --- a/lib/opt.c > +++ b/lib/opt.c > @@ -60,16 +60,28 @@ wait_for_option (struct nbd_handle *h) > return 0; > } > > +static int > +go_complete (void *opaque, int *err) > +{ > + int *i = opaque; > + *i = *err; > + return 0; > +} > + > /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) and wait for the reply. */ > int > nbd_unlocked_opt_go (struct nbd_handle *h) > { > - int r = nbd_unlocked_aio_opt_go (h); > + int err; > + nbd_completion_callback c = { .callback = go_complete, .user_data = &err }; > + int r = nbd_unlocked_aio_opt_go (h, c); > > if (r == -1) > return r; > > r = wait_for_option (h); > + if (r == 0) > + r = err; > if (r == 0 && nbd_internal_is_state_negotiating (get_next_state (h))) > return -1; /* NBD_OPT_GO failed, but can be attempted again */ > return r; > @@ -132,9 +144,11 @@ nbd_unlocked_opt_list (struct nbd_handle *h, nbd_list_callback list) > > /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ > int > -nbd_unlocked_aio_opt_go (struct nbd_handle *h) > +nbd_unlocked_aio_opt_go (struct nbd_handle *h, > + nbd_completion_callback complete) > { > h->current_opt = NBD_OPT_GO; > + h->opt_cb.completion = complete; > > if (nbd_internal_run (h, cmd_issue) == -1) > debug (h, "option queued, ignoring state machine failure"); > -- > 2.28.0 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Apparently Analagous Threads
- [libnbd PATCH v3 2/2] api: Add nbd_aio_opt_list
- [PATCH libnbd PROPOSAL] Add APIs for listing exports from an NBD server.
- [libnbd PATCH 2/2] generator: Free closures on failure
- [libnbd PATCH 3/3] api: Add nbd_opt_list_meta_context
- [libnbd PATCH 1/2] generator: Refactor handling of closures in unlocked functions