Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 00/18] Improve NBD_OPT_ control
v2 was here: https://listman.redhat.com/archives/libguestfs/2022-August/029726.html Since then, I've addressed even more bug fixes, improved comments and commit messages to better explain the changes along the way, added a couple more APIs (nbd_opt_list_meta_context_queries, nbd_opt_structured_reply), and improved some unit tests. A lot of this work is thanks to Laszlo's careful reviews. I still want to add nbd_opt_starttls, but this series has already grown big enough that it is worth getting more of it committed. Eric Blake (18): api: Fix crashes on nbd_connect_command with bad argv tests: Add coverage of unusual nbd_connect_command argv api: Allow nbd_opt_list_meta_context() without SR tests: Test nbd_opt_list_meta_context() without SR api: Localize list used during NBD_OPT_*_META_CONTEXT api: Add nbd_[aio_]opt_list_meta_context_queries tests: Language port of nbd_opt_list_meta_context_queries() api: Add nbd_set_request_meta_context() tests: Language port of nbd_set_request_meta_context() tests info: Explicitly skip NBD_OPT_SET_META_CONTEXT in --list mode api: Make nbd_opt_list_meta_context stateless tests: Add coverage for stateless nbd_opt_list_meta_context api: Reset state on changed nbd_set_export_name() tests: Add coverage for nbd_set_export_name fix api: Add nbd_[aio_]opt_set_meta_context[_queries] tests: Language port of nbd_opt_set_meta_context() tests api: Add nbd_[aio_]opt_structured_reply() tests: Language port of nbd_opt_structured_reply() tests lib/internal.h | 6 +- generator/API.ml | 430 +++++++++++++++++- generator/C.ml | 2 +- generator/states-newstyle-opt-go.c | 2 + generator/states-newstyle-opt-meta-context.c | 85 ++-- generator/states-newstyle-opt-starttls.c | 1 + .../states-newstyle-opt-structured-reply.c | 22 +- generator/states-newstyle.c | 4 + lib/connect.c | 10 +- lib/flags.c | 4 +- lib/handle.c | 22 + lib/opt.c | 143 +++++- lib/utils.c | 81 +++- python/t/110-defaults.py | 1 + python/t/120-set-non-defaults.py | 2 + python/t/230-opt-info.py | 36 +- python/t/240-opt-list-meta.py | 61 ++- python/t/245-opt-list-meta-queries.py | 68 +++ python/t/250-opt-set-meta.py | 134 ++++++ python/t/255-opt-set-meta-queries.py | 76 ++++ ocaml/tests/Makefile.am | 3 + ocaml/tests/test_110_defaults.ml | 2 + ocaml/tests/test_120_set_non_defaults.ml | 3 + ocaml/tests/test_230_opt_info.ml | 43 +- ocaml/tests/test_240_opt_list_meta.ml | 73 ++- ocaml/tests/test_245_opt_list_meta_queries.ml | 68 +++ ocaml/tests/test_250_opt_set_meta.ml | 170 +++++++ ocaml/tests/test_255_opt_set_meta_queries.ml | 79 ++++ tests/Makefile.am | 29 ++ tests/errors-connect-null.c | 88 ++++ tests/opt-info.c | 92 +++- tests/opt-list-meta-queries.c | 146 ++++++ tests/opt-list-meta.c | 135 +++++- tests/opt-set-meta-queries.c | 150 ++++++ tests/opt-set-meta.c | 259 +++++++++++ tests/opt-structured-twice.c | 145 ++++++ .gitignore | 5 + golang/Makefile.am | 3 + golang/libnbd_110_defaults_test.go | 8 + golang/libnbd_120_set_non_defaults_test.go | 12 + golang/libnbd_230_opt_info_test.go | 111 ++++- golang/libnbd_240_opt_list_meta_test.go | 176 ++++++- .../libnbd_245_opt_list_meta_queries_test.go | 115 +++++ golang/libnbd_250_opt_set_meta_test.go | 307 +++++++++++++ .../libnbd_255_opt_set_meta_queries_test.go | 141 ++++++ info/list.c | 3 +- info/show.c | 3 +- 47 files changed, 3405 insertions(+), 154 deletions(-) create mode 100644 python/t/245-opt-list-meta-queries.py create mode 100644 python/t/250-opt-set-meta.py create mode 100644 python/t/255-opt-set-meta-queries.py create mode 100644 ocaml/tests/test_245_opt_list_meta_queries.ml create mode 100644 ocaml/tests/test_250_opt_set_meta.ml create mode 100644 ocaml/tests/test_255_opt_set_meta_queries.ml create mode 100644 tests/errors-connect-null.c create mode 100644 tests/opt-list-meta-queries.c create mode 100644 tests/opt-set-meta-queries.c create mode 100644 tests/opt-set-meta.c create mode 100644 tests/opt-structured-twice.c create mode 100644 golang/libnbd_245_opt_list_meta_queries_test.go create mode 100644 golang/libnbd_250_opt_set_meta_test.go create mode 100644 golang/libnbd_255_opt_set_meta_queries_test.go -- 2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 01/18] api: Fix crashes on nbd_connect_command with bad argv
nbd_connect_command (h, (char **) { NULL }) triggers SIGABRT when
preparing to exec a NULL command name (during
enter_STATE_CONNECT_COMMAND_START in v1.0).
nbd_connect_command (h, NULL) in newer releases triggers SIGSEGV by
trying to dereference NULL (with LIBNBD_DEBUG=1, during
nbd_internal_printable_string_list in v1.4; otherwise, during
nbd_internal_set_argv in v1.6). In older releases, it behaves the
same as (char **) { NULL } and triggers SIGABRT.
Both crashes are corner cases that have existed for a long time, and
unlikely to hit in real code. Still, we shouldn't crash in a library.
Forbid a NULL StringList in general (similar to how we already forbid
a NULL String); which also means a StringList parameter implies
may_set_error=true. Refactor nbd_internal_set_argv() to split out the
vector population into a new helper function that can only fail with
ENOMEM (this function will also be useful in later patches that want
to copy a StringList, but can tolerate 0 elements), as well as to set
errors itself (all 2 callers updated) since detecting NULL for argv[0]
is unique to argv.
Tests are added in the next patch, to make it easier to review by
temporarily swapping patch order.
Changes from:
$ nbdsh -c 'h.connect_command([])'
nbdsh: generator/states-connect.c:247: enter_STATE_CONNECT_COMMAND_START:
Assertion `h->argv.ptr[0]' failed.
Aborted (core dumped)
to:
nbdsh: command line script failed: nbd_connect_command: missing command name in
argv list: Invalid argument
Fixes: 0b7bdf62 ("generator: Improve trace messages.", v1.3.2)
Fixes: 6f3eee2e ("lib: Replace a few uses of realloc with nbdkit vector
library.", v1.5.5)
Fixes: 8b164376 ("api: Get rid of nbd_connection.", v0.1)
---
lib/internal.h | 3 ++-
generator/API.ml | 3 ++-
generator/C.ml | 2 +-
lib/connect.c | 10 +++-------
lib/utils.c | 45 ++++++++++++++++++++++++++++++++++-----------
5 files changed, 42 insertions(+), 21 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index e4c08ca3..04bd8134 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -476,7 +476,8 @@ extern int nbd_internal_aio_get_direction (enum state
state);
/* utils.c */
extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp);
-extern int nbd_internal_set_argv (string_vector *v, char **argv);
+extern int nbd_internal_copy_string_list (string_vector *v, char **in);
+extern int nbd_internal_set_argv (struct nbd_handle *h, char **argv);
extern const char *nbd_internal_fork_safe_itoa (long v, char *buf, size_t len);
extern void nbd_internal_fork_safe_perror (const char *s);
extern char *nbd_internal_printable_buffer (const void *buf, size_t count);
diff --git a/generator/API.ml b/generator/API.ml
index abf77972..7be870a4 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -3458,7 +3458,8 @@ let () | name, { args; may_set_error = false }
when List.exists
(function
- | Closure _ | Enum _ | Flags _ | String _ -> true
+ | Closure _ | Enum _ | Flags _
+ | String _ | StringList _ -> true
| _ -> false) args ->
failwithf "%s: if args contains Closure/Enum/Flags/String
parameter, may_set_error must be false" name
diff --git a/generator/C.ml b/generator/C.ml
index b2d46f98..73ecffa0 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -570,7 +570,7 @@ let
need_out_label := true
| Flags (n, flags) ->
print_flags_check n flags None
- | String n ->
+ | String n | StringList n ->
let value = match errcode with
| Some value -> value
| None -> assert false in
diff --git a/lib/connect.c b/lib/connect.c
index 50080630..ffa50d5b 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 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
@@ -251,10 +251,8 @@ nbd_unlocked_aio_connect_socket (struct nbd_handle *h, int
sock)
int
nbd_unlocked_aio_connect_command (struct nbd_handle *h, char **argv)
{
- if (nbd_internal_set_argv (&h->argv, argv) == -1) {
- set_error (errno, "realloc");
+ if (nbd_internal_set_argv (h, argv) == -1)
return -1;
- }
return nbd_internal_run (h, cmd_connect_command);
}
@@ -263,10 +261,8 @@ int
nbd_unlocked_aio_connect_systemd_socket_activation (struct nbd_handle *h,
char **argv)
{
- if (nbd_internal_set_argv (&h->argv, argv) == -1) {
- set_error (errno, "realloc");
+ if (nbd_internal_set_argv (h, argv) == -1)
return -1;
- }
return nbd_internal_run (h, cmd_connect_sa);
}
diff --git a/lib/utils.c b/lib/utils.c
index 3d3b7f45..da3cee72 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 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
@@ -53,16 +53,17 @@ nbd_internal_hexdump (const void *data, size_t len, FILE
*fp)
}
}
-/* Replace the string_vector with a deep copy of argv (including final NULL).
*/
+/* Replace a string_vector with a deep copy of in (including final NULL). */
int
-nbd_internal_set_argv (string_vector *v, char **argv)
+nbd_internal_copy_string_list (string_vector *v, char **in)
{
size_t i;
+ assert (in);
string_vector_reset (v);
- for (i = 0; argv[i] != NULL; ++i) {
- char *copy = strdup (argv[i]);
+ for (i = 0; in[i] != NULL; ++i) {
+ char *copy = strdup (in[i]);
if (copy == NULL)
return -1;
if (string_vector_append (v, copy) == -1) {
@@ -74,6 +75,24 @@ nbd_internal_set_argv (string_vector *v, char **argv)
return string_vector_append (v, NULL);
}
+/* Store argv into h, or diagnose an error on failure. */
+int
+nbd_internal_set_argv (struct nbd_handle *h, char **argv)
+{
+ assert (argv);
+
+ if (argv[0] == NULL) {
+ set_error (EINVAL, "missing command name in argv list");
+ return -1;
+ }
+ if (nbd_internal_copy_string_list (&h->argv, argv) == -1) {
+ set_error (errno, "realloc");
+ return -1;
+ }
+
+ return 0;
+}
+
/* Like sprintf ("%ld", v), but safe to use between fork and exec.
Do
* not use this function in any other context.
*
@@ -247,13 +266,17 @@ nbd_internal_printable_string_list (char **list)
if (fp == NULL)
return NULL;
- fprintf (fp, "[");
- for (i = 0; list[i] != NULL; ++i) {
- if (i > 0)
- fprintf (fp, ", ");
- printable_string (list[i], fp);
+ if (list == NULL)
+ fprintf (fp, "NULL");
+ else {
+ fprintf (fp, "[");
+ for (i = 0; list[i] != NULL; ++i) {
+ if (i > 0)
+ fprintf (fp, ", ");
+ printable_string (list[i], fp);
+ }
+ fprintf (fp, "]");
}
- fprintf (fp, "]");
fclose (fp);
return s;
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 02/18] tests: Add coverage of unusual nbd_connect_command argv
Catch the library crashes patched in the previous commit.
---
tests/Makefile.am | 5 +++
tests/errors-connect-null.c | 88 +++++++++++++++++++++++++++++++++++++
.gitignore | 1 +
3 files changed, 94 insertions(+)
create mode 100644 tests/errors-connect-null.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7fc7dec3..f6a2c110 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -168,6 +168,7 @@ check_PROGRAMS += \
errors-not-connected \
errors-name-too-long \
errors-poll-no-fd \
+ errors-connect-null \
errors-connect-twice \
errors-not-negotiating \
errors-notify-not-blocked \
@@ -233,6 +234,7 @@ TESTS += \
errors-not-connected \
errors-name-too-long \
errors-poll-no-fd \
+ errors-connect-null \
errors-connect-twice \
errors-not-negotiating \
errors-notify-not-blocked \
@@ -309,6 +311,9 @@ errors_name_too_long_LDADD = $(top_builddir)/lib/libnbd.la
errors_poll_no_fd_SOURCES = errors-poll-no-fd.c
errors_poll_no_fd_LDADD = $(top_builddir)/lib/libnbd.la
+errors_connect_null_SOURCES = errors-connect-null.c
+errors_connect_null_LDADD = $(top_builddir)/lib/libnbd.la
+
errors_connect_twice_SOURCES = errors-connect-twice.c
errors_connect_twice_LDADD = $(top_builddir)/lib/libnbd.la
diff --git a/tests/errors-connect-null.c b/tests/errors-connect-null.c
new file mode 100644
index 00000000..9cda242e
--- /dev/null
+++ b/tests/errors-connect-null.c
@@ -0,0 +1,88 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2022 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
+ */
+
+/* Deliberately provoke some errors and check the error messages from
+ * nbd_get_error etc look reasonable.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+static char *progname;
+
+static void
+check (int experr, const char *prefix)
+{
+ const char *msg = nbd_get_error ();
+ int errnum = nbd_get_errno ();
+
+ fprintf (stderr, "error: \"%s\"\n", msg);
+ fprintf (stderr, "errno: %d (%s)\n", errnum, strerror (errnum));
+ if (strncmp (msg, prefix, strlen (prefix)) != 0) {
+ fprintf (stderr, "%s: test failed: missing context prefix: %s\n",
+ progname, msg);
+ exit (EXIT_FAILURE);
+ }
+ if (errnum != experr) {
+ fprintf (stderr, "%s: test failed: "
+ "expected errno = %d (%s), but got %d\n",
+ progname, experr, strerror (experr), errnum);
+ exit (EXIT_FAILURE);
+ }
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ const char *cmd[] = { NULL };
+
+ progname = argv[0];
+
+ nbd = nbd_create ();
+ if (nbd == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ /* Connect to an invalid command. */
+ if (nbd_connect_command (nbd, NULL) != -1) {
+ fprintf (stderr, "%s: test failed: "
+ "nbd_connect_command did not reject NULL argv\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ check (EFAULT, "nbd_connect_command: ");
+
+ if (nbd_connect_command (nbd, (char **) cmd) != -1) {
+ fprintf (stderr, "%s: test failed: "
+ "nbd_connect_command did not reject empty argv\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ check (EINVAL, "nbd_connect_command: ");
+
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index 071393fb..bc49860d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -204,6 +204,7 @@ Makefile.in
/tests/errors-client-unaligned
/tests/errors-client-unknown-flags
/tests/errors-client-zerosize
+/tests/errors-connect-null
/tests/errors-connect-twice
/tests/errors-enum
/tests/errors-multiple-disconnects
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 03/18] api: Allow nbd_opt_list_meta_context() without SR
Upstream NBD clarified (see NBD commit 13a4e33a8) that since
NBD_OPT_LIST_META_CONTEXT is stateless on the server side, it is
acceptable (but not mandatory) for servers to accept it without the
client having pre-negotiated structured replies. We aren't quite
stateless on the client side yet - that will be fixed in a later patch
to keep this one small and easier to test. The test is in the next
patch, to make it easier to temporarily reorder the series to see that
it catches the problem.
---
generator/states-newstyle-opt-meta-context.c | 4 +---
lib/opt.c | 6 +-----
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 84dbcd69..bfc51eb4 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -33,7 +33,7 @@ NEWSTYLE.OPT_META_CONTEXT.START:
* nbd_opt_go()
* -> conditionally use SET, next state OPT_GO for NBD_OPT_GO
* nbd_opt_list_meta_context()
- * -> conditionally use LIST, next state NEGOTIATING
+ * -> unconditionally use LIST, next state NEGOTIATING
*
* For now, we start by unconditionally clearing h->exportsize and
friends,
* as well as h->meta_contexts and h->meta_valid.
@@ -42,7 +42,6 @@ NEWSTYLE.OPT_META_CONTEXT.START:
* SET then manipulates h->meta_contexts, and sets h->meta_valid on
success.
* If OPT_GO is later successful, it populates h->exportsize and friends,
* and also sets h->meta_valid if we skipped SET here.
- * LIST is conditional, skipped if structured replies were not negotiated.
* There is a callback if and only if the command is LIST.
*/
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
@@ -52,7 +51,6 @@ NEWSTYLE.OPT_META_CONTEXT.START:
meta_vector_reset (&h->meta_contexts);
if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) {
assert (h->opt_mode);
- assert (h->structured_replies);
assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context));
opt = h->opt_current;
}
diff --git a/lib/opt.c b/lib/opt.c
index e5802f4d..d9114f4b 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2020-2021 Red Hat Inc.
+ * Copyright (C) 2020-2022 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
@@ -290,10 +290,6 @@ nbd_unlocked_aio_opt_list_meta_context (struct nbd_handle
*h,
set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
return -1;
}
- if (!h->structured_replies) {
- set_error (ENOTSUP, "server lacks structured replies");
- return -1;
- }
assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
h->opt_cb.fn.context = *context;
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 04/18] tests: Test nbd_opt_list_meta_context() without SR
This test proves that an attempt to list meta contexts goes to the
server rather than being rejected at the client, even with older
nbdkit that fails the attempt[1]; and includes ports to the various
languages. Done as a separate commit to allow temporary reordering of
the series to prove the test works.
[1] nbdkit didn't accept LIST_META_CONTENT before STRUCTURED_REPLY
until commit c39cba73 [v1.27.3, backported to 1.26.6]. Similarly,
qemu-nbd didn't accept it until commit da24597d [v6.2]. So, I was
able to test the graceful skip by adding a build of nbdkit without
that support at the front of my PATH.
---
python/t/240-opt-list-meta.py | 25 ++++++++++
ocaml/tests/test_240_opt_list_meta.ml | 30 ++++++++++++
tests/opt-list-meta.c | 34 ++++++++++++--
golang/libnbd_240_opt_list_meta_test.go | 61 ++++++++++++++++++++++++-
4 files changed, 144 insertions(+), 6 deletions(-)
diff --git a/python/t/240-opt-list-meta.py b/python/t/240-opt-list-meta.py
index 2b66eb5d..50fcfd69 100644
--- a/python/t/240-opt-list-meta.py
+++ b/python/t/240-opt-list-meta.py
@@ -31,6 +31,7 @@ def f(user_data, name):
seen = True
+# Get into negotiating state.
h = nbd.NBD()
h.set_opt_mode(True)
h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
@@ -77,3 +78,27 @@ def f(user_data, name):
assert seen is True
h.opt_abort()
+
+# Repeat but this time without structured replies. Deal gracefully
+# with older servers that don't allow the attempt.
+h = nbd.NBD()
+h.set_opt_mode(True)
+h.set_request_structured_replies(False)
+h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M"])
+bytes = h.stats_bytes_sent()
+
+try:
+ count = 0
+ seen = False
+ r = h.opt_list_meta_context(lambda *args: f(42, *args))
+ assert r == count
+ assert r >= 1
+ assert seen is True
+except nbd.Error as ex:
+ assert h.stats_bytes_sent() > bytes
+ print("ignoring failure from old server: %s" % ex.string)
+
+# FIXME: Once nbd_opt_structured_reply exists, use it here and retry.
+
+h.opt_abort()
diff --git a/ocaml/tests/test_240_opt_list_meta.ml
b/ocaml/tests/test_240_opt_list_meta.ml
index 639d33c8..64159185 100644
--- a/ocaml/tests/test_240_opt_list_meta.ml
+++ b/ocaml/tests/test_240_opt_list_meta.ml
@@ -17,6 +17,8 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*)
+open Printf
+
let count = ref 0
let seen = ref false
let f user_data name @@ -27,6 +29,7 @@ let
0
let () + (* Get into negotiating state. *)
let nbd = NBD.create () in
NBD.set_opt_mode nbd true;
NBD.connect_command nbd
@@ -75,6 +78,33 @@ let
assert (r = !count);
assert !seen;
+ NBD.opt_abort nbd;
+
+ (* Repeat but this time without structured replies. Deal gracefully
+ * with older servers that don't allow the attempt.
+ *)
+ let nbd = NBD.create () in
+ NBD.set_opt_mode nbd true;
+ NBD.set_request_structured_replies nbd false;
+ NBD.connect_command nbd
+ ["nbdkit"; "-s";
"--exit-with-parent"; "-v";
+ "memory"; "size=1M"];
+ let bytes = NBD.stats_bytes_sent nbd in
+ (try
+ count := 0;
+ seen := false;
+ let r = NBD.opt_list_meta_context nbd (f 42) in
+ assert (r = !count);
+ assert (r >= 1);
+ assert !seen
+ with
+ NBD.Error (errstr, errno) ->
+ assert (NBD.stats_bytes_sent nbd > bytes);
+ printf "ignoring failure from old server %s\n" errstr
+ );
+
+ (* FIXME: Once nbd_opt_structured_reply exists, use it here and retry. *)
+
NBD.opt_abort nbd
let () = Gc.compact ()
diff --git a/tests/opt-list-meta.c b/tests/opt-list-meta.c
index 9ad8e37e..dc9c6799 100644
--- a/tests/opt-list-meta.c
+++ b/tests/opt-list-meta.c
@@ -17,6 +17,7 @@
*/
/* Test behavior of nbd_opt_list_meta_context. */
+/* See also unit test 240 in the various language ports. */
#include <config.h>
@@ -56,6 +57,7 @@ main (int argc, char *argv[])
"memory", "size=1M", NULL };
int max;
char *tmp;
+ uint64_t bytes;
/* Get into negotiating state. */
nbd = nbd_create ();
@@ -164,7 +166,9 @@ main (int argc, char *argv[])
nbd_opt_abort (nbd);
nbd_close (nbd);
- /* Repeat but this time without structured replies. */
+ /* Repeat but this time without structured replies. Deal gracefully
+ * with older servers that don't allow the attempt.
+ */
nbd = nbd_create ();
if (nbd == NULL ||
nbd_set_opt_mode (nbd, true) == -1 ||
@@ -173,16 +177,36 @@ main (int argc, char *argv[])
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
+ bytes = nbd_stats_bytes_sent (nbd);
- /* FIXME: For now, we reject this client-side, but it is overly strict. */
p = (struct progress) { .count = 0 };
r = nbd_opt_list_meta_context (nbd,
(nbd_context_callback) { .callback = check,
.user_data =
&p});
- if (r != -1) {
- fprintf (stderr, "not expecting command to succeed\n");
- exit (EXIT_FAILURE);
+ if (r == -1) {
+ if (nbd_stats_bytes_sent (nbd) == bytes) {
+ fprintf (stderr, "bug: client failed to send request\n");
+ exit (EXIT_FAILURE);
+ }
+ fprintf (stdout, "ignoring failure from old server: %s\n",
+ nbd_get_error ());
}
+ else {
+ if (r != p.count) {
+ fprintf (stderr, "inconsistent return value %d, expected %d\n",
+ r, p.count);
+ exit (EXIT_FAILURE);
+ }
+ if (r < 1 || !p.seen) {
+ fprintf (stderr, "server did not reply with
base:allocation\n");
+ exit (EXIT_FAILURE);
+ }
+ }
+
+ /* FIXME: Once nbd_opt_structured_reply() exists, use it here and retry. */
+
+ nbd_opt_abort (nbd);
+ nbd_close (nbd);
exit (EXIT_SUCCESS);
}
diff --git a/golang/libnbd_240_opt_list_meta_test.go
b/golang/libnbd_240_opt_list_meta_test.go
index d7322752..2b49c895 100644
--- a/golang/libnbd_240_opt_list_meta_test.go
+++ b/golang/libnbd_240_opt_list_meta_test.go
@@ -1,5 +1,5 @@
/* libnbd golang tests
- * Copyright (C) 2013-2021 Red Hat Inc.
+ * Copyright (C) 2013-2022 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
@@ -19,6 +19,7 @@
package libnbd
import (
+ "fmt";
"testing"
)
@@ -37,6 +38,7 @@ func listmetaf(user_data int, name string) int {
}
func Test240OptListMeta(t *testing.T) {
+ /* Get into negotiating state. */
h, err := Create()
if err != nil {
t.Fatalf("could not create handle: %s", err)
@@ -143,4 +145,61 @@ func Test240OptListMeta(t *testing.T) {
if err != nil {
t.Fatalf("could not request opt_abort: %s", err)
}
+
+ /* Repeat but this time without structured replies. Deal gracefully
+ * with older servers that don't allow the attempt.
+ */
+ h, err = Create()
+ if err != nil {
+ t.Fatalf("could not create handle: %s", err)
+ }
+ defer h.Close()
+
+ err = h.SetOptMode(true)
+ if err != nil {
+ t.Fatalf("could not set opt mode: %s", err)
+ }
+
+ err = h.SetRequestStructuredReplies(false)
+ if err != nil {
+ t.Fatalf("could not set request structured replies: %s", err)
+ }
+
+ err = h.ConnectCommand([]string{
+ "nbdkit", "-s", "--exit-with-parent",
"-v",
+ "memory", "size=1M",
+ })
+ if err != nil {
+ t.Fatalf("could not connect: %s", err)
+ }
+
+ bytes, err := h.StatsBytesSent()
+ if err != nil {
+ t.Fatalf("could not collect stats: %s", err)
+ }
+
+ count = 0
+ seen = false
+ r, err = h.OptListMetaContext(func(name string) int {
+ return listmetaf(42, name)
+ })
+ if err != nil {
+ bytes2, err2 := h.StatsBytesSent()
+ if err2 != nil {
+ t.Fatalf("could not collect stats: %s", err2)
+ }
+ if bytes2 <= bytes {
+ t.Fatalf("unexpected bytes sent after opt_list_meta_context")
+ }
+ fmt.Printf("ignoring failure from old server: %s", err)
+ } else if r < 1 || r != count || !seen {
+ t.Fatalf("unexpected count after opt_list_meta_context")
+ }
+
+ /* FIXME: Once nbd_opt_structured_reply exists, use it here and retry. */
+
+ err = h.OptAbort()
+ if err != nil {
+ t.Fatalf("could not request opt_abort: %s", err)
+ }
}
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 05/18] api: Localize list used during NBD_OPT_*_META_CONTEXT
Right now, our generator ensures that we are in the Negotiating state
before a client can call nbd_add_meta_context() while there is an
active server connection. That is, even if the user piles up enough
meta contexts that a slow-response server then causes nbd_aio_opt_go()
and friends to return early on blocked writes, with the state machine
now tracking pointers into the middle of the global list, those
pointers are not at risk of becoming a use-after-free because the user
cannot successfully call nbd_add_meta_context() to alter the global
list until they first resume the state machine (using nbd_aio_poll or
similar) to finish flushing the client's write to the server and
eventually get back to the Negotiating state.
However, it took me a while to perform that audit to ensure that the
user can't mess with the global list while the state machine is
peering into the middle of it (that is, state machine interlocks act
at a distance). Easier is to have the state machine act on a local
copy, which cannot be directly altered by user action regardless of
the current state; and therefore even if we later change our mind
about which states can call nbd_add_meta_context() (the state
restrictions in API.ml), that change won't accidentally cause our
state machine to start seeing corrupted list contents on the uncommon
case of blocked writes (in states-newstyle-opt-meta-context.c).
In this patch, I added a helper function to copy from either a
StringList (no clients of that parameter in this patch, but coming
soon) or from the implicit list. Also, at the time of this patch, the
copying is done once in the state machine, but later patches will
hoist it earlier into lib/opt.c as part of adding more APIs for
fine-grained control over nbd_set_opt_mode management of context
lists, starting with nbd_opt_list_meta_context_queries() which will
take an explicit list of meta contexts to query.
---
lib/internal.h | 2 ++
generator/states-newstyle-opt-meta-context.c | 16 +++++----
lib/handle.c | 1 +
lib/utils.c | 36 ++++++++++++++++++++
4 files changed, 49 insertions(+), 6 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 04bd8134..308e65ef 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -285,6 +285,7 @@ struct nbd_handle {
int connect_errno;
/* When sending metadata contexts, this is used. */
+ string_vector querylist;
size_t querynum;
/* When receiving block status, this is used. */
@@ -478,6 +479,7 @@ extern int nbd_internal_aio_get_direction (enum state
state);
extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp);
extern int nbd_internal_copy_string_list (string_vector *v, char **in);
extern int nbd_internal_set_argv (struct nbd_handle *h, char **argv);
+extern int nbd_internal_set_querylist (struct nbd_handle *h, char **in);
extern const char *nbd_internal_fork_safe_itoa (long v, char *buf, size_t len);
extern void nbd_internal_fork_safe_perror (const char *s);
extern char *nbd_internal_printable_buffer (const void *buf, size_t count);
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index bfc51eb4..31e84f35 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -65,10 +65,14 @@ NEWSTYLE.OPT_META_CONTEXT.START:
assert (!h->meta_valid);
+ if (nbd_internal_set_querylist (h, NULL) == -1) {
+ SET_NEXT_STATE (%.DEAD);
+ return 0;
+ }
/* Calculate the length of the option request data. */
len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
- for (i = 0; i < h->request_meta_contexts.len; ++i)
- len += 4 /* length of query */ + strlen
(h->request_meta_contexts.ptr[i]);
+ for (i = 0; i < h->querylist.len; ++i)
+ len += 4 /* length of query */ + strlen (h->querylist.ptr[i]);
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
h->sbuf.option.option = htobe32 (opt);
@@ -107,7 +111,7 @@ NEWSTYLE.OPT_META_CONTEXT.SEND_EXPORTNAME:
switch (send_from_wbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
- h->sbuf.nrqueries = htobe32 (h->request_meta_contexts.len);
+ h->sbuf.nrqueries = htobe32 (h->querylist.len);
h->wbuf = &h->sbuf;
h->wlen = sizeof h->sbuf.nrqueries;
h->wflags = MSG_MORE;
@@ -125,12 +129,12 @@ NEWSTYLE.OPT_META_CONTEXT.SEND_NRQUERIES:
return 0;
NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY:
- if (h->querynum >= h->request_meta_contexts.len) {
+ if (h->querynum >= h->querylist.len) {
/* end of list of requested meta contexts */
SET_NEXT_STATE (%PREPARE_FOR_REPLY);
return 0;
}
- const char *query = h->request_meta_contexts.ptr[h->querynum];
+ const char *query = h->querylist.ptr[h->querynum];
h->sbuf.len = htobe32 (strlen (query));
h->wbuf = &h->sbuf.len;
@@ -140,7 +144,7 @@ NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY:
return 0;
NEWSTYLE.OPT_META_CONTEXT.SEND_QUERYLEN:
- const char *query = h->request_meta_contexts.ptr[h->querynum];
+ const char *query = h->querylist.ptr[h->querynum];
switch (send_from_wbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
diff --git a/lib/handle.c b/lib/handle.c
index d4426260..6f262ba3 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -133,6 +133,7 @@ nbd_close (struct nbd_handle *h)
/* Free user callbacks first. */
nbd_unlocked_clear_debug_callback (h);
+ string_vector_reset (&h->querylist);
free (h->bs_entries);
nbd_internal_reset_size_and_flags (h);
for (i = 0; i < h->meta_contexts.len; ++i)
diff --git a/lib/utils.c b/lib/utils.c
index da3cee72..7b514421 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -93,6 +93,42 @@ nbd_internal_set_argv (struct nbd_handle *h, char **argv)
return 0;
}
+/* Copy queries (defaulting to h->request_meta_contexts) into
h->querylist.
+ * Set an error on failure.
+ */
+int
+nbd_internal_set_querylist (struct nbd_handle *h, char **queries)
+{
+ size_t i;
+
+ if (queries) {
+ if (nbd_internal_copy_string_list (&h->querylist, queries) == -1) {
+ set_error (errno, "realloc");
+ return -1;
+ }
+ /* Drop trailing NULL */
+ assert (h->querylist.len > 0);
+ string_vector_remove (&h->querylist, h->querylist.len - 1);
+ }
+ else {
+ string_vector_reset (&h->querylist);
+ for (i = 0; i < h->request_meta_contexts.len; ++i) {
+ char *copy = strdup (h->request_meta_contexts.ptr[i]);
+ if (copy == NULL) {
+ set_error (errno, "strdup");
+ return -1;
+ }
+ if (string_vector_append (&h->querylist, copy) == -1) {
+ set_error (errno, "realloc");
+ free (copy);
+ return -1;
+ }
+ }
+ }
+
+ return 0;
+}
+
/* Like sprintf ("%ld", v), but safe to use between fork and exec.
Do
* not use this function in any other context.
*
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 06/18] api: Add nbd_[aio_]opt_list_meta_context_queries
Rather than always relying on the implicit global list managed by
nbd_add_meta_context() and nbd_clear_meta_contexts(), we can take the
user's list of contexts as a direct parameter. This finally makes use
of the non-NULL queries parameter of the nbd_internal_set_querylist()
added in a previous patch.
The C test is added along with this commit (applying the new test
independently wouldn't compile), but the language bindings are done
separately for ease of review.
---
generator/API.ml | 99 ++++++++++++-
generator/states-newstyle-opt-meta-context.c | 8 +-
lib/opt.c | 25 +++-
tests/Makefile.am | 5 +
tests/opt-list-meta-queries.c | 145 +++++++++++++++++++
.gitignore | 1 +
6 files changed, 272 insertions(+), 11 deletions(-)
create mode 100644 tests/opt-list-meta-queries.c
diff --git a/generator/API.ml b/generator/API.ml
index 7be870a4..0c72c356 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -1169,12 +1169,15 @@ "opt_list_meta_context", {
default_call with
args = [ Closure context_closure ]; ret = RInt;
permitted_states = [ Negotiating ];
- shortdesc = "request the server to list available meta contexts";
+ shortdesc = "list available meta contexts, using implicit query
list";
longdesc = "\
Request that the server list available meta contexts associated with
the export previously specified by the most recent
-L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>. This can only
be
-used if L<nbd_set_opt_mode(3)> enabled option mode.
+L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>, and with a
+list of queries from prior calls to L<nbd_add_meta_context(3)>
+(see L<nbd_opt_list_meta_context_queries(3)> if you want to supply
+an explicit query list instead). This can only be used if
+L<nbd_set_opt_mode(3)> enabled option mode.
The NBD protocol allows a client to decide how many queries to ask
the server. Rather than taking that list of queries as a parameter
@@ -1211,10 +1214,61 @@ "opt_list_meta_context", {
replies that might be advertised, so client code should be aware that
a server may send a lengthy list.";
see_also = [Link "set_opt_mode"; Link
"aio_opt_list_meta_context";
+ Link "opt_list_meta_context_queries";
Link "add_meta_context"; Link
"clear_meta_contexts";
Link "opt_go"; Link "set_export_name"];
};
+ "opt_list_meta_context_queries", {
+ default_call with
+ args = [ StringList "queries"; Closure context_closure ]; ret =
RInt;
+ permitted_states = [ Negotiating ];
+ shortdesc = "list available meta contexts, using explicit query
list";
+ longdesc = "\
+Request that the server list available meta contexts associated with
+the export previously specified by the most recent
+L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>, and with an
+explicit list of queries provided as a parameter (see
+L<nbd_opt_list_meta_context(3)> if you want to reuse an
+implicit query list instead). This can only be used if
+L<nbd_set_opt_mode(3)> enabled option mode.
+
+The NBD protocol allows a client to decide how many queries to ask
+the server. For this function, the list is explicit in the C<queries>
+parameter. When the list is empty, a server will typically reply with all
+contexts that it supports; when the list is non-empty, the server
+will reply only with supported contexts that match the client's
+request. Note that a reply by the server might be encoded to
+represent several feasible contexts within one string, rather than
+multiple strings per actual context name that would actually succeed
+during L<nbd_opt_go(3)>; so it is still necessary to use
+L<nbd_can_meta_context(3)> after connecting to see which contexts
+are actually supported.
+
+The C<context> function is called once per server reply, with any
+C<user_data> passed to this function, and with C<name> supplied by
+the server. Remember that it is not safe to call
+L<nbd_add_meta_context(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.
+
+For convenience, when this function succeeds, it returns the number
+of replies returned by the server.
+
+Not all servers understand this request, and even when it is understood,
+the server might intentionally send an empty list because it does not
+support the requested context, or may encounter a failure after
+delivering partial results. Thus, this function may succeed even when
+no contexts 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
+replies that might be advertised, so client code should be aware that
+a server may send a lengthy list.";
+ see_also = [Link "set_opt_mode"; Link
"aio_opt_list_meta_context_queries";
+ Link "opt_list_meta_context";
+ Link "opt_go"; Link "set_export_name"];
+ };
+
"add_meta_context", {
default_call with
args = [ String "name" ]; ret = RErr;
@@ -2599,11 +2653,14 @@ "aio_opt_list_meta_context", {
args = [ Closure context_closure ]; ret = RInt;
optargs = [ OClosure completion_closure ];
permitted_states = [ Negotiating ];
- shortdesc = "request the server to list available meta contexts";
+ shortdesc = "request list of available meta contexts, using implicit
query";
longdesc = "\
Request that the server list available meta contexts associated with
the export previously specified by the most recent
-L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>. This can only
be
+L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>, and with a
+list of queries from prior calls to L<nbd_add_meta_context(3)>
+(see L<nbd_aio_opt_list_meta_context_queries(3)> if you want to
+supply an explicit query list instead). This can only be
used if L<nbd_set_opt_mode(3)> enabled option mode.
To determine when the request completes, wait for
@@ -2614,7 +2671,35 @@ "aio_opt_list_meta_context", {
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_meta_context"];
+ see_also = [Link "set_opt_mode"; Link
"opt_list_meta_context";
+ Link "aio_opt_list_meta_context_queries"];
+ };
+
+ "aio_opt_list_meta_context_queries", {
+ default_call with
+ args = [ StringList "queries"; Closure context_closure ]; ret =
RInt;
+ optargs = [ OClosure completion_closure ];
+ permitted_states = [ Negotiating ];
+ shortdesc = "request list of available meta contexts, using explicit
query";
+ longdesc = "\
+Request that the server list available meta contexts associated with
+the export previously specified by the most recent
+L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>, and with an
+explicit list of queries provided as a parameter (see
+L<nbd_aio_opt_list_meta_context(3)> if you want to reuse an
+implicit query list instead). 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_meta_context_queries";
+ Link "aio_opt_list_meta_context"];
};
"aio_pread", {
@@ -3347,6 +3432,8 @@ let first_version "stats_chunks_sent", (1,
16);
"stats_bytes_received", (1, 16);
"stats_chunks_received", (1, 16);
+ "opt_list_meta_context_queries", (1, 16);
+ "aio_opt_list_meta_context_queries", (1, 16);
(* 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-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 31e84f35..1eb9fbe7 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -61,14 +61,14 @@ NEWSTYLE.OPT_META_CONTEXT.START:
SET_NEXT_STATE (%^OPT_GO.START);
return 0;
}
+ if (nbd_internal_set_querylist (h, NULL) == -1) {
+ SET_NEXT_STATE (%.DEAD);
+ return 0;
+ }
}
assert (!h->meta_valid);
- if (nbd_internal_set_querylist (h, NULL) == -1) {
- SET_NEXT_STATE (%.DEAD);
- return 0;
- }
/* Calculate the length of the option request data. */
len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
for (i = 0; i < h->querylist.len; ++i)
diff --git a/lib/opt.c b/lib/opt.c
index d9114f4b..cfdabb9e 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -197,12 +197,21 @@ context_complete (void *opaque, int *err)
int
nbd_unlocked_opt_list_meta_context (struct nbd_handle *h,
nbd_context_callback *context)
+{
+ return nbd_unlocked_opt_list_meta_context_queries (h, NULL, context);
+}
+
+/* Issue NBD_OPT_LIST_META_CONTEXT and wait for the reply. */
+int
+nbd_unlocked_opt_list_meta_context_queries (struct nbd_handle *h,
+ char **queries,
+ nbd_context_callback *context)
{
struct context_helper s = { .context = *context };
nbd_context_callback l = { .callback = context_visitor, .user_data = &s
};
nbd_completion_callback c = { .callback = context_complete, .user_data =
&s };
- if (nbd_unlocked_aio_opt_list_meta_context (h, &l, &c) == -1)
+ if (nbd_unlocked_aio_opt_list_meta_context_queries (h, queries, &l,
&c) == -1)
return -1;
SET_CALLBACK_TO_NULL (*context);
@@ -285,12 +294,26 @@ int
nbd_unlocked_aio_opt_list_meta_context (struct nbd_handle *h,
nbd_context_callback *context,
nbd_completion_callback *complete)
+{
+ return nbd_unlocked_aio_opt_list_meta_context_queries (h, NULL, context,
+ complete);
+}
+
+/* Issue NBD_OPT_LIST_META_CONTEXT without waiting. */
+int
+nbd_unlocked_aio_opt_list_meta_context_queries (struct nbd_handle *h,
+ char **queries,
+ nbd_context_callback *context,
+ 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;
}
+ if (nbd_internal_set_querylist (h, queries) == -1)
+ return -1;
+
assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
h->opt_cb.fn.context = *context;
SET_CALLBACK_TO_NULL (*context);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f6a2c110..dfb7f8bd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -214,6 +214,7 @@ check_PROGRAMS += \
opt-list \
opt-info \
opt-list-meta \
+ opt-list-meta-queries \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -280,6 +281,7 @@ TESTS += \
opt-list \
opt-info \
opt-list-meta \
+ opt-list-meta-queries \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -538,6 +540,9 @@ opt_info_LDADD = $(top_builddir)/lib/libnbd.la
opt_list_meta_SOURCES = opt-list-meta.c
opt_list_meta_LDADD = $(top_builddir)/lib/libnbd.la
+opt_list_meta_queries_SOURCES = opt-list-meta-queries.c
+opt_list_meta_queries_LDADD = $(top_builddir)/lib/libnbd.la
+
connect_systemd_socket_activation_SOURCES = \
connect-systemd-socket-activation.c \
requires.c \
diff --git a/tests/opt-list-meta-queries.c b/tests/opt-list-meta-queries.c
new file mode 100644
index 00000000..8570f967
--- /dev/null
+++ b/tests/opt-list-meta-queries.c
@@ -0,0 +1,145 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2022 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Test behavior of nbd_opt_list_meta_context_queries. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <inttypes.h>
+#include <string.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+struct progress {
+ int count;
+ bool seen;
+};
+
+static int
+check (void *user_data, const char *name)
+{
+ struct progress *p = user_data;
+
+ p->count++;
+ if (strcmp (name, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0)
+ p->seen = true;
+ return 0;
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int r;
+ struct progress p;
+ char *args[] = { "nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M", NULL };
+ nbd_context_callback ctx = { .callback = check,
+ .user_data = &p};
+
+ /* 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);
+ }
+
+ /* A NULL query is an error (C-only test) */
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_list_meta_context_queries (nbd, NULL, ctx);
+ if (r != -1 || nbd_get_errno () != EFAULT) {
+ fprintf (stderr, "expected EFAULT for NULL query list\n");
+ exit (EXIT_FAILURE);
+ }
+ if (p.count != 0 || p.seen) {
+ fprintf (stderr, "unexpected use of callback on failure\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* First pass: empty query should give at least "base:allocation".
+ * The explicit query overrides a non-empty nbd_add_meta_context.
+ */
+ p = (struct progress) { .count = 0 };
+ if (nbd_add_meta_context (nbd, "x-nosuch:") == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ {
+ char *empty[] = { NULL };
+ r = nbd_opt_list_meta_context_queries (nbd, empty, ctx);
+ }
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != p.count) {
+ fprintf (stderr, "inconsistent return value %d, expected %d\n",
r, p.count);
+ exit (EXIT_FAILURE);
+ }
+ if (r < 1 || !p.seen) {
+ fprintf (stderr, "server did not reply with base:allocation\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Second pass: bogus query has no response. */
+ p = (struct progress) { .count = 0 };
+ r = nbd_clear_meta_contexts (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ {
+ char *nosuch[] = { "x-nosuch:", NULL };
+ r = nbd_opt_list_meta_context_queries (nbd, nosuch, ctx);
+ }
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 0 || p.count != 0 || p.seen) {
+ fprintf (stderr, "expecting no contexts, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Third pass: specific query should have one match. */
+ p = (struct progress) { .count = 0 };
+ {
+ char *pair[] = { "x-nosuch:", LIBNBD_CONTEXT_BASE_ALLOCATION,
NULL };
+ r = nbd_opt_list_meta_context_queries (nbd, pair, ctx);
+ }
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1 || p.count != 1 || !p.seen) {
+ fprintf (stderr, "expecting exactly one context, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_opt_abort (nbd);
+ nbd_close (nbd);
+
+ exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index bc49860d..272d1ec1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -233,6 +233,7 @@ Makefile.in
/tests/opt-info
/tests/opt-list
/tests/opt-list-meta
+/tests/opt-list-meta-queries
/tests/pki/
/tests/pread-initialize
/tests/private-data
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 07/18] tests: Language port of nbd_opt_list_meta_context_queries()
Port the C test of the previous commit over to other language
bindings, to prove that we are handling StringList translations
correctly. Golang is particularly annoying with all tests sharing the
same namespace for globals, requiring some renaming in test 240 in
relation to the code added in test 245.
---
python/t/245-opt-list-meta-queries.py | 68 +++++++++++
ocaml/tests/Makefile.am | 1 +
ocaml/tests/test_245_opt_list_meta_queries.ml | 68 +++++++++++
tests/opt-list-meta-queries.c | 1 +
golang/Makefile.am | 1 +
golang/libnbd_240_opt_list_meta_test.go | 40 +++---
.../libnbd_245_opt_list_meta_queries_test.go | 115 ++++++++++++++++++
7 files changed, 274 insertions(+), 20 deletions(-)
create mode 100644 python/t/245-opt-list-meta-queries.py
create mode 100644 ocaml/tests/test_245_opt_list_meta_queries.ml
create mode 100644 golang/libnbd_245_opt_list_meta_queries_test.go
diff --git a/python/t/245-opt-list-meta-queries.py
b/python/t/245-opt-list-meta-queries.py
new file mode 100644
index 00000000..561997ed
--- /dev/null
+++ b/python/t/245-opt-list-meta-queries.py
@@ -0,0 +1,68 @@
+# libnbd Python bindings
+# Copyright (C) 2010-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+import nbd
+
+
+count = 0
+seen = False
+
+
+def f(user_data, name):
+ global count
+ global seen
+ assert user_data == 42
+ count = count + 1
+ if name == nbd.CONTEXT_BASE_ALLOCATION:
+ seen = True
+
+
+# Get into negotiating state.
+h = nbd.NBD()
+h.set_opt_mode(True)
+h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M"])
+
+# First pass: empty query should give at least "base:allocation".
+# The explicit query overrides a non-empty nbd_add_meta_context.
+count = 0
+seen = False
+h.add_meta_context("x-nosuch:")
+r = h.opt_list_meta_context_queries([], lambda *args: f(42, *args))
+assert r == count
+assert r >= 1
+assert seen is True
+
+# Second pass: bogus query has no response.
+count = 0
+seen = False
+h.clear_meta_contexts()
+r = h.opt_list_meta_context_queries(["x-nosuch:"], lambda *args:
f(42, *args))
+assert r == 0
+assert r == count
+assert seen is False
+
+# Third pass: specific query should have one match.
+count = 0
+seen = False
+r = h.opt_list_meta_context_queries(["x-nosuch:",
nbd.CONTEXT_BASE_ALLOCATION],
+ lambda *args: f(42, *args))
+assert r == 1
+assert count == 1
+assert seen is True
+
+h.opt_abort()
diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am
index c4751ad3..1b4d1135 100644
--- a/ocaml/tests/Makefile.am
+++ b/ocaml/tests/Makefile.am
@@ -31,6 +31,7 @@ ML_TESTS = \
test_220_opt_list.ml \
test_230_opt_info.ml \
test_240_opt_list_meta.ml \
+ test_245_opt_list_meta_queries.ml \
test_300_get_size.ml \
test_400_pread.ml \
test_405_pread_structured.ml \
diff --git a/ocaml/tests/test_245_opt_list_meta_queries.ml
b/ocaml/tests/test_245_opt_list_meta_queries.ml
new file mode 100644
index 00000000..4c5e01ef
--- /dev/null
+++ b/ocaml/tests/test_245_opt_list_meta_queries.ml
@@ -0,0 +1,68 @@
+(* hey emacs, this is OCaml code: -*- tuareg -*- *)
+(* libnbd OCaml test case
+ * Copyright (C) 2013-2022 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *)
+
+open Printf
+
+let count = ref 0
+let seen = ref false
+let f user_data name + assert (user_data = 42);
+ count := !count + 1;
+ if name = NBD.context_base_allocation then
+ seen := true;
+ 0
+
+let () + (* Get into negotiating state. *)
+ let nbd = NBD.create () in
+ NBD.set_opt_mode nbd true;
+ NBD.connect_command nbd
+ ["nbdkit"; "-s";
"--exit-with-parent"; "-v";
+ "memory"; "size=1M"];
+
+ (* First pass: empty query should give at least "base:allocation".
*)
+ count := 0;
+ seen := false;
+ NBD.add_meta_context nbd "x-nosuch:";
+ let r = NBD.opt_list_meta_context_queries nbd [] (f 42) in
+ assert (r = !count);
+ assert (r >= 1);
+ assert !seen;
+
+ (* Second pass: bogus query has no response. *)
+ count := 0;
+ seen := false;
+ NBD.clear_meta_contexts nbd;
+ let r = NBD.opt_list_meta_context_queries nbd ["x-nosuch:"] (f 42)
in
+ assert (r = 0);
+ assert (r = !count);
+ assert (not !seen);
+
+ (* Third pass: specific query should have one match. *)
+ count := 0;
+ seen := false;
+ let r = NBD.opt_list_meta_context_queries nbd [
+ "x-nosuch:"; NBD.context_base_allocation ] (f 42) in
+ assert (r = 1);
+ assert (r = !count);
+ assert !seen;
+
+ NBD.opt_abort nbd
+
+let () = Gc.compact ()
diff --git a/tests/opt-list-meta-queries.c b/tests/opt-list-meta-queries.c
index 8570f967..3075e905 100644
--- a/tests/opt-list-meta-queries.c
+++ b/tests/opt-list-meta-queries.c
@@ -17,6 +17,7 @@
*/
/* Test behavior of nbd_opt_list_meta_context_queries. */
+/* See also unit test 245 in the various language ports. */
#include <config.h>
diff --git a/golang/Makefile.am b/golang/Makefile.am
index 4e49a9a1..6adf6ae8 100644
--- a/golang/Makefile.am
+++ b/golang/Makefile.am
@@ -35,6 +35,7 @@ source_files = \
libnbd_220_opt_list_test.go \
libnbd_230_opt_info_test.go \
libnbd_240_opt_list_meta_test.go \
+ libnbd_245_opt_list_meta_queries_test.go \
libnbd_300_get_size_test.go \
libnbd_400_pread_test.go \
libnbd_405_pread_structured_test.go \
diff --git a/golang/libnbd_240_opt_list_meta_test.go
b/golang/libnbd_240_opt_list_meta_test.go
index 2b49c895..c329aaf6 100644
--- a/golang/libnbd_240_opt_list_meta_test.go
+++ b/golang/libnbd_240_opt_list_meta_test.go
@@ -23,16 +23,16 @@
"testing"
)
-var count uint
-var seen bool
+var list_count uint
+var list_seen bool
func listmetaf(user_data int, name string) int {
if user_data != 42 {
panic("expected user_data == 42")
}
- count++
+ list_count++
if (name == context_base_allocation) {
- seen = true
+ list_seen = true
}
return 0
}
@@ -59,22 +59,22 @@ func Test240OptListMeta(t *testing.T) {
}
/* First pass: empty query should give at least "base:allocation".
*/
- count = 0
- seen = false
+ list_count = 0
+ list_seen = false
r, err := h.OptListMetaContext(func(name string) int {
return listmetaf(42, name)
})
if err != nil {
t.Fatalf("could not request opt_list_meta_context: %s", err)
}
- if r != count || r < 1 || !seen {
+ if r != list_count || r < 1 || !list_seen {
t.Fatalf("unexpected count after opt_list_meta_context")
}
- max := count
+ max := list_count
/* Second pass: bogus query has no response. */
- count = 0
- seen = false
+ list_count = 0
+ list_seen = false
err = h.AddMetaContext("x-nosuch:")
if err != nil {
t.Fatalf("could not request add_meta_context: %s", err)
@@ -85,13 +85,13 @@ func Test240OptListMeta(t *testing.T) {
if err != nil {
t.Fatalf("could not request opt_list_meta_context: %s", err)
}
- if r != 0 || r != count || seen {
+ if r != 0 || r != list_count || list_seen {
t.Fatalf("unexpected count after opt_list_meta_context")
}
/* Third pass: specific query should have one match. */
- count = 0
- seen = false
+ list_count = 0
+ list_seen = false
err = h.AddMetaContext(context_base_allocation)
if err != nil {
t.Fatalf("could not request add_meta_context: %s", err)
@@ -116,13 +116,13 @@ func Test240OptListMeta(t *testing.T) {
if err != nil {
t.Fatalf("could not request opt_list_meta_context: %s", err)
}
- if r != 1 || r != count || !seen {
+ if r != 1 || r != list_count || !list_seen {
t.Fatalf("unexpected count after opt_list_meta_context")
}
/* Final pass: "base:" query should get at least
"base:allocation" */
- count = 0
- seen = false
+ list_count = 0
+ list_seen = false
err = h.ClearMetaContexts()
if err != nil {
t.Fatalf("could not request clear_meta_contexts: %s", err)
@@ -137,7 +137,7 @@ func Test240OptListMeta(t *testing.T) {
if err != nil {
t.Fatalf("could not request opt_list_meta_context: %s", err)
}
- if r < 1 || r > max || r != count || !seen {
+ if r < 1 || r > max || r != list_count || !list_seen {
t.Fatalf("unexpected count after opt_list_meta_context")
}
@@ -178,8 +178,8 @@ func Test240OptListMeta(t *testing.T) {
t.Fatalf("could not collect stats: %s", err)
}
- count = 0
- seen = false
+ list_count = 0
+ list_seen = false
r, err = h.OptListMetaContext(func(name string) int {
return listmetaf(42, name)
})
@@ -192,7 +192,7 @@ func Test240OptListMeta(t *testing.T) {
t.Fatalf("unexpected bytes sent after opt_list_meta_context")
}
fmt.Printf("ignoring failure from old server: %s", err)
- } else if r < 1 || r != count || !seen {
+ } else if r < 1 || r != list_count || !list_seen {
t.Fatalf("unexpected count after opt_list_meta_context")
}
diff --git a/golang/libnbd_245_opt_list_meta_queries_test.go
b/golang/libnbd_245_opt_list_meta_queries_test.go
new file mode 100644
index 00000000..1a1f0bfb
--- /dev/null
+++ b/golang/libnbd_245_opt_list_meta_queries_test.go
@@ -0,0 +1,115 @@
+/* libnbd golang tests
+ * Copyright (C) 2013-2022 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
+ */
+
+package libnbd
+
+import "testing"
+
+var listq_count uint
+var listq_seen bool
+
+func listmetaqf(user_data int, name string) int {
+ if user_data != 42 {
+ panic("expected user_data == 42")
+ }
+ listq_count++
+ if (name == context_base_allocation) {
+ listq_seen = true
+ }
+ return 0
+}
+
+func Test245OptListMetaQueries(t *testing.T) {
+ /* Get into negotiating state. */
+ h, err := Create()
+ if err != nil {
+ t.Fatalf("could not create handle: %s", err)
+ }
+ defer h.Close()
+
+ err = h.SetOptMode(true)
+ if err != nil {
+ t.Fatalf("could not set opt mode: %s", err)
+ }
+
+ err = h.ConnectCommand([]string{
+ "nbdkit", "-s", "--exit-with-parent",
"-v",
+ "memory", "size=1M",
+ })
+ if err != nil {
+ t.Fatalf("could not connect: %s", err)
+ }
+
+ /* First pass: empty query should give at least "base:allocation".
+ * The explicit query overrides a non-empty nbd_add_meta_context.
+ */
+ listq_count = 0
+ listq_seen = false
+ err = h.AddMetaContext("x-nosuch:")
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ r, err := h.OptListMetaContextQueries([]string{ },
+ func(name string) int {
+ return listmetaqf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_list_meta_context_queries: %s",
err)
+ }
+ if r != listq_count || r < 1 || !listq_seen {
+ t.Fatalf("unexpected count after opt_list_meta_context_queries")
+ }
+
+ /* Second pass: bogus query has no response. */
+ listq_count = 0
+ listq_seen = false
+ err = h.ClearMetaContexts()
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ r, err = h.OptListMetaContextQueries([]string{ "x-nosuch:" },
+ func(name string) int {
+ return listmetaqf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_list_meta_context_queries: %s",
err)
+ }
+ if r != 0 || r != listq_count || listq_seen {
+ t.Fatalf("unexpected count after opt_list_meta_context_queries")
+ }
+
+ /* Third pass: specific query should have one match. */
+ listq_count = 0
+ listq_seen = false
+ r, err = h.OptListMetaContextQueries([]string{
+ "x-nosuch:", context_base_allocation },
+ func(name string) int {
+ return listmetaqf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_list_meta_context_queries: %s",
err)
+ }
+ if r != 1 || r != listq_count || !listq_seen {
+ t.Fatalf("unexpected count after opt_list_meta_context_queries")
+ }
+
+ err = h.OptAbort()
+ if err != nil {
+ t.Fatalf("could not request opt_abort: %s", err)
+ }
+}
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 08/18] api: Add nbd_set_request_meta_context()
Add a new control knob nbd_set_request_meta_context(), modeled after
the existing nbd_set_request_structured_replies(), to make it possible
to skip the NBD_OPT_SET_META_CONTEXT half of the two-command sequence
currently performed in nbd_opt_go() and nbd_opt_info(). Also add a
counterpart nbd_get_request_meta_context() for symmetry.
A later patch will then add the ability for the user to manually
invoke nbd_opt_set_meta_context() at a time of their choosing during
option negotiation; but even without that patch, this new API has some
demonstrable effects by itself:
- skipping meta contexts but not structured replies lets us test
additional corner cases of servers (for example, while trying to
write my unit tests, I quickly found out that with structured
replies negotiated, current nbdkit ALWAYS emulates and advertises
the base:allocation context rather than consulting .can_extents as a
way to suppress it on a per-export basis, even when a corresponding
.open would fail. A future nbdkit may make .can_extents tri-state to
make it possible to do structured reads but not extents; however,
the current nbdkit behavior appears to comply with the NBD spec,
which allows but does not require NBD_OPT_SET_META_CONTEXT to pay
attention to the export name)
- back-to-back nbd_opt_info() and nbd_opt_go() was performing a
redundant SET_META_CONTEXT; with this new API, we can get by with
less network traffic during negotiation. Most clients don't attempt
this (nbd_opt_info is used by 'nbdinfo --list', but wasn't using
nbd_add_meta_context(); most other clients don't use nbd_opt_info)
- nbd_opt_info() has to be client-side stateful (you check things like
nbd_get_size after the fact), but based on the name, the fact that
it was also server-side stateful was surprising. Skipping
SET_META_CONTEXT during nbd_opt_info(), and instead using
stateless[1] nbd_opt_list_meta_context(), avoids messing with server
state and can also be more convenient in getting supported context
names by callback instead of lots of post-process
nbd_can_meta_context() calls
Things to note in the patch: the choice of when to change
h->meta_valid is moved around. Marking contexts invalid is no longer
a side effect of clearing h->exportsize (that is, once
set_request_meta_context is false, nbd_opt_go() and nbd_opt_info()
should inherit what contexts are previously negotiated); it is now an
explicit action when changing the export name[2], starting an actual
NBD_OPT_SET_META_CONTEXT request, or upon failure of NBD_OPT_GO/INFO.
Likewise, the action of copying the implicit list of contexts during
NBD_OPT_SET_META_CONTEXT is relocated into a code block guarded by opt
!= h->current_opt; per the comments on this function, this is
currently only for nbd_opt_list_meta_context[_queries], but it sets
the stage for adding nbd_opt_set_meta_context[_queries] in a coming
patch.
The testsuite changes added here depend on the new API; therefore,
there is no benefit to separating the C change to a separate patch (if
split and you rearranged the series, it would fail to compile).
However, for ease of review, porting the test to its counterparts in
other languages is split out.
[1] nbd_opt_list_meta_context() is slightly improved here, but has
other client-side state effects that are left for a later patch to
minimize the size of this one
[2] nbd_set_export_name() should also reset size, but I'm leaving that
for a later patch to minimize this one
---
lib/internal.h | 1 +
generator/API.ml | 69 ++++++++++++++++++--
generator/states-newstyle-opt-go.c | 1 +
generator/states-newstyle-opt-meta-context.c | 25 +++----
lib/flags.c | 4 +-
lib/handle.c | 17 +++++
tests/opt-info.c | 68 +++++++++++++++++--
7 files changed, 161 insertions(+), 24 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 308e65ef..3c9a2c10 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -111,6 +111,7 @@ struct nbd_handle {
/* Desired metadata contexts. */
bool request_sr;
+ bool request_meta;
string_vector request_meta_contexts;
/* Allowed in URIs, see lib/uri.c. */
diff --git a/generator/API.ml b/generator/API.ml
index 0c72c356..0f70dcd1 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -830,6 +830,46 @@ "get_structured_replies_negotiated", {
Link "get_protocol"];
};
+ "set_request_meta_context", {
+ default_call with
+ args = [Bool "request"]; ret = RErr;
+ permitted_states = [ Created; Negotiating ];
+ shortdesc = "control whether connect automatically requests meta
contexts";
+ longdesc = "\
+This function controls whether the act of connecting to an export
+(all C<nbd_connect_*> calls when L<nbd_set_opt_mode(3)> is false,
+or L<nbd_opt_go(3)> and L<nbd_opt_info(3)> when option mode is
+enabled) will also try to issue NBD_OPT_SET_META_CONTEXT when
+the server supports structured replies and any contexts were
+registered by L<nbd_add_meta_context(3)>. The default setting
+is true; however the extra step of negotiating meta contexts is
+not always desirable: performing both info and go on the same
+export works without needing to re-negotiate contexts on the
+second call; and even when using just L<nbd_opt_info(3)>, it
+can be faster to collect the server's results by relying on the
+callback function passed to L<nbd_opt_list_meta_context(3)> than
+a series of post-process calls to L<nbd_can_meta_context(3)>.
+
+Note that this control has no effect if the server does not
+negotiate structured replies, or if the client did not request
+any contexts via L<nbd_add_meta_context(3)>. Setting this
+control to false may cause L<nbd_block_status(3)> to fail.";
+ see_also = [Link "set_opt_mode"; Link "opt_go"; Link
"opt_info";
+ Link "opt_list_meta_context";
+ Link "get_structured_replies_negotiated";
+ Link "get_request_meta_context"; Link
"can_meta_context"];
+ };
+
+ "get_request_meta_context", {
+ default_call with
+ args = []; ret = RBool;
+ permitted_states = [];
+ shortdesc = "see if connect automatically requests meta
contexts";
+ longdesc = "\
+Return the state of the automatic meta context request flag on this
handle.";
+ see_also = [Link "set_request_meta_context"];
+ };
+
"set_handshake_flags", {
default_call with
args = [ Flags ("flags", handshake_flags) ]; ret = RErr;
@@ -1079,12 +1119,17 @@ "opt_go", {
or L<nbd_connect_uri(3)>. This can only be used if
L<nbd_set_opt_mode(3)> enabled option mode.
+By default, libnbd will automatically request all meta contexts
+registered by L<nbd_add_meta_context(3)> as part of this call; but
+this can be suppressed with L<nbd_set_request_meta_context(3)>.
+
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"; Link "connect_uri";
Link "opt_info"];
+ Link "set_export_name"; Link "connect_uri";
Link "opt_info";
+ Link "add_meta_context"; Link
"set_request_meta_context"];
};
"opt_abort", {
@@ -1153,16 +1198,23 @@ "opt_info", {
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.
+L<nbd_get_size(3)> will report details about that export. If
+L<nbd_set_request_meta_context(3)> is set (the default) and
+structured replies were negotiated, it is also valid to use
+L<nbd_can_meta_context(3)> after this call. However, it may be
+more efficient to clear that setting and manually utilize
+L<nbd_opt_list_meta_context(3)> with its callback approach, for
+learning which contexts an export supports. 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"];
+ Link "set_export_name"; Link
"set_request_meta_context";
+ Link "opt_list_meta_context"];
};
"opt_list_meta_context", {
@@ -1946,7 +1998,8 @@ "can_meta_context", {
^ non_blocking_test_call_description;
see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "add_meta_context";
- Link "block_status"; Link
"aio_block_status"];
+ Link "block_status"; Link
"aio_block_status";
+ Link "set_request_meta_context"];
};
"get_protocol", {
@@ -3434,6 +3487,8 @@ let first_version "stats_chunks_received",
(1, 16);
"opt_list_meta_context_queries", (1, 16);
"aio_opt_list_meta_context_queries", (1, 16);
+ "set_request_meta_context", (1, 16);
+ "get_request_meta_context", (1, 16);
(* 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 4c6e9900..3c29137d 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -270,6 +270,7 @@ NEWSTYLE.OPT_GO.CHECK_REPLY:
reply);
}
nbd_internal_reset_size_and_flags (h);
+ h->meta_valid = false;
err = nbd_get_errno () ? : ENOTSUP;
break;
case NBD_REP_ACK:
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 1eb9fbe7..5dc7e9ad 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -35,20 +35,16 @@ NEWSTYLE.OPT_META_CONTEXT.START:
* nbd_opt_list_meta_context()
* -> unconditionally use LIST, next state NEGOTIATING
*
- * For now, we start by unconditionally clearing h->exportsize and
friends,
- * as well as h->meta_contexts and h->meta_valid.
- * If SET is conditional, we skip it if structured replies were
- * not negotiated, or if there were no contexts to request.
+ * For now, we start by unconditionally clearing h->exportsize and
friends.
+ * If SET is conditional, we skip it if h->request_meta is false, if
+ * structured replies were not negotiated, or if no contexts to request.
* SET then manipulates h->meta_contexts, and sets h->meta_valid on
success.
* If OPT_GO is later successful, it populates h->exportsize and friends,
- * and also sets h->meta_valid if we skipped SET here.
+ * and also sets h->meta_valid if h->request_meta but we skipped SET
here.
* There is a callback if and only if the command is LIST.
*/
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
nbd_internal_reset_size_and_flags (h);
- for (i = 0; i < h->meta_contexts.len; ++i)
- free (h->meta_contexts.ptr[i].name);
- meta_vector_reset (&h->meta_contexts);
if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) {
assert (h->opt_mode);
assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context));
@@ -57,7 +53,16 @@ NEWSTYLE.OPT_META_CONTEXT.START:
else {
assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
opt = NBD_OPT_SET_META_CONTEXT;
- if (!h->structured_replies || h->request_meta_contexts.len == 0) {
+ if (h->request_meta) {
+ for (i = 0; i < h->meta_contexts.len; ++i)
+ free (h->meta_contexts.ptr[i].name);
+ meta_vector_reset (&h->meta_contexts);
+ h->meta_valid = false;
+ }
+ }
+ if (opt != h->opt_current) {
+ if (!h->request_meta || !h->structured_replies ||
+ h->request_meta_contexts.len == 0) {
SET_NEXT_STATE (%^OPT_GO.START);
return 0;
}
@@ -67,8 +72,6 @@ NEWSTYLE.OPT_META_CONTEXT.START:
}
}
- assert (!h->meta_valid);
-
/* Calculate the length of the option request data. */
len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
for (i = 0; i < h->querylist.len; ++i)
diff --git a/lib/flags.c b/lib/flags.c
index 9619f235..03a1b12d 100644
--- a/lib/flags.c
+++ b/lib/flags.c
@@ -35,7 +35,6 @@ nbd_internal_reset_size_and_flags (struct nbd_handle *h)
{
h->exportsize = 0;
h->eflags = 0;
- h->meta_valid = false;
h->block_minimum = 0;
h->block_preferred = 0;
h->block_maximum = 0;
@@ -71,7 +70,8 @@ nbd_internal_set_size_and_flags (struct nbd_handle *h,
eflags &= ~NBD_FLAG_SEND_FAST_ZERO;
}
- if (!h->structured_replies || h->request_meta_contexts.len == 0) {
+ if (h->request_meta &&
+ (!h->structured_replies || h->request_meta_contexts.len == 0)) {
assert (h->meta_contexts.len == 0);
h->meta_valid = true;
}
diff --git a/lib/handle.c b/lib/handle.c
index 6f262ba3..e59e6160 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -70,6 +70,7 @@ nbd_create (void)
h->unique = 1;
h->tls_verify_peer = true;
h->request_sr = true;
+ h->request_meta = true;
h->request_block_size = true;
h->pread_initialize = true;
@@ -239,6 +240,7 @@ nbd_unlocked_set_export_name (struct nbd_handle *h, const
char *export_name)
free (h->export_name);
h->export_name = new_name;
+ h->meta_valid = false;
return 0;
}
@@ -398,6 +400,21 @@ nbd_unlocked_get_request_structured_replies (struct
nbd_handle *h)
return h->request_sr;
}
+int
+nbd_unlocked_set_request_meta_context (struct nbd_handle *h,
+ bool request)
+{
+ h->request_meta = request;
+ return 0;
+}
+
+/* NB: may_set_error = false. */
+int
+nbd_unlocked_get_request_meta_context (struct nbd_handle *h)
+{
+ return h->request_meta;
+}
+
int
nbd_unlocked_get_structured_replies_negotiated (struct nbd_handle *h)
{
diff --git a/tests/opt-info.c b/tests/opt-info.c
index b9739a5f..cf423d5c 100644
--- a/tests/opt-info.c
+++ b/tests/opt-info.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 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
@@ -102,11 +102,15 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- /* info for a different export */
+ /* info for a different export, with automatic meta_context disabled */
if (nbd_set_export_name (nbd, "b") == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
+ if (nbd_set_request_meta_context (nbd, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
if (nbd_opt_info (nbd) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
@@ -119,8 +123,12 @@ main (int argc, char *argv[])
fprintf (stderr, "expecting read-write export, got %" PRId64
"\n", r);
exit (EXIT_FAILURE);
}
- if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) {
- fprintf (stderr, "expecting can_meta_context true, got %" PRId64
"\n", r);
+ if (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) != -1) {
+ fprintf (stderr, "expecting error for can_meta_context\n");
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_set_request_meta_context (nbd, 1) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
@@ -189,8 +197,60 @@ main (int argc, char *argv[])
fprintf (stderr, "expecting size of 4, got %" PRId64
"\n", r);
exit (EXIT_FAILURE);
}
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) {
+ fprintf (stderr, "expecting can_meta_context true, got %" PRId64
"\n", r);
+ exit (EXIT_FAILURE);
+ }
nbd_shutdown (nbd, 0);
nbd_close (nbd);
+
+ /* Another connection. This time, check that SET_META triggered by opt_info
+ * persists through nbd_opt_go with set_request_meta_context disabled.
+ */
+ nbd = nbd_create ();
+ if (nbd == NULL ||
+ nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_connect_command (nbd, args) == -1 ||
+ nbd_add_meta_context (nbd, "x-unexpected:bogus") == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ if (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) != -1) {
+ fprintf (stderr, "expecting error for can_meta_context\n");
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_opt_info (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "expecting can_meta_context false, got %" PRId64
"\n", r);
+
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_set_request_meta_context (nbd, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ /* Adding to the request list now won't matter */
+ if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) != 0) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_opt_go (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "expecting can_meta_context false, got %" PRId64
"\n", r);
+
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_shutdown (nbd, 0);
+ nbd_close (nbd);
+
exit (EXIT_SUCCESS);
}
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 09/18] tests: Language port of nbd_set_request_meta_context() tests
As promised in the previous patch, also test the new
nbd_set_request_meta_context() API in Python, OCaml, and Golang. No
direct C counterpart in tests/, but it's always good to check that our
language bindings are complete.
Acked-by: Laszlo Ersek <lersek at redhat.com>
---
python/t/110-defaults.py | 1 +
python/t/120-set-non-defaults.py | 2 +
python/t/230-opt-info.py | 25 +++++-
ocaml/tests/test_110_defaults.ml | 2 +
ocaml/tests/test_120_set_non_defaults.ml | 3 +
ocaml/tests/test_230_opt_info.ml | 32 +++++++-
golang/libnbd_110_defaults_test.go | 8 ++
golang/libnbd_120_set_non_defaults_test.go | 12 +++
golang/libnbd_230_opt_info_test.go | 88 ++++++++++++++++++++--
9 files changed, 161 insertions(+), 12 deletions(-)
diff --git a/python/t/110-defaults.py b/python/t/110-defaults.py
index 749c94f4..6b62c8a4 100644
--- a/python/t/110-defaults.py
+++ b/python/t/110-defaults.py
@@ -22,6 +22,7 @@
assert h.get_full_info() is False
assert h.get_tls() == nbd.TLS_DISABLE
assert h.get_request_structured_replies() is True
+assert h.get_request_meta_context() is True
assert h.get_request_block_size() is True
assert h.get_pread_initialize() is True
assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK
diff --git a/python/t/120-set-non-defaults.py b/python/t/120-set-non-defaults.py
index 61a4160c..8b48b4ad 100644
--- a/python/t/120-set-non-defaults.py
+++ b/python/t/120-set-non-defaults.py
@@ -33,6 +33,8 @@
assert h.get_tls() == nbd.TLS_ALLOW
h.set_request_structured_replies(False)
assert h.get_request_structured_replies() is False
+h.set_request_meta_context(False)
+assert h.get_request_meta_context() is False
h.set_request_block_size(False)
assert h.get_request_block_size() is False
h.set_pread_initialize(False)
diff --git a/python/t/230-opt-info.py b/python/t/230-opt-info.py
index 5e571376..c326bc30 100644
--- a/python/t/230-opt-info.py
+++ b/python/t/230-opt-info.py
@@ -52,12 +52,14 @@ def must_fail(f, *args, **kwds):
must_fail(h.is_read_only)
must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
-# info for a different export
+# info for a different export, with automatic meta_context disabled
h.set_export_name("b")
+h.set_request_meta_context(False)
h.opt_info()
assert h.get_size() == 1
assert h.is_read_only() is False
-assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
+h.set_request_meta_context(True)
# go on something not present
h.set_export_name("a")
@@ -78,5 +80,24 @@ def must_fail(f, *args, **kwds):
assert h.get_export_name() == "good"
must_fail(h.opt_info)
assert h.get_size() == 4
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+
+h.shutdown()
+
+# Another connection. This time, check that SET_META triggered by opt_info
+# persists through nbd_opt_go with set_request_meta_context disabled.
+h = nbd.NBD()
+h.set_opt_mode(True)
+h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v", "sh", script])
+h.add_meta_context("x-unexpected:bogus")
+
+must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
+h.opt_info()
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+h.set_request_meta_context(False)
+# Adding to the request list now won't matter
+h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+h.opt_go()
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
h.shutdown()
diff --git a/ocaml/tests/test_110_defaults.ml b/ocaml/tests/test_110_defaults.ml
index ccec9c6d..768ad636 100644
--- a/ocaml/tests/test_110_defaults.ml
+++ b/ocaml/tests/test_110_defaults.ml
@@ -28,6 +28,8 @@ let
assert (tls = NBD.TLS.DISABLE);
let sr = NBD.get_request_structured_replies nbd in
assert sr;
+ let meta = NBD.get_request_meta_context nbd in
+ assert meta;
let bs = NBD.get_request_block_size nbd in
assert bs;
let init = NBD.get_pread_initialize nbd in
diff --git a/ocaml/tests/test_120_set_non_defaults.ml
b/ocaml/tests/test_120_set_non_defaults.ml
index 092287e3..76ff82fb 100644
--- a/ocaml/tests/test_120_set_non_defaults.ml
+++ b/ocaml/tests/test_120_set_non_defaults.ml
@@ -42,6 +42,9 @@ let
NBD.set_request_structured_replies nbd false;
let sr = NBD.get_request_structured_replies nbd in
assert (not sr);
+ NBD.set_request_meta_context nbd false;
+ let meta = NBD.get_request_meta_context nbd in
+ assert (not meta);
NBD.set_request_block_size nbd false;
let bs = NBD.get_request_block_size nbd in
assert (not bs);
diff --git a/ocaml/tests/test_230_opt_info.ml b/ocaml/tests/test_230_opt_info.ml
index 693a6630..ec735ff1 100644
--- a/ocaml/tests/test_230_opt_info.ml
+++ b/ocaml/tests/test_230_opt_info.ml
@@ -69,15 +69,16 @@ let
fail_unary NBD.is_read_only nbd;
fail_binary NBD.can_meta_context nbd NBD.context_base_allocation;
- (* info for a different export *)
+ (* info for a different export, with automatic meta_context disabled *)
NBD.set_export_name nbd "b";
+ NBD.set_request_meta_context nbd false;
NBD.opt_info nbd;
let size = NBD.get_size nbd in
assert (size = 1L);
let ro = NBD.is_read_only nbd in
assert (not ro);
- let meta = NBD.can_meta_context nbd NBD.context_base_allocation in
- assert meta;
+ fail_binary NBD.can_meta_context nbd NBD.context_base_allocation;
+ NBD.set_request_meta_context nbd true;
(* go on something not present *)
NBD.set_export_name nbd "a";
@@ -103,6 +104,31 @@ let
fail_unary NBD.opt_info nbd;
let size = NBD.get_size nbd in
assert (size = 4L);
+ let meta = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert meta;
+
+ NBD.shutdown nbd;
+
+ (* Another connection. This time, check that SET_META triggered by opt_info
+ * persists through nbd_opt_go with set_request_meta_context disabled.
+ *)
+ let nbd = NBD.create () in
+ NBD.set_opt_mode nbd true;
+ NBD.connect_command nbd
+ ["nbdkit"; "-s";
"--exit-with-parent"; "-v";
+ "sh"; script];
+ NBD.add_meta_context nbd "x-unexpected:bogus";
+
+ fail_binary NBD.can_meta_context nbd NBD.context_base_allocation;
+ NBD.opt_info nbd;
+ let meta = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not meta);
+ NBD.set_request_meta_context nbd false;
+ (* Adding to the request list now won't matter *)
+ NBD.add_meta_context nbd NBD.context_base_allocation;
+ NBD.opt_go nbd;
+ let meta = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not meta);
NBD.shutdown nbd
diff --git a/golang/libnbd_110_defaults_test.go
b/golang/libnbd_110_defaults_test.go
index f56c9656..d7ad319c 100644
--- a/golang/libnbd_110_defaults_test.go
+++ b/golang/libnbd_110_defaults_test.go
@@ -59,6 +59,14 @@ func Test110Defaults(t *testing.T) {
t.Fatalf("unexpected structured replies state")
}
+ meta, err := h.GetRequestMetaContext()
+ if err != nil {
+ t.Fatalf("could not get meta context state: %s", err)
+ }
+ if meta != true {
+ t.Fatalf("unexpected meta context state")
+ }
+
bs, err := h.GetRequestBlockSize()
if err != nil {
t.Fatalf("could not get block size state: %s", err)
diff --git a/golang/libnbd_120_set_non_defaults_test.go
b/golang/libnbd_120_set_non_defaults_test.go
index a4c411d0..06bb29d5 100644
--- a/golang/libnbd_120_set_non_defaults_test.go
+++ b/golang/libnbd_120_set_non_defaults_test.go
@@ -93,6 +93,18 @@ func Test120SetNonDefaults(t *testing.T) {
t.Fatalf("unexpected structured replies state")
}
+ err = h.SetRequestMetaContext(false)
+ if err != nil {
+ t.Fatalf("could not set meta context state: %s", err)
+ }
+ meta, err := h.GetRequestMetaContext()
+ if err != nil {
+ t.Fatalf("could not get meta context state: %s", err)
+ }
+ if meta != false {
+ t.Fatalf("unexpected meta context state")
+ }
+
err = h.SetRequestBlockSize(false)
if err != nil {
t.Fatalf("could not set block size state: %s", err)
diff --git a/golang/libnbd_230_opt_info_test.go
b/golang/libnbd_230_opt_info_test.go
index 3dd231a7..bc4cadfb 100644
--- a/golang/libnbd_230_opt_info_test.go
+++ b/golang/libnbd_230_opt_info_test.go
@@ -1,5 +1,5 @@
/* libnbd golang tests
- * Copyright (C) 2013-2021 Red Hat Inc.
+ * Copyright (C) 2013-2022 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
@@ -113,11 +113,15 @@ func Test230OptInfo(t *testing.T) {
t.Fatalf("expected error")
}
- /* info for a different export */
+ /* info for a different export, with automatic meta_context disabled */
err = h.SetExportName("b")
if err != nil {
t.Fatalf("set export name failed unexpectedly: %s", err)
}
+ err = h.SetRequestMetaContext(false)
+ if err != nil {
+ t.Fatalf("set request meta context failed unexpectedly: %s", err)
+ }
err = h.OptInfo()
if err != nil {
t.Fatalf("opt_info failed unexpectedly: %s", err)
@@ -136,12 +140,13 @@ func Test230OptInfo(t *testing.T) {
if ro {
t.Fatalf("unexpected readonly")
}
- meta, err = h.CanMetaContext(context_base_allocation)
- if err != nil {
- t.Fatalf("can_meta failed unexpectedly: %s", err)
+ _, err = h.CanMetaContext(context_base_allocation)
+ if err == nil {
+ t.Fatalf("expected error")
}
- if !meta {
- t.Fatalf("unexpected meta context")
+ err = h.SetRequestMetaContext(true)
+ if err != nil {
+ t.Fatalf("set request meta context failed unexpectedly: %s", err)
}
/* go on something not present */
@@ -220,6 +225,75 @@ func Test230OptInfo(t *testing.T) {
if size != 4 {
t.Fatalf("unexpected size")
}
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("can_meta failed unexpectedly: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected meta context")
+ }
+
+ h.Shutdown(nil)
+
+ /* Another cnonection. This time, check that SET_META triggered by OptInfo
+ * persists through OptGo with SetRequestMetaContext disabled.
+ */
+ h, err = Create()
+ if err != nil {
+ t.Fatalf("could not create handle: %s", err)
+ }
+ defer h.Close()
+
+ err = h.SetOptMode(true)
+ if err != nil {
+ t.Fatalf("could not set opt mode: %s", err)
+ }
+ err = h.ConnectCommand([]string{
+ "nbdkit", "-s", "--exit-with-parent",
"-v", "sh", script,
+ })
+ if err != nil {
+ t.Fatalf("could not connect: %s", err)
+ }
+ err = h.AddMetaContext("x-unexpected:bogus")
+ if err != nil {
+ t.Fatalf("could not add meta context: %s", err)
+ }
+
+ _, err = h.CanMetaContext(context_base_allocation)
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+ err = h.OptInfo()
+ if err != nil {
+ t.Fatalf("opt_info failed unexpectedly: %s", err)
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("can_meta failed unexpectedly: %s", err)
+ }
+ if meta {
+ t.Fatalf("unexpected meta context")
+ }
+ err = h.SetRequestMetaContext(false)
+ if err != nil {
+ t.Fatalf("set request meta context failed unexpectedly: %s", err)
+ }
+ /* Adding to the request list now won't matter */
+ err = h.AddMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not add meta context: %s", err)
+ }
+ err = h.OptGo()
+ if err != nil {
+ t.Fatalf("opt_go failed unexpectedly: %s", err)
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("can_meta failed unexpectedly: %s", err)
+ }
+ if meta {
+ t.Fatalf("unexpected meta context")
+ }
h.Shutdown(nil)
}
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 10/18] info: Explicitly skip NBD_OPT_SET_META_CONTEXT in --list mode
When listing information about an export, we do not use
nbd_block_status() or nbd_can_meta_context(), because we were already
utilizing the callback to nbd_opt_list_meta_context() to collect
supported context names. In practice, because we also never called
nbd_add_meta_context() (important because nbd_opt_list_meta_context()
only outputs all possible contexts when passed an empty list on
input), we were not causing nbd_opt_info() to invoke
NBD_OPT_SET_META_CONTEXT (SET on an empty list selects nothing, so the
state machine short-circuits that step). However, now that we have
the API to express our intentions, we might as well be explicit that
we don't need the server to set any contexts, even if such a change
does not impact the amount of traffic sent over the wire in list mode.
Acked-by: Laszlo Ersek <lersek at redhat.com>
---
info/list.c | 3 ++-
info/show.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/info/list.c b/info/list.c
index 7646bf1c..c2741ba0 100644
--- a/info/list.c
+++ b/info/list.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2020-2021 Red Hat Inc.
+ * Copyright (C) 2020-2022 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
@@ -104,6 +104,7 @@ list_all_exports (void)
}
nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */
nbd_set_opt_mode (nbd2, true);
+ nbd_set_request_meta_context (nbd2, false);
nbd_set_full_info (nbd2, true);
do_connect (nbd2);
diff --git a/info/show.c b/info/show.c
index 79038b05..4bf59671 100644
--- a/info/show.c
+++ b/info/show.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2020-2021 Red Hat Inc.
+ * Copyright (C) 2020-2022 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
@@ -65,6 +65,7 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
* advertising something it later refuses to serve), return rather
* than exit, to allow output on the rest of the list.
*/
+ nbd_set_request_meta_context (nbd, false);
if (nbd_aio_is_negotiating (nbd) &&
nbd_opt_info (nbd) == -1 &&
nbd_opt_go (nbd) == -1) {
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 11/18] api: Make nbd_opt_list_meta_context stateless
Since NBD_OPT_LIST_META_CONTEXTS is stateless from the server's point
of view, there is no reason to make it clear state on the client's
side. A previous patch already fixed it to not wipe h->meta_contexts
for NBD_OPT_LIST_META_CONTEXTS (since that tracks the server state
affected only by NBD_OPT_SET_META_CONTEXTS); but we were still wiping
h->exportsize and friends. Better is to wipe size information only if
we actually enter NEWSTYLE.OPT_GO.START to attempt NBD_OPT_GO or
NBD_OPT_INFO. This change will also help when a future patch adds
nbd_opt_set_meta_context() as another API that wants to modify
h->meta_contexts but not h->exportsize.
Testsuite coverage for this is in the next patch, for ease of review
(that is, this is one case where it is easy to swap the patch order to
see a failure fixed by this patch).
---
generator/states-newstyle-opt-go.c | 1 +
generator/states-newstyle-opt-meta-context.c | 5 ++---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index 3c29137d..9881d0f2 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 = 0;
+ nbd_internal_reset_size_and_flags (h);
if (h->request_block_size)
nrinfos++;
if (h->full_info)
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 5dc7e9ad..e124d06f 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -35,16 +35,15 @@ NEWSTYLE.OPT_META_CONTEXT.START:
* nbd_opt_list_meta_context()
* -> unconditionally use LIST, next state NEGOTIATING
*
- * For now, we start by unconditionally clearing h->exportsize and
friends.
* If SET is conditional, we skip it if h->request_meta is false, if
* structured replies were not negotiated, or if no contexts to request.
- * SET then manipulates h->meta_contexts, and sets h->meta_valid on
success.
+ * SET then manipulates h->meta_contexts, and sets h->meta_valid on
+ * success, while LIST is stateless.
* If OPT_GO is later successful, it populates h->exportsize and friends,
* and also sets h->meta_valid if h->request_meta but we skipped SET
here.
* There is a callback if and only if the command is LIST.
*/
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
- nbd_internal_reset_size_and_flags (h);
if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) {
assert (h->opt_mode);
assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context));
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 12/18] tests: Add coverage for stateless nbd_opt_list_meta_context
Add test coverage for the previous patch not wiping state during
nbd_opt_list_meta_context. The change is logically identical in each
language that has the same unit test.
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
python/t/240-opt-list-meta.py | 27 ++++++++-
ocaml/tests/test_240_opt_list_meta.ml | 34 ++++++++++-
tests/opt-list-meta.c | 76 +++++++++++++++++++++++--
golang/libnbd_240_opt_list_meta_test.go | 62 +++++++++++++++++++-
4 files changed, 191 insertions(+), 8 deletions(-)
diff --git a/python/t/240-opt-list-meta.py b/python/t/240-opt-list-meta.py
index 50fcfd69..8cafdd54 100644
--- a/python/t/240-opt-list-meta.py
+++ b/python/t/240-opt-list-meta.py
@@ -31,6 +31,14 @@ def f(user_data, name):
seen = True
+def must_fail(f, *args, **kwds):
+ try:
+ f(*args, **kwds)
+ assert False
+ except nbd.Error:
+ pass
+
+
# Get into negotiating state.
h = nbd.NBD()
h.set_opt_mode(True)
@@ -66,10 +74,27 @@ def f(user_data, name):
assert count == 1
assert seen is True
-# Final pass: "base:" query should get at least
"base:allocation"
+# Fourth pass: opt_list_meta_context is stateless, so it should
+# not wipe status learned during opt_info
count = 0
seen = False
+must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
+must_fail(h.get_size)
+h.opt_info()
+assert h.get_size() == 1024*1024
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
h.clear_meta_contexts()
+h.add_meta_context("x-nosuch:")
+r = h.opt_list_meta_context(lambda *args: f(42, *args))
+assert r == 0
+assert count == 0
+assert seen is False
+assert h.get_size() == 1048576
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+
+# Final pass: "base:" query should get at least
"base:allocation"
+count = 0
+seen = False
h.add_meta_context("base:")
r = h.opt_list_meta_context(lambda *args: f(42, *args))
assert r >= 1
diff --git a/ocaml/tests/test_240_opt_list_meta.ml
b/ocaml/tests/test_240_opt_list_meta.ml
index 64159185..18582c44 100644
--- a/ocaml/tests/test_240_opt_list_meta.ml
+++ b/ocaml/tests/test_240_opt_list_meta.ml
@@ -67,10 +67,42 @@ let
assert (r = !count);
assert !seen;
- (* Final pass: "base:" query should get at least
"base:allocation" *)
+ (* Fourth pass: opt_list_meta_context is stateless, so it should
+ * not wipe status learned during opt_info
+ *)
count := 0;
seen := false;
+ (try
+ let _ = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
+ (try
+ let _ = NBD.get_size nbd in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
+ NBD.opt_info nbd;
+ let s = NBD.get_size nbd in
+ assert (s = 1048576_L);
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert m;
NBD.clear_meta_contexts nbd;
+ NBD.add_meta_context nbd "x-nosuch:";
+ let r = NBD.opt_list_meta_context nbd (f 42) in
+ assert (r = 0);
+ assert (r = !count);
+ assert (not !seen);
+ let s = NBD.get_size nbd in
+ assert (s = 1048576_L);
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert m;
+
+ (* Final pass: "base:" query should get at least
"base:allocation" *)
+ count := 0;
+ seen := false;
NBD.add_meta_context nbd "base:";
let r = NBD.opt_list_meta_context nbd (f 42) in
assert (r >= 1);
diff --git a/tests/opt-list-meta.c b/tests/opt-list-meta.c
index dc9c6799..05c1a5eb 100644
--- a/tests/opt-list-meta.c
+++ b/tests/opt-list-meta.c
@@ -139,13 +139,79 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
+ /* Fourth pass: nbd_opt_list_meta_context is stateless, so it should
+ * not wipe status learned during nbd_opt_info().
+ */
+ r = nbd_get_size (nbd);
+ if (r != -1) {
+ fprintf (stderr, "expecting get_size to fail, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+ r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION);
+ if (r != -1) {
+ fprintf (stderr, "expecting can_meta_context to fail, got %d\n",
r);
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_opt_info (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ r = nbd_get_size (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1024*1024) {
+ fprintf (stderr, "expecting get_size of 1M, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+ r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "expecting can_meta_context to succeed, got
%d\n", r);
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_clear_meta_contexts (nbd) == -1 ||
+ nbd_add_meta_context (nbd, "x-nosuch:") == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_list_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data =
&p});
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 0 || p.count != 0 || p.seen) {
+ fprintf (stderr, "expecting no contexts, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+ r = nbd_get_size (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1048576) {
+ fprintf (stderr, "expecting get_size of 1M, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+ r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "expecting can_meta_context to succeed, got
%d\n", r);
+ exit (EXIT_FAILURE);
+ }
+
/* Final pass: "base:" query should get at least
"base:allocation" */
p = (struct progress) { .count = 0 };
- r = nbd_clear_meta_contexts (nbd);
- if (r == -1) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- exit (EXIT_FAILURE);
- }
r = nbd_add_meta_context (nbd, "base:");
if (r == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
diff --git a/golang/libnbd_240_opt_list_meta_test.go
b/golang/libnbd_240_opt_list_meta_test.go
index c329aaf6..9d6f6f28 100644
--- a/golang/libnbd_240_opt_list_meta_test.go
+++ b/golang/libnbd_240_opt_list_meta_test.go
@@ -120,13 +120,73 @@ func Test240OptListMeta(t *testing.T) {
t.Fatalf("unexpected count after opt_list_meta_context")
}
- /* Final pass: "base:" query should get at least
"base:allocation" */
+ /* Fourth pass: opt_list_meta_context is stateless, so it should
+ * not wipe status learned during opt_info
+ */
list_count = 0
list_seen = false
+ _, err = h.GetSize()
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+ _, err = h.CanMetaContext(context_base_allocation)
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+ err = h.OptInfo()
+ if err != nil {
+ t.Fatalf("opt_info failed unexpectedly: %s", err)
+ }
+ size, err := h.GetSize()
+ if err != nil {
+ t.Fatalf("get_size failed unexpectedly: %s", err)
+ }
+ if size != 1048576 {
+ t.Fatalf("get_size gave wrong size")
+ }
+ meta, err := h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("can_meta_context failed unexpectedly: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected count after can_meta_context")
+ }
err = h.ClearMetaContexts()
if err != nil {
t.Fatalf("could not request clear_meta_contexts: %s", err)
}
+ err = h.AddMetaContext("x-nosuch:")
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ r, err = h.OptListMetaContext(func(name string) int {
+ return listmetaf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_list_meta_context: %s", err)
+ }
+ if r != 0 || r != list_count || list_seen {
+ t.Fatalf("unexpected count after opt_list_meta_context")
+ }
+ size, err = h.GetSize()
+ if err != nil {
+ t.Fatalf("get_size failed unexpectedly: %s", err)
+ }
+ if size != 1048576 {
+ t.Fatalf("get_size gave wrong size")
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("can_meta_context failed unexpectedly: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected count after can_meta_context")
+ }
+ err = h.ClearMetaContexts()
+
+ /* Final pass: "base:" query should get at least
"base:allocation" */
+ list_count = 0
+ list_seen = false
err = h.AddMetaContext("base:")
if err != nil {
t.Fatalf("could not request add_meta_context: %s", err)
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 13/18] api: Reset state on changed nbd_set_export_name()
The documentation for nbd_internal_reset_size_and_flags() claims we
should wipe all knowledge of a previously-negotiated export when
swapping export names. However, we weren't actually doing that, which
could lead to a user calling nbd_opt_info() for one export, then
switching to another export name, then having nbd_get_size() still
report the size of the old export. The next patch will add testsuite
coverage (done separately, to make it easier to prove the test catches
our flaw without this fix by applying patches out of order).
While at it, there is no need to wipe state if we change to the same
name. Note, however, that we do not bother to store the last name
we've exposed to the server; as a result, there are instances where we
clear state but the server does not. But in general, this is not a
problem; it's always okay to be more conservative and not utilize
something the server supports, than to be be overly risky and use the
server on the basis of stale state. Here's an example (unlikely to
happen in real code) where we are over-eager in wiping state, even
though the server never knows we considered a different name:
nbd_set_export_name(nbd, "a");
nbd_opt_info(nbd, callback); => NBD_OPT_SET_META_CONTEXT("a")
=> NBD_OPT_INFO("a")
nbd_set_export_name(nbd, "b");
nbd_set_request_meta_context(nbd, false);
nbd_set_export_name(nbd, "a");
nbd_opt_go(nbd); => NBD_OPT_GO("a")
At any rate, this patch, plus several earlier ones, give us the
following state transitions:
export information (nbd_get_size, nbd_is_read_only, nbd_can_trim, ...)
- gated by h->eflags being non-zero, but tracked in multiple
variables such as h->eflags, h->exportsize, h->block_minimum, ...
- made valid by successful NBD_OPT_INFO or NBD_OPT_GO
- wiped by:
- failed NBD_OPT_INFO or NBD_OPT_GO
- any NBD_OPT_STARTTLS (a later patch may add nbd_opt_starttls;
for now, it is not possible to tickle this)
- nbd_set_export_name() changing names (not directly caused by
server state, but because of how many of our interfaces
implicitly reuse h->export_name)
- persists over:
- NBD_OPT_LIST_META_CONTEXT
- NBD_OPT_SET_META_CONTEXT
context mappings (nbd_can_meta_context)
- gated by h->meta_valid, tracked by h->meta_contexts
- made valid (returns 0/1 for all names) by:
- successful NBD_OPT_SET_META_CONTEXT
- if nbd_set_request_meta_context() is true, a successful
nbd_opt_info() or nbd_opt_go() that was unable to set contexts
(server was oldstyle, or structured replies are lacking, or
client didn't use nbd_add_meta_context)
- wiped (returns -1 for all names) by:
- failed NBD_OPT_SET_META_CONTEXT
- any NBD_OPT_STARTTLS
- nbd_set_export_name() changing names
- persists over:
- NBD_OPT_GO, NBD_OPT_LIST
- NBD_OPT_LIST_META_CONTEXT
Fixes: 437a472d ("api: Add nbd_opt_go and nbd_aio_opt_go", v1.3.12)
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
lib/handle.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/handle.c b/lib/handle.c
index e59e6160..2be1a0e6 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -232,6 +232,9 @@ nbd_unlocked_set_export_name (struct nbd_handle *h, const
char *export_name)
return -1;
}
+ if (strcmp (export_name, h->export_name) == 0)
+ return 0;
+
new_name = strdup (export_name);
if (!new_name) {
set_error (errno, "strdup");
@@ -240,6 +243,7 @@ nbd_unlocked_set_export_name (struct nbd_handle *h, const
char *export_name)
free (h->export_name);
h->export_name = new_name;
+ nbd_internal_reset_size_and_flags (h);
h->meta_valid = false;
return 0;
}
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 14/18] tests: Add coverage for nbd_set_export_name fix
Add test coverage for the sequences fixed in the previous patch, where
changing the export name should clean up anything remembered about a
former export name. The tests in other language bindings are
logically equivalent.
Acked-by: Laszlo Ersek <lersek at redhat.com>
---
python/t/230-opt-info.py | 11 ++++++++---
ocaml/tests/test_230_opt_info.ml | 11 ++++++++---
tests/opt-info.c | 24 ++++++++++++++++++------
golang/libnbd_230_opt_info_test.go | 23 +++++++++++++++++------
4 files changed, 51 insertions(+), 18 deletions(-)
diff --git a/python/t/230-opt-info.py b/python/t/230-opt-info.py
index c326bc30..45682dd9 100644
--- a/python/t/230-opt-info.py
+++ b/python/t/230-opt-info.py
@@ -45,17 +45,22 @@ def must_fail(f, *args, **kwds):
assert h.is_read_only() is True
assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
-# info on something not present fails, wipes out prior info
-h.set_export_name("a")
-must_fail(h.opt_info)
+# changing export wipes out prior info
+h.set_export_name("b")
must_fail(h.get_size)
must_fail(h.is_read_only)
must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
+# info on something not present fails
+h.set_export_name("a")
+must_fail(h.opt_info)
+
# info for a different export, with automatic meta_context disabled
h.set_export_name("b")
h.set_request_meta_context(False)
h.opt_info()
+# idempotent name change is no-op
+h.set_export_name("b")
assert h.get_size() == 1
assert h.is_read_only() is False
must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
diff --git a/ocaml/tests/test_230_opt_info.ml b/ocaml/tests/test_230_opt_info.ml
index ec735ff1..0403d14e 100644
--- a/ocaml/tests/test_230_opt_info.ml
+++ b/ocaml/tests/test_230_opt_info.ml
@@ -62,17 +62,22 @@ let
let meta = NBD.can_meta_context nbd NBD.context_base_allocation in
assert meta;
- (* info on something not present fails, wipes out prior info *)
- NBD.set_export_name nbd "a";
- fail_unary NBD.opt_info nbd;
+ (* changing export wipes out prior info *)
+ NBD.set_export_name nbd "b";
fail_unary NBD.get_size nbd;
fail_unary NBD.is_read_only nbd;
fail_binary NBD.can_meta_context nbd NBD.context_base_allocation;
+ (* info on something not present fails *)
+ NBD.set_export_name nbd "a";
+ fail_unary NBD.opt_info nbd;
+
(* info for a different export, with automatic meta_context disabled *)
NBD.set_export_name nbd "b";
NBD.set_request_meta_context nbd false;
NBD.opt_info nbd;
+ (* idempotent name change is no-op *)
+ NBD.set_export_name nbd "b";
let size = NBD.get_size nbd in
assert (size = 1L);
let ro = NBD.is_read_only nbd in
diff --git a/tests/opt-info.c b/tests/opt-info.c
index cf423d5c..737af2a4 100644
--- a/tests/opt-info.c
+++ b/tests/opt-info.c
@@ -17,6 +17,7 @@
*/
/* Test behavior of nbd_opt_info. */
+/* See also unit test 230 in the various language ports. */
#include <config.h>
@@ -80,15 +81,11 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- /* info on something not present fails, wipes out prior info */
- if (nbd_set_export_name (nbd, "a") == -1) {
+ /* changing export wipes out prior info */
+ if (nbd_set_export_name (nbd, "b") == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
- if (nbd_opt_info (nbd) != -1) {
- fprintf (stderr, "expecting error for opt_info\n");
- exit (EXIT_FAILURE);
- }
if (nbd_get_size (nbd) != -1) {
fprintf (stderr, "expecting error for get_size\n");
exit (EXIT_FAILURE);
@@ -102,6 +99,16 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
+ /* info on something not present fails */
+ if (nbd_set_export_name (nbd, "a") == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_opt_info (nbd) != -1) {
+ fprintf (stderr, "expecting error for opt_info\n");
+ exit (EXIT_FAILURE);
+ }
+
/* info for a different export, with automatic meta_context disabled */
if (nbd_set_export_name (nbd, "b") == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
@@ -115,6 +122,11 @@ main (int argc, char *argv[])
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
+ /* idempotent name change is no-op */
+ if (nbd_set_export_name (nbd, "b") == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
if ((r = nbd_get_size (nbd)) != 1) {
fprintf (stderr, "expecting size of 1, got %" PRId64
"\n", r);
exit (EXIT_FAILURE);
diff --git a/golang/libnbd_230_opt_info_test.go
b/golang/libnbd_230_opt_info_test.go
index bc4cadfb..e16f6611 100644
--- a/golang/libnbd_230_opt_info_test.go
+++ b/golang/libnbd_230_opt_info_test.go
@@ -91,15 +91,11 @@ func Test230OptInfo(t *testing.T) {
t.Fatalf("unexpected meta context")
}
- /* info on something not present fails, wipes out prior info */
- err = h.SetExportName("a")
+ /* changing export wipes out prior info */
+ err = h.SetExportName("b")
if err != nil {
t.Fatalf("set export name failed unexpectedly: %s", err)
}
- err = h.OptInfo()
- if err == nil {
- t.Fatalf("expected error")
- }
_, err = h.GetSize()
if err == nil {
t.Fatalf("expected error")
@@ -113,6 +109,16 @@ func Test230OptInfo(t *testing.T) {
t.Fatalf("expected error")
}
+ /* info on something not present fails */
+ err = h.SetExportName("a")
+ if err != nil {
+ t.Fatalf("set export name failed unexpectedly: %s", err)
+ }
+ err = h.OptInfo()
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+
/* info for a different export, with automatic meta_context disabled */
err = h.SetExportName("b")
if err != nil {
@@ -126,6 +132,11 @@ func Test230OptInfo(t *testing.T) {
if err != nil {
t.Fatalf("opt_info failed unexpectedly: %s", err)
}
+ /* idempotent name change is no-op */
+ err = h.SetExportName("b")
+ if err != nil {
+ t.Fatalf("set export name failed unexpectedly: %s", err)
+ }
size, err = h.GetSize()
if err != nil {
t.Fatalf("size failed unexpectedly: %s", err)
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 15/18] api: Add nbd_[aio_]opt_set_meta_context[_queries]
Make it possible for the client to choose when to manually request
meta contexts when using nbd_set_opt_mode(). By itself, this patch
allows us to test how a server reacts to NBD_OPT_SET_META_CONTEXT
without a previous negotiation of structured headers; then an upcoming
patch will also add APIs for choosing when to negotiate OPT_STARTTLS
and OPT_STRUCTURED_REPLY for even more fine-grained testing control.
Most users won't need this API (the automatic defaults are good
enough), but when writing a server, this level of integration testing
can be useful. Similar to the existing nbd_opt_list_meta_context
API, also offer a variant that takes an explicit query list.
As this is a new API, the unit test in C is included in this patch;
there is no point in splitting it as reordering the patches would not
compile. However, porting the tests to other languages will be done
in the next commit.
---
generator/API.ml | 207 +++++++++++++++-
generator/states-newstyle-opt-meta-context.c | 39 ++--
generator/states-newstyle.c | 1 +
lib/opt.c | 70 +++++-
tests/Makefile.am | 14 ++
tests/opt-set-meta-queries.c | 149 ++++++++++++
tests/opt-set-meta.c | 234 +++++++++++++++++++
.gitignore | 2 +
8 files changed, 690 insertions(+), 26 deletions(-)
create mode 100644 tests/opt-set-meta-queries.c
create mode 100644 tests/opt-set-meta.c
diff --git a/generator/API.ml b/generator/API.ml
index 0f70dcd1..9ffe6516 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -845,17 +845,20 @@ "set_request_meta_context", {
is true; however the extra step of negotiating meta contexts is
not always desirable: performing both info and go on the same
export works without needing to re-negotiate contexts on the
-second call; and even when using just L<nbd_opt_info(3)>, it
-can be faster to collect the server's results by relying on the
-callback function passed to L<nbd_opt_list_meta_context(3)> than
-a series of post-process calls to L<nbd_can_meta_context(3)>.
+second call; integration testing of other servers may benefit
+from manual invocation of L<nbd_opt_set_meta_context(3)> at
+other times in the negotiation sequence; and even when using just
+L<nbd_opt_info(3)>, it can be faster to collect the server's
+results by relying on the callback function passed to
+L<nbd_opt_list_meta_context(3)> than a series of post-process
+calls to L<nbd_can_meta_context(3)>.
Note that this control has no effect if the server does not
negotiate structured replies, or if the client did not request
any contexts via L<nbd_add_meta_context(3)>. Setting this
control to false may cause L<nbd_block_status(3)> to fail.";
see_also = [Link "set_opt_mode"; Link "opt_go"; Link
"opt_info";
- Link "opt_list_meta_context";
+ Link "opt_list_meta_context"; Link
"opt_set_meta_context";
Link "get_structured_replies_negotiated";
Link "get_request_meta_context"; Link
"can_meta_context"];
};
@@ -1095,6 +1098,7 @@ "set_opt_mode", {
see_also = [Link "get_opt_mode"; Link
"aio_is_negotiating";
Link "opt_abort"; Link "opt_go"; Link
"opt_list";
Link "opt_info"; Link
"opt_list_meta_context";
+ Link "opt_set_meta_context";
Link "aio_connect"];
};
@@ -1121,7 +1125,9 @@ "opt_go", {
By default, libnbd will automatically request all meta contexts
registered by L<nbd_add_meta_context(3)> as part of this call; but
-this can be suppressed with L<nbd_set_request_meta_context(3)>.
+this can be suppressed with L<nbd_set_request_meta_context(3)>,
+particularly if L<nbd_opt_set_meta_context(3)> was used earlier
+in the negotiation sequence.
If this fails, the server may still be in negotiation, where it is
possible to attempt another option such as a different export name;
@@ -1129,7 +1135,8 @@ "opt_go", {
example = Some "examples/list-exports.c";
see_also = [Link "set_opt_mode"; Link "aio_opt_go";
Link "opt_abort";
Link "set_export_name"; Link "connect_uri";
Link "opt_info";
- Link "add_meta_context"; Link
"set_request_meta_context"];
+ Link "add_meta_context"; Link
"set_request_meta_context";
+ Link "opt_set_meta_context"];
};
"opt_abort", {
@@ -1268,7 +1275,8 @@ "opt_list_meta_context", {
see_also = [Link "set_opt_mode"; Link
"aio_opt_list_meta_context";
Link "opt_list_meta_context_queries";
Link "add_meta_context"; Link
"clear_meta_contexts";
- Link "opt_go"; Link "set_export_name"];
+ Link "opt_go"; Link "set_export_name";
+ Link "opt_set_meta_context"];
};
"opt_list_meta_context_queries", {
@@ -1321,6 +1329,118 @@ "opt_list_meta_context_queries", {
Link "opt_go"; Link "set_export_name"];
};
+ "opt_set_meta_context", {
+ default_call with
+ args = [ Closure context_closure ]; ret = RInt;
+ permitted_states = [ Negotiating ];
+ shortdesc = "select specific meta contexts, using implicit query
list";
+ longdesc = "\
+Request that the server supply all recognized meta contexts
+registered through prior calls to L<nbd_add_meta_context(3)>, in
+conjunction with the export previously specified by the most
+recent L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.
+This can only be used if L<nbd_set_opt_mode(3)> enabled option
+mode. Normally, this function is redundant, as L<nbd_opt_go(3)>
+automatically does the same task if structured replies have
+already been negotiated. But manual control over meta context
+requests can be useful for fine-grained testing of how a server
+handles unusual negotiation sequences. Often, use of this
+function is coupled with L<nbd_set_request_meta_context(3)> to
+bypass the automatic context request normally performed by
+L<nbd_opt_go(3)>.
+
+The NBD protocol allows a client to decide how many queries to ask
+the server. Rather than taking that list of queries as a parameter
+to this function, libnbd reuses the current list of requested meta
+contexts as set by L<nbd_add_meta_context(3)>; you can use
+L<nbd_clear_meta_contexts(3)> to set up a different list of queries
+(see L<nbd_opt_set_meta_context_queries(3)> to pass an explicit
+list of contexts instead). Since this function is primarily
+designed for testing servers, libnbd does not prevent the use
+of this function on an empty list or when
+L<nbd_set_request_structured_replies(3)> has disabled structured
+replies, in order to see how a server behaves.
+
+The C<context> function is called once per server reply, with any
+C<user_data> passed to this function, and with C<name> supplied by
+the server. Additionally, each server name will remain visible through
+L<nbd_can_meta_context(3)> until the next attempt at
+L<nbd_set_export_name(3)> or L<nbd_opt_set_meta_context(3)>, as
+well as L<nbd_opt_go(3)> or L<nbd_opt_info(3)> that trigger an
+automatic meta context request. Remember that it is not safe to
+call any C<nbd_*> APIs from within the context of the callback
+function. At present, the return value of the callback is
+ignored, although a return of -1 should be avoided.
+
+For convenience, when this function succeeds, it returns the number
+of replies returned by the server.
+
+Not all servers understand this request, and even when it is understood,
+the server might intentionally send an empty list because it does not
+support the requested context, or may encounter a failure after
+delivering partial results. Thus, this function may succeed even when
+no contexts are reported, or may fail but have a non-empty list.";
+ see_also = [Link "set_opt_mode"; Link
"aio_opt_set_meta_context";
+ Link "add_meta_context"; Link
"clear_meta_contexts";
+ Link "opt_set_meta_context_queries";
+ Link "opt_go"; Link "set_export_name";
+ Link "opt_list_meta_context"; Link
"set_request_meta_context";
+ Link "can_meta_context"];
+ };
+
+ "opt_set_meta_context_queries", {
+ default_call with
+ args = [ StringList "queries"; Closure context_closure ]; ret =
RInt;
+ permitted_states = [ Negotiating ];
+ shortdesc = "select specific meta contexts, using explicit query
list";
+ longdesc = "\
+Request that the server supply all recognized meta contexts
+passed in through C<queries>, in conjunction with the export
+previously specified by the most recent L<nbd_set_export_name(3)>
+or L<nbd_connect_uri(3)>. This can only be used if
+L<nbd_set_opt_mode(3)> enabled option mode. Normally, this
+function is redundant, as L<nbd_opt_go(3)> automatically does
+the same task if structured replies have already been
+negotiated. But manual control over meta context requests can
+be useful for fine-grained testing of how a server handles
+unusual negotiation sequences. Often, use of this function is
+coupled with L<nbd_set_request_meta_context(3)> to bypass the
+automatic context request normally performed by L<nbd_opt_go(3)>.
+
+The NBD protocol allows a client to decide how many queries to ask
+the server. This function takes an explicit list of queries; to
+instead reuse an implicit list, see L<nbd_opt_set_meta_context(3)>.
+Since this function is primarily designed for testing servers,
+libnbd does not prevent the use of this function on an empty
+list or when L<nbd_set_request_structured_replies(3)> has
+disabled structured replies, in order to see how a server behaves.
+
+The C<context> function is called once per server reply, with any
+C<user_data> passed to this function, and with C<name> supplied by
+the server. Additionally, each server name will remain visible through
+L<nbd_can_meta_context(3)> until the next attempt at
+L<nbd_set_export_name(3)> or L<nbd_opt_set_meta_context(3)>, as
+well as L<nbd_opt_go(3)> or L<nbd_opt_info(3)> that trigger an
+automatic meta context request. Remember that it is not safe to
+call any C<nbd_*> APIs from within the context of the callback
+function. At present, the return value of the callback is
+ignored, although a return of -1 should be avoided.
+
+For convenience, when this function succeeds, it returns the number
+of replies returned by the server.
+
+Not all servers understand this request, and even when it is understood,
+the server might intentionally send an empty list because it does not
+support the requested context, or may encounter a failure after
+delivering partial results. Thus, this function may succeed even when
+no contexts are reported, or may fail but have a non-empty list.";
+ see_also = [Link "set_opt_mode"; Link
"aio_opt_set_meta_context_queries";
+ Link "opt_set_meta_context";
+ Link "opt_go"; Link "set_export_name";
+ Link "opt_list_meta_context_queries";
+ Link "set_request_meta_context"; Link
"can_meta_context"];
+ };
+
"add_meta_context", {
default_call with
args = [ String "name" ]; ret = RErr;
@@ -1999,7 +2119,7 @@ "can_meta_context", {
see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "add_meta_context";
Link "block_status"; Link
"aio_block_status";
- Link "set_request_meta_context"];
+ Link "set_request_meta_context"; Link
"opt_set_meta_context"];
};
"get_protocol", {
@@ -2755,6 +2875,71 @@ "aio_opt_list_meta_context_queries", {
Link "aio_opt_list_meta_context"];
};
+ "aio_opt_set_meta_context", {
+ default_call with
+ args = [ Closure context_closure ]; ret = RInt;
+ optargs = [ OClosure completion_closure ];
+ permitted_states = [ Negotiating ];
+ shortdesc = "select specific meta contexts, with implicit query
list";
+ longdesc = "\
+Request that the server supply all recognized meta contexts
+registered through prior calls to L<nbd_add_meta_context(3)>, in
+conjunction with the export previously specified by the most
+recent L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.
+This can only be used if L<nbd_set_opt_mode(3)> enabled option
+mode. Normally, this function is redundant, as L<nbd_opt_go(3)>
+automatically does the same task if structured replies have
+already been negotiated. But manual control over meta context
+requests can be useful for fine-grained testing of how a server
+handles unusual negotiation sequences. Often, use of this
+function is coupled with L<nbd_set_request_meta_context(3)> to
+bypass the automatic context request normally performed by
+L<nbd_opt_go(3)>.
+
+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_set_meta_context";
+ Link "aio_opt_set_meta_context_queries"];
+ };
+
+ "aio_opt_set_meta_context_queries", {
+ default_call with
+ args = [ StringList "queries"; Closure context_closure ]; ret =
RInt;
+ optargs = [ OClosure completion_closure ];
+ permitted_states = [ Negotiating ];
+ shortdesc = "select specific meta contexts, with explicit query
list";
+ longdesc = "\
+Request that the server supply all recognized meta contexts
+passed in through C<queries>, in conjunction with the export
+previously specified by the most recent L<nbd_set_export_name(3)>
+or L<nbd_connect_uri(3)>. This can only be used
+if L<nbd_set_opt_mode(3)> enabled option mode. Normally, this
+function is redundant, as L<nbd_opt_go(3)> automatically does
+the same task if structured replies have already been
+negotiated. But manual control over meta context requests can
+be useful for fine-grained testing of how a server handles
+unusual negotiation sequences. Often, use of this function is
+coupled with L<nbd_set_request_meta_context(3)> to bypass the
+automatic context request normally performed by L<nbd_opt_go(3)>.
+
+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_set_meta_context_queries";
+ Link "aio_opt_set_meta_context"];
+ };
+
"aio_pread", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
@@ -3489,6 +3674,10 @@ let first_version
"aio_opt_list_meta_context_queries", (1, 16);
"set_request_meta_context", (1, 16);
"get_request_meta_context", (1, 16);
+ "opt_set_meta_context", (1, 16);
+ "opt_set_meta_context_queries", (1, 16);
+ "aio_opt_set_meta_context", (1, 16);
+ "aio_opt_set_meta_context_queries", (1, 16);
(* 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-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index e124d06f..46fee15b 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -34,6 +34,8 @@ NEWSTYLE.OPT_META_CONTEXT.START:
* -> conditionally use SET, next state OPT_GO for NBD_OPT_GO
* nbd_opt_list_meta_context()
* -> unconditionally use LIST, next state NEGOTIATING
+ * nbd_opt_set_meta_context()
+ * -> unconditionally use SET, next state NEGOTIATING
*
* If SET is conditional, we skip it if h->request_meta is false, if
* structured replies were not negotiated, or if no contexts to request.
@@ -41,7 +43,7 @@ NEWSTYLE.OPT_META_CONTEXT.START:
* success, while LIST is stateless.
* If OPT_GO is later successful, it populates h->exportsize and friends,
* and also sets h->meta_valid if h->request_meta but we skipped SET
here.
- * There is a callback if and only if the command is LIST.
+ * There is a callback if and only if the command is unconditional.
*/
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) {
@@ -50,9 +52,12 @@ NEWSTYLE.OPT_META_CONTEXT.START:
opt = h->opt_current;
}
else {
- assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
+ if (h->opt_current == NBD_OPT_SET_META_CONTEXT)
+ assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context));
+ else
+ assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
opt = NBD_OPT_SET_META_CONTEXT;
- if (h->request_meta) {
+ if (h->request_meta || h->opt_current == opt) {
for (i = 0; i < h->meta_contexts.len; ++i)
free (h->meta_contexts.ptr[i].name);
meta_vector_reset (&h->meta_contexts);
@@ -215,15 +220,15 @@ NEWSTYLE.OPT_META_CONTEXT.CHECK_REPLY:
len = be32toh (h->sbuf.or.option_reply.replylen);
switch (reply) {
case NBD_REP_ACK: /* End of list of replies. */
- if (opt == NBD_OPT_LIST_META_CONTEXT) {
- SET_NEXT_STATE (%.NEGOTIATING);
- CALL_CALLBACK (h->opt_cb.completion, &err);
- nbd_internal_free_option (h);
- }
- else {
- SET_NEXT_STATE (%^OPT_GO.START);
+ if (opt == NBD_OPT_SET_META_CONTEXT)
h->meta_valid = true;
+ if (opt == h->opt_current) {
+ SET_NEXT_STATE (%.NEGOTIATING);
+ CALL_CALLBACK (h->opt_cb.completion, &err);
+ nbd_internal_free_option (h);
}
+ else
+ SET_NEXT_STATE (%^OPT_GO.START);
break;
case NBD_REP_META_CONTEXT: /* A context. */
if (len > maxpayload)
@@ -242,10 +247,10 @@ NEWSTYLE.OPT_META_CONTEXT.CHECK_REPLY:
}
debug (h, "negotiated %s with context ID %" PRIu32,
meta_context.name, meta_context.context_id);
- if (opt == NBD_OPT_LIST_META_CONTEXT) {
+ if (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context))
CALL_CALLBACK (h->opt_cb.fn.context, meta_context.name);
+ if (opt == NBD_OPT_LIST_META_CONTEXT)
free (meta_context.name);
- }
else if (meta_vector_append (&h->meta_contexts, meta_context) ==
-1) {
set_error (errno, "realloc");
free (meta_context.name);
@@ -256,25 +261,27 @@ NEWSTYLE.OPT_META_CONTEXT.CHECK_REPLY:
SET_NEXT_STATE (%PREPARE_FOR_REPLY);
break;
default:
- /* Anything else is an error, ignore it for SET, report it for LIST */
+ /* Anything else is an error, report it for explicit LIST/SET, ignore it
+ * for automatic progress (nbd_connect_*, nbd_opt_info, nbd_opt_go).
+ */
if (handle_reply_error (h) == -1) {
SET_NEXT_STATE (%.DEAD);
return 0;
}
- if (opt == NBD_OPT_LIST_META_CONTEXT) {
+ if (opt == h->opt_current) {
/* XXX Should we decode specific expected errors, like
* REP_ERR_UNKNOWN to ENOENT or REP_ERR_TOO_BIG to ERANGE?
*/
err = ENOTSUP;
set_error (err, "unexpected response, possibly the server does not
"
- "support listing contexts");
+ "support meta contexts");
CALL_CALLBACK (h->opt_cb.completion, &err);
nbd_internal_free_option (h);
SET_NEXT_STATE (%.NEGOTIATING);
}
else {
- debug (h, "handshake: unexpected error from "
+ debug (h, "handshake: ignoring unexpected error from "
"NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")",
reply);
SET_NEXT_STATE (%^OPT_GO.START);
}
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index e84823e3..60e0bc50 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -140,6 +140,7 @@ NEWSTYLE.START:
SET_NEXT_STATE (%PREPARE_OPT_ABORT);
return 0;
case NBD_OPT_LIST_META_CONTEXT:
+ case NBD_OPT_SET_META_CONTEXT:
SET_NEXT_STATE (%OPT_META_CONTEXT.START);
return 0;
case 0:
diff --git a/lib/opt.c b/lib/opt.c
index cfdabb9e..ee036235 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -32,7 +32,8 @@ nbd_internal_free_option (struct nbd_handle *h)
{
if (h->opt_current == NBD_OPT_LIST)
FREE_CALLBACK (h->opt_cb.fn.list);
- else if (h->opt_current == NBD_OPT_LIST_META_CONTEXT)
+ else if (h->opt_current == NBD_OPT_LIST_META_CONTEXT ||
+ h->opt_current == NBD_OPT_SET_META_CONTEXT)
FREE_CALLBACK (h->opt_cb.fn.context);
FREE_CALLBACK (h->opt_cb.completion);
}
@@ -224,6 +225,37 @@ nbd_unlocked_opt_list_meta_context_queries (struct
nbd_handle *h,
return s.count;
}
+/* Issue NBD_OPT_SET_META_CONTEXT and wait for the reply. */
+int
+nbd_unlocked_opt_set_meta_context (struct nbd_handle *h,
+ nbd_context_callback *context)
+{
+ return nbd_unlocked_opt_set_meta_context_queries (h, NULL, context);
+}
+
+/* Issue NBD_OPT_SET_META_CONTEXT and wait for the reply. */
+int
+nbd_unlocked_opt_set_meta_context_queries (struct nbd_handle *h,
+ char **queries,
+ nbd_context_callback *context)
+{
+ struct context_helper s = { .context = *context };
+ nbd_context_callback l = { .callback = context_visitor, .user_data = &s
};
+ nbd_completion_callback c = { .callback = context_complete, .user_data =
&s };
+
+ if (nbd_unlocked_aio_opt_set_meta_context_queries (h, queries, &l,
&c) == -1)
+ return -1;
+
+ SET_CALLBACK_TO_NULL (*context);
+ if (wait_for_option (h) == -1)
+ return -1;
+ if (s.err) {
+ set_error (s.err, "server replied with error to set meta context
request");
+ return -1;
+ }
+ return s.count;
+}
+
/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */
int
nbd_unlocked_aio_opt_go (struct nbd_handle *h,
@@ -324,3 +356,39 @@ nbd_unlocked_aio_opt_list_meta_context_queries (struct
nbd_handle *h,
debug (h, "option queued, ignoring state machine failure");
return 0;
}
+
+/* Issue NBD_OPT_SET_META_CONTEXT without waiting. */
+int
+nbd_unlocked_aio_opt_set_meta_context (struct nbd_handle *h,
+ nbd_context_callback *context,
+ nbd_completion_callback *complete)
+{
+ return nbd_unlocked_aio_opt_set_meta_context_queries (h, NULL, context,
+ complete);
+}
+
+/* Issue NBD_OPT_SET_META_CONTEXT without waiting. */
+int
+nbd_unlocked_aio_opt_set_meta_context_queries (struct nbd_handle *h,
+ char **queries,
+ nbd_context_callback *context,
+ 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;
+ }
+
+ if (nbd_internal_set_querylist (h, queries) == -1)
+ return -1;
+
+ assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
+ h->opt_cb.fn.context = *context;
+ SET_CALLBACK_TO_NULL (*context);
+ h->opt_cb.completion = *complete;
+ SET_CALLBACK_TO_NULL (*complete);
+ h->opt_current = NBD_OPT_SET_META_CONTEXT;
+ 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 dfb7f8bd..ec95495e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -215,6 +215,8 @@ check_PROGRAMS += \
opt-info \
opt-list-meta \
opt-list-meta-queries \
+ opt-set-meta \
+ opt-set-meta-queries \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -282,6 +284,8 @@ TESTS += \
opt-info \
opt-list-meta \
opt-list-meta-queries \
+ opt-set-meta \
+ opt-set-meta-queries \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -543,6 +547,16 @@ opt_list_meta_LDADD = $(top_builddir)/lib/libnbd.la
opt_list_meta_queries_SOURCES = opt-list-meta-queries.c
opt_list_meta_queries_LDADD = $(top_builddir)/lib/libnbd.la
+opt_set_meta_SOURCES = opt-set-meta.c
+opt_set_meta_CPPFLAGS = \
+ $(AM_CPPFLAGS) \
+ -I$(top_srcdir)/common/include \
+ $(NULL)
+opt_set_meta_LDADD = $(top_builddir)/lib/libnbd.la
+
+opt_set_meta_queries_SOURCES = opt-set-meta-queries.c
+opt_set_meta_queries_LDADD = $(top_builddir)/lib/libnbd.la
+
connect_systemd_socket_activation_SOURCES = \
connect-systemd-socket-activation.c \
requires.c \
diff --git a/tests/opt-set-meta-queries.c b/tests/opt-set-meta-queries.c
new file mode 100644
index 00000000..0b162607
--- /dev/null
+++ b/tests/opt-set-meta-queries.c
@@ -0,0 +1,149 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2022 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_set_meta_context_queries. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <inttypes.h>
+#include <string.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+struct progress {
+ int count;
+ bool seen;
+};
+
+static int
+check (void *user_data, const char *name)
+{
+ struct progress *p = user_data;
+
+ p->count++;
+ if (strcmp (name, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0)
+ p->seen = true;
+ return 0;
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int r;
+ struct progress p;
+ char *args[] = { "nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M", NULL };
+ nbd_context_callback ctx = { .callback = check,
+ .user_data = &p};
+
+ /* 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);
+ }
+
+ /* nbdkit does not match wildcard for SET, even though it does for LIST */
+ p = (struct progress) { .count = 0 };
+ {
+ char *base[] = { "base:", NULL };
+ r = nbd_opt_set_meta_context_queries (nbd, base, ctx);
+ }
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != p.count || r != 0 || p.seen) {
+ fprintf (stderr, "inconsistent return value %d, expected %d\n",
r, p.count);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Negotiating with no contexts is not an error, but selects nothing.
+ * An explicit empty list overrides a non-empty implicit list.
+ */
+ if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ p = (struct progress) { .count = 0 };
+ {
+ char *empty[] = { NULL };
+ r = nbd_opt_set_meta_context_queries (nbd, empty, ctx);
+ }
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 0 || p.count || p.seen) {
+ fprintf (stderr, "expecting set_meta to select nothing\n");
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Request 2 with expectation of 1. */
+ p = (struct progress) { .count = 0 };
+ {
+ char *pair[] = { "x-nosuch:context",
LIBNBD_CONTEXT_BASE_ALLOCATION, NULL };
+ r = nbd_opt_set_meta_context_queries (nbd, pair, ctx);
+ }
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1 || p.count != 1 || !p.seen) {
+ fprintf (stderr, "expecting one context, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) {
+ fprintf (stderr, "can_meta_context got %d, expected 1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Transition to transmission phase with with set_request_meta_context off,
+ * our last set should remain active
+ */
+ if (nbd_set_request_meta_context (nbd, 0) == -1 ||
+ nbd_opt_go (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) {
+ fprintf (stderr, "can_meta_context got %d, expected 1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_shutdown (nbd, 0);
+ nbd_close (nbd);
+
+ exit (EXIT_SUCCESS);
+}
diff --git a/tests/opt-set-meta.c b/tests/opt-set-meta.c
new file mode 100644
index 00000000..95f5a6d7
--- /dev/null
+++ b/tests/opt-set-meta.c
@@ -0,0 +1,234 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2022 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_set_meta_context. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <inttypes.h>
+#include <string.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+#include "array-size.h"
+
+struct progress {
+ int count;
+ bool seen;
+};
+
+static int
+check (void *user_data, const char *name)
+{
+ struct progress *p = user_data;
+
+ p->count++;
+ if (strcmp (name, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0)
+ p->seen = true;
+ return 0;
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int r;
+ struct progress p;
+ /* Leave room for --no-sr in second process */
+ char *args[] = { "nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M", NULL, NULL };
+
+ /* First process, with structured replies. 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);
+ }
+
+ /* No contexts negotiated yet; can_meta should be error if any requested */
+ if ((r = nbd_get_structured_replies_negotiated (nbd)) != 1) {
+ fprintf (stderr, "structured replies got %d, expected 1\n", r);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != -1) {
+ fprintf (stderr, "can_meta_context got %d, expected -1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* FIXME: Once nbd_opt_structured_reply exists, check that set before
+ * SR fails server-side, then enable SR for rest of process.
+ */
+
+ /* nbdkit does not match wildcard for SET, even though it does for LIST */
+ if (nbd_clear_meta_contexts (nbd) == -1 ||
+ nbd_add_meta_context (nbd, "base:") == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_set_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data = &p});
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != p.count || r != 0 || p.seen) {
+ fprintf (stderr, "inconsistent return value %d, expected %d\n",
r, p.count);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Negotiating with no contexts is not an error, but selects nothing */
+ r = nbd_clear_meta_contexts (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_set_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data = &p});
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 0 || p.count || p.seen) {
+ fprintf (stderr, "expecting set_meta to select nothing\n");
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Request 2 with expectation of 1; with set_request_meta_context off */
+ if (nbd_add_meta_context (nbd, "x-nosuch:context") == -1 ||
+ nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1 ||
+ nbd_set_request_meta_context (nbd, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_set_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data = &p});
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1 || p.count != 1 || !p.seen) {
+ fprintf (stderr, "expecting one context, got %d\n", r);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) {
+ fprintf (stderr, "can_meta_context got %d, expected 1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Transition to transmission phase; our last set should remain active */
+ if (nbd_clear_meta_contexts (nbd) == -1 ||
+ nbd_add_meta_context (nbd, "x-nosuch:context") == -1 ||
+ nbd_opt_go (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) {
+ fprintf (stderr, "can_meta_context got %d, expected 1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Now too late to set; but should not lose earlier state */
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_set_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data = &p});
+ if (r != -1 || p.count || p.seen) {
+ fprintf (stderr, "expecting set_meta failure\n");
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 1) {
+ fprintf (stderr, "can_meta_context got %d, expected 1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_shutdown (nbd, 0);
+ nbd_close (nbd);
+
+ /* Second process, this time without structured replies server-side. */
+ args[ARRAY_SIZE (args) - 2] = (char *) "--no-sr";
+ nbd = nbd_create ();
+ if (nbd == NULL ||
+ nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1 ||
+ nbd_connect_command (nbd, args) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_get_structured_replies_negotiated (nbd)) != 0) {
+ fprintf (stderr, "structured replies got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Expect server-side failure here */
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_set_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data = &p});
+ if (r != -1 || p.count || p.seen) {
+ fprintf (stderr, "expecting set_meta failure\n");
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != -1) {
+ fprintf (stderr, "can_meta_context got %d, expected -1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Even though can_meta fails after failed SET, it returns 0 after go */
+ if (nbd_opt_go (nbd) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_shutdown (nbd, 0);
+ nbd_close (nbd);
+
+ exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index 272d1ec1..50e4616e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -234,6 +234,8 @@ Makefile.in
/tests/opt-list
/tests/opt-list-meta
/tests/opt-list-meta-queries
+/tests/opt-set-meta
+/tests/opt-set-meta-queries
/tests/pki/
/tests/pread-initialize
/tests/private-data
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 16/18] tests: Language port of nbd_opt_set_meta_context() tests
As promised in the previous patch, also test the new
nbd_opt_set_meta_context() API in Python, OCaml, and Golang.
---
python/t/250-opt-set-meta.py | 126 ++++++++
python/t/255-opt-set-meta-queries.py | 76 +++++
ocaml/tests/Makefile.am | 2 +
ocaml/tests/test_250_opt_set_meta.ml | 151 ++++++++++
ocaml/tests/test_255_opt_set_meta_queries.ml | 79 +++++
tests/opt-set-meta-queries.c | 1 +
tests/opt-set-meta.c | 1 +
golang/Makefile.am | 2 +
golang/libnbd_250_opt_set_meta_test.go | 276 ++++++++++++++++++
.../libnbd_255_opt_set_meta_queries_test.go | 141 +++++++++
10 files changed, 855 insertions(+)
create mode 100644 python/t/250-opt-set-meta.py
create mode 100644 python/t/255-opt-set-meta-queries.py
create mode 100644 ocaml/tests/test_250_opt_set_meta.ml
create mode 100644 ocaml/tests/test_255_opt_set_meta_queries.ml
create mode 100644 golang/libnbd_250_opt_set_meta_test.go
create mode 100644 golang/libnbd_255_opt_set_meta_queries_test.go
diff --git a/python/t/250-opt-set-meta.py b/python/t/250-opt-set-meta.py
new file mode 100644
index 00000000..c543a5d6
--- /dev/null
+++ b/python/t/250-opt-set-meta.py
@@ -0,0 +1,126 @@
+# libnbd Python bindings
+# Copyright (C) 2010-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+import nbd
+
+
+count = 0
+seen = False
+
+
+def f(user_data, name):
+ global count
+ global seen
+ assert user_data == 42
+ count = count + 1
+ if name == nbd.CONTEXT_BASE_ALLOCATION:
+ seen = True
+
+
+def must_fail(f, *args, **kwds):
+ try:
+ f(*args, **kwds)
+ assert False
+ except nbd.Error:
+ pass
+
+
+# First process, with structured replies. Get into negotiating state.
+h = nbd.NBD()
+h.set_opt_mode(True)
+h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M"])
+
+# No contexts negotiated yet; can_meta should be error if any requested
+assert h.get_structured_replies_negotiated() is True
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
+
+# FIXME: Once nbd_opt_structured_reply exists, check that set before
+# SR fails server-side, then enable SR for rest of process.
+
+# nbdkit does not match wildcard for SET, even though it does for LIST
+count = 0
+seen = False
+h.clear_meta_contexts()
+h.add_meta_context("base:")
+r = h.opt_set_meta_context(lambda *args: f(42, *args))
+assert r == count
+assert r == 0
+assert seen is False
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+
+# Negotiating with no contexts is not an error, but selects nothing
+count = 0
+seen = False
+h.clear_meta_contexts()
+r = h.opt_set_meta_context(lambda *args: f(42, *args))
+assert r == 0
+assert r == count
+assert seen is False
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+
+# Request 2 with expectation of 1; with set_request_meta_context off
+count = 0
+seen = False
+h.add_meta_context("x-nosuch:context")
+h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+h.set_request_meta_context(False)
+r = h.opt_set_meta_context(lambda *args: f(42, *args))
+assert r == 1
+assert count == 1
+assert seen is True
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+
+# Transition to transmission phase; our last set should remain active
+h.clear_meta_contexts()
+h.add_meta_context("x-nosuch:context")
+h.opt_go()
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+
+# Now too late to set; but should not lose earlier state
+count = 0
+seen = False
+must_fail(h.opt_set_meta_context, lambda *args: f(42, *args))
+assert count == 0
+assert seen is False
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+
+h.shutdown()
+
+# Second process, this time without structured replies server-side.
+h = nbd.NBD()
+h.set_opt_mode(True)
+h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M",
"--no-sr"])
+assert h.get_structured_replies_negotiated() is False
+
+# Expect server-side failure here
+count = 0
+seen = False
+must_fail(h.opt_set_meta_context, lambda *args: f(42, *args))
+assert count == 0
+assert seen is False
+must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
+
+# Even though can_meta fails after failed SET, it returns 0 after go
+h.opt_go()
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+
+h.shutdown()
diff --git a/python/t/255-opt-set-meta-queries.py
b/python/t/255-opt-set-meta-queries.py
new file mode 100644
index 00000000..54a10bc0
--- /dev/null
+++ b/python/t/255-opt-set-meta-queries.py
@@ -0,0 +1,76 @@
+# libnbd Python bindings
+# Copyright (C) 2010-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+import nbd
+
+
+count = 0
+seen = False
+
+
+def f(user_data, name):
+ global count
+ global seen
+ assert user_data == 42
+ count = count + 1
+ if name == nbd.CONTEXT_BASE_ALLOCATION:
+ seen = True
+
+
+# Get into negotiating state.
+h = nbd.NBD()
+h.set_opt_mode(True)
+h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
+ "memory", "size=1M"])
+
+# nbdkit does not match wildcard for SET, even though it does for LIST
+count = 0
+seen = False
+r = h.opt_set_meta_context_queries(["base:"], lambda *args: f(42,
*args))
+assert r == count
+assert r == 0
+assert seen is False
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+
+# Negotiating with no contexts is not an error, but selects nothing.
+# An explicit empty list overrides a non-empty implicit list.
+count = 0
+seen = False
+h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+r = h.opt_set_meta_context_queries([], lambda *args: f(42, *args))
+assert r == 0
+assert r == count
+assert seen is False
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+
+# Request 2 with expectation of 1.
+count = 0
+seen = False
+r = h.opt_set_meta_context_queries(
+ ["x-nosuch:context", nbd.CONTEXT_BASE_ALLOCATION],
+ lambda *args: f(42, *args))
+assert r == 1
+assert count == 1
+assert seen is True
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+
+# Transition to transmission phase; our last set should remain active
+h.set_request_meta_context(False)
+h.opt_go()
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
+
+h.shutdown()
diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am
index 1b4d1135..328d53e5 100644
--- a/ocaml/tests/Makefile.am
+++ b/ocaml/tests/Makefile.am
@@ -32,6 +32,8 @@ ML_TESTS = \
test_230_opt_info.ml \
test_240_opt_list_meta.ml \
test_245_opt_list_meta_queries.ml \
+ test_250_opt_set_meta.ml \
+ test_255_opt_set_meta_queries.ml \
test_300_get_size.ml \
test_400_pread.ml \
test_405_pread_structured.ml \
diff --git a/ocaml/tests/test_250_opt_set_meta.ml
b/ocaml/tests/test_250_opt_set_meta.ml
new file mode 100644
index 00000000..f35012fd
--- /dev/null
+++ b/ocaml/tests/test_250_opt_set_meta.ml
@@ -0,0 +1,151 @@
+(* hey emacs, this is OCaml code: -*- tuareg -*- *)
+(* libnbd OCaml test case
+ * Copyright (C) 2013-2022 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
+ *)
+
+let count = ref 0
+let seen = ref false
+let f user_data name + assert (user_data = 42);
+ count := !count + 1;
+ if name = NBD.context_base_allocation then
+ seen := true;
+ 0
+
+let () + (* First process, with structured replies. Get into negotiating
state. *)
+ let nbd = NBD.create () in
+ NBD.set_opt_mode nbd true;
+ NBD.connect_command nbd
+ ["nbdkit"; "-s";
"--exit-with-parent"; "-v";
+ "memory"; "size=1M"];
+
+ (* No contexts negotiated yet; can_meta should be error if any requested *)
+ let sr = NBD.get_structured_replies_negotiated nbd in
+ assert sr;
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not m);
+ NBD.add_meta_context nbd NBD.context_base_allocation;
+ (try
+ let _ = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
+
+ (* FIXME: Once nbd_opt_structured_reply exists, check that set before
+ * SR fails server-side, then enable SR for rest of process.
+ *)
+
+ (* nbdkit does not match wildcard for SET, even though it does for LIST *)
+ count := 0;
+ seen := false;
+ NBD.clear_meta_contexts nbd;
+ NBD.add_meta_context nbd "base:";
+ let r = NBD.opt_set_meta_context nbd (f 42) in
+ assert (r = !count);
+ assert (r = 0);
+ assert (not !seen);
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not m);
+
+ (* Negotiating with no contexts is not an error, but selects nothing *)
+ count := 0;
+ seen := false;
+ NBD.clear_meta_contexts nbd;
+ let r = NBD.opt_set_meta_context nbd (f 42) in
+ assert (r = 0);
+ assert (r = !count);
+ assert (not !seen);
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not m);
+
+ (* Request 2 with expectation of 1; with set_request_meta_context off *)
+ count := 0;
+ seen := false;
+ NBD.add_meta_context nbd "x-nosuch:context";
+ NBD.add_meta_context nbd NBD.context_base_allocation;
+ NBD.set_request_meta_context nbd false;
+ let r = NBD.opt_set_meta_context nbd (f 42) in
+ assert (r = 1);
+ assert (r = !count);
+ assert !seen;
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert m;
+
+ (* Transition to transmission phase; our last set should remain active *)
+ NBD.clear_meta_contexts nbd;
+ NBD.add_meta_context nbd "x-nosuch:context";
+ NBD.opt_go nbd;
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert m;
+
+ (* Now too late to set; but should not lose earlier state *)
+ count := 0;
+ seen := false;
+ (try
+ let _ = NBD.opt_set_meta_context nbd (f 42) in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
+ assert (0 = !count);
+ assert (not !seen);
+ let s = NBD.get_size nbd in
+ assert (s = 1048576_L);
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert m;
+
+ NBD.shutdown nbd;
+
+ (* Second process, this time without structured replies server-side. *)
+ let nbd = NBD.create () in
+ NBD.set_opt_mode nbd true;
+ NBD.add_meta_context nbd NBD.context_base_allocation;
+ NBD.connect_command nbd
+ ["nbdkit"; "-s";
"--exit-with-parent"; "-v";
+ "memory"; "size=1M";
"--no-sr"];
+ let sr = NBD.get_structured_replies_negotiated nbd in
+ assert (not sr);
+
+ (* Expect server-side failure here *)
+ count := 0;
+ seen := false;
+ NBD.add_meta_context nbd "base:";
+ (try
+ let _ = NBD.opt_set_meta_context nbd (f 42) in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
+ assert (0 = !count);
+ assert (not !seen);
+ (try
+ let _ = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
+
+ (* Even though can_meta fails after failed SET, it returns 0 after go *)
+ NBD.opt_go nbd;
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not m);
+
+ NBD.shutdown nbd
+
+let () = Gc.compact ()
diff --git a/ocaml/tests/test_255_opt_set_meta_queries.ml
b/ocaml/tests/test_255_opt_set_meta_queries.ml
new file mode 100644
index 00000000..86e27e1f
--- /dev/null
+++ b/ocaml/tests/test_255_opt_set_meta_queries.ml
@@ -0,0 +1,79 @@
+(* hey emacs, this is OCaml code: -*- tuareg -*- *)
+(* libnbd OCaml test case
+ * Copyright (C) 2013-2022 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
+ *)
+
+let count = ref 0
+let seen = ref false
+let f user_data name + assert (user_data = 42);
+ count := !count + 1;
+ if name = NBD.context_base_allocation then
+ seen := true;
+ 0
+
+let () + (* Get into negotiating state. *)
+ let nbd = NBD.create () in
+ NBD.set_opt_mode nbd true;
+ NBD.connect_command nbd
+ ["nbdkit"; "-s";
"--exit-with-parent"; "-v";
+ "memory"; "size=1M"];
+
+ (* nbdkit does not match wildcard for SET, even though it does for LIST *)
+ count := 0;
+ seen := false;
+ let r = NBD.opt_set_meta_context_queries nbd ["base:"] (f 42) in
+ assert (r = !count);
+ assert (r = 0);
+ assert (not !seen);
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not m);
+
+ (* Negotiating with no contexts is not an error, but selects nothing.
+ * An explicit empty list overrides a non-empty implicit list.
+ *)
+ count := 0;
+ seen := false;
+ NBD.add_meta_context nbd NBD.context_base_allocation;
+ let r = NBD.opt_set_meta_context_queries nbd [] (f 42) in
+ assert (r = 0);
+ assert (r = !count);
+ assert (not !seen);
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert (not m);
+
+ (* Request 2 with expectation of 1. *)
+ count := 0;
+ seen := false;
+ let r = NBD.opt_set_meta_context_queries nbd
+ ["x-nosuch:context"; NBD.context_base_allocation] (f 42)
in
+ assert (r = 1);
+ assert (r = !count);
+ assert !seen;
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert m;
+
+ (* Transition to transmission phase; our last set should remain active *)
+ NBD.set_request_meta_context nbd false;
+ NBD.opt_go nbd;
+ let m = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert m;
+
+ NBD.shutdown nbd
+
+let () = Gc.compact ()
diff --git a/tests/opt-set-meta-queries.c b/tests/opt-set-meta-queries.c
index 0b162607..2c3ee29a 100644
--- a/tests/opt-set-meta-queries.c
+++ b/tests/opt-set-meta-queries.c
@@ -17,6 +17,7 @@
*/
/* Test behavior of nbd_opt_set_meta_context_queries. */
+/* See also unit test 255 in the various language ports. */
#include <config.h>
diff --git a/tests/opt-set-meta.c b/tests/opt-set-meta.c
index 95f5a6d7..a4366d24 100644
--- a/tests/opt-set-meta.c
+++ b/tests/opt-set-meta.c
@@ -17,6 +17,7 @@
*/
/* Test behavior of nbd_opt_set_meta_context. */
+/* See also unit test 250 in the various language ports. */
#include <config.h>
diff --git a/golang/Makefile.am b/golang/Makefile.am
index 6adf6ae8..0808a639 100644
--- a/golang/Makefile.am
+++ b/golang/Makefile.am
@@ -36,6 +36,8 @@ source_files = \
libnbd_230_opt_info_test.go \
libnbd_240_opt_list_meta_test.go \
libnbd_245_opt_list_meta_queries_test.go \
+ libnbd_250_opt_set_meta_test.go \
+ libnbd_255_opt_set_meta_queries_test.go \
libnbd_300_get_size_test.go \
libnbd_400_pread_test.go \
libnbd_405_pread_structured_test.go \
diff --git a/golang/libnbd_250_opt_set_meta_test.go
b/golang/libnbd_250_opt_set_meta_test.go
new file mode 100644
index 00000000..1740d83e
--- /dev/null
+++ b/golang/libnbd_250_opt_set_meta_test.go
@@ -0,0 +1,276 @@
+/* libnbd golang tests
+ * Copyright (C) 2013-2022 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
+ */
+
+package libnbd
+
+import "testing"
+
+var set_count uint
+var set_seen bool
+
+func setmetaf(user_data int, name string) int {
+ if user_data != 42 {
+ panic("expected user_data == 42")
+ }
+ set_count++
+ if (name == context_base_allocation) {
+ set_seen = true
+ }
+ return 0
+}
+
+func Test250OptSetMeta(t *testing.T) {
+ /* First process, with structured replies. Get into negotiating state. */
+ h, err := Create()
+ if err != nil {
+ t.Fatalf("could not create handle: %s", err)
+ }
+ defer h.Close()
+
+ err = h.SetOptMode(true)
+ if err != nil {
+ t.Fatalf("could not set opt mode: %s", err)
+ }
+
+ err = h.ConnectCommand([]string{
+ "nbdkit", "-s", "--exit-with-parent",
"-v",
+ "memory", "size=1M",
+ })
+ if err != nil {
+ t.Fatalf("could not connect: %s", err)
+ }
+
+ /* No contexts negotiated yet; CanMeta should be error if any requested */
+ sr, err := h.GetStructuredRepliesNegotiated()
+ if err != nil {
+ t.Fatalf("could not check structured replies negotiated: %s", err)
+ }
+ if !sr {
+ t.Fatalf("unexpected structured replies state")
+ }
+ meta, err := h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+ err = h.AddMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ _, err = h.CanMetaContext(context_base_allocation)
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+
+ /* FIXME: Once OptStructuredReply exists, check that set before
+ * SR fails server-side, then enable SR for rest of process.
+ */
+
+ /* nbdkit does not match wildcard for SET, even though it does for LIST */
+ set_count = 0
+ set_seen = false
+ err = h.ClearMetaContexts()
+ if err != nil {
+ t.Fatalf("could not request clear_meta_contexts: %s", err)
+ }
+ err = h.AddMetaContext("base:")
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ r, err := h.OptSetMetaContext(func(name string) int {
+ return setmetaf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_set_meta_context: %s", err)
+ }
+ if r != set_count || r != 0 || set_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context")
+ }
+
+ /* Negotiating with no contexts is not an error, but selects nothing */
+ set_count = 0
+ set_seen = false
+ err = h.ClearMetaContexts()
+ if err != nil {
+ t.Fatalf("could not request clear_meta_contexts: %s", err)
+ }
+ r, err = h.OptSetMetaContext(func(name string) int {
+ return setmetaf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_set_meta_context: %s", err)
+ }
+ if r != set_count || r != 0 || set_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context")
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ /* Request 2 with expectation of 1; with SetRequestMetaContext off */
+ set_count = 0
+ set_seen = false
+ err = h.AddMetaContext("x-nosuch:context")
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ err = h.AddMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ err = h.SetRequestMetaContext(false)
+ if err != nil {
+ t.Fatalf("could not set_request_meta_context: %s", err)
+ }
+ r, err = h.OptSetMetaContext(func(name string) int {
+ return setmetaf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_set_meta_context: %s", err)
+ }
+ if r != 1 || r != set_count || !set_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context")
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ /* Transition to transmission phase; our last set should remain active */
+ err = h.ClearMetaContexts()
+ if err != nil {
+ t.Fatalf("could not request clear_meta_contexts: %s", err)
+ }
+ err = h.AddMetaContext("x-nosuch:context")
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ err = h.OptGo()
+ if err != nil {
+ t.Fatalf("could not request opt_go: %s", err)
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ /* Now too late to set; but should not lose earlier state */
+ set_count = 0
+ set_seen = false
+ _, err = h.OptSetMetaContext(func(name string) int {
+ return setmetaf(42, name)
+ })
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+ if set_count != 0 || set_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context")
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ err = h.Shutdown(nil)
+ if err != nil {
+ t.Fatalf("could not request shutdown: %s", err)
+ }
+
+ /* Second process, this time without structured replies server-side. */
+ h, err = Create()
+ if err != nil {
+ t.Fatalf("could not create handle: %s", err)
+ }
+ defer h.Close()
+
+ err = h.SetOptMode(true)
+ if err != nil {
+ t.Fatalf("could not set opt mode: %s", err)
+ }
+
+ err = h.AddMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+
+ err = h.ConnectCommand([]string{
+ "nbdkit", "-s", "--exit-with-parent",
"-v",
+ "memory", "size=1M", "--no-sr",
+ })
+ if err != nil {
+ t.Fatalf("could not connect: %s", err)
+ }
+
+ sr, err = h.GetStructuredRepliesNegotiated()
+ if err != nil {
+ t.Fatalf("could not check structured replies negotiated: %s", err)
+ }
+ if sr {
+ t.Fatalf("unexpected structured replies state")
+ }
+
+ /* Expect server-side failure here */
+ set_count = 0
+ set_seen = false
+ _, err = h.OptSetMetaContext(func(name string) int {
+ return setmetaf(42, name)
+ })
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+ if set_count != 0 || set_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context")
+ }
+ _, err = h.CanMetaContext(context_base_allocation)
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+
+ /* Even though CanMeta fails after failed SET, it returns 0 after go */
+ err = h.OptGo()
+ if err != nil {
+ t.Fatalf("could not request opt_go: %s", err)
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ err = h.Shutdown(nil)
+ if err != nil {
+ t.Fatalf("could not request shutdown: %s", err)
+ }
+}
diff --git a/golang/libnbd_255_opt_set_meta_queries_test.go
b/golang/libnbd_255_opt_set_meta_queries_test.go
new file mode 100644
index 00000000..96b37d76
--- /dev/null
+++ b/golang/libnbd_255_opt_set_meta_queries_test.go
@@ -0,0 +1,141 @@
+/* libnbd golang tests
+ * Copyright (C) 2013-2022 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
+ */
+
+package libnbd
+
+import "testing"
+
+var setq_count uint
+var setq_seen bool
+
+func setmetaqf(user_data int, name string) int {
+ if user_data != 42 {
+ panic("expected user_data == 42")
+ }
+ setq_count++
+ if (name == context_base_allocation) {
+ setq_seen = true
+ }
+ return 0
+}
+
+func Test255OptSetMetaQueries(t *testing.T) {
+ /* Get into negotiating state. */
+ h, err := Create()
+ if err != nil {
+ t.Fatalf("could not create handle: %s", err)
+ }
+ defer h.Close()
+
+ err = h.SetOptMode(true)
+ if err != nil {
+ t.Fatalf("could not set opt mode: %s", err)
+ }
+
+ err = h.ConnectCommand([]string{
+ "nbdkit", "-s", "--exit-with-parent",
"-v",
+ "memory", "size=1M",
+ })
+ if err != nil {
+ t.Fatalf("could not connect: %s", err)
+ }
+
+ /* nbdkit does not match wildcard for SET, even though it does for LIST */
+ setq_count = 0
+ setq_seen = false
+ r, err := h.OptSetMetaContextQueries([]string{"base:"},
+ func(name string) int {
+ return setmetaqf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_set_meta_context_queries: %s", err)
+ }
+ if r != setq_count || r != 0 || setq_seen {
+ t.Fatalf("unexpected count after opt_set_meta_context_queries")
+ }
+
+ /* Negotiating with no contexts is not an error, but selects nothing.
+ * An explicit empty list overrides a non-empty implicit list.
+ */
+ setq_count = 0
+ setq_seen = false
+ err = h.AddMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not request add_meta_context: %s", err)
+ }
+ r, err = h.OptSetMetaContextQueries([]string{}, func(name string) int {
+ return setmetaqf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_set_meta_context_queries: %s", err)
+ }
+ if r != setq_count || r != 0 || setq_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context_queries")
+ }
+ meta, err := h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ /* Request 2 with expectation of 1; with SetRequestMetaContext off */
+ setq_count = 0
+ setq_seen = false
+ r, err = h.OptSetMetaContextQueries([]string{
+ "x-nosuch:context", context_base_allocation},
+ func(name string) int {
+ return setmetaqf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_set_meta_context_queries: %s", err)
+ }
+ if r != 1 || r != setq_count || !setq_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context_queries")
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ /* Transition to transmission phase; our last set should remain active */
+ err = h.SetRequestMetaContext(false)
+ if err != nil {
+ t.Fatalf("could not set_request_meta_context: %s", err)
+ }
+ err = h.OptGo()
+ if err != nil {
+ t.Fatalf("could not request opt_go: %s", err)
+ }
+ meta, err = h.CanMetaContext(context_base_allocation)
+ if err != nil {
+ t.Fatalf("could not check can meta context: %s", err)
+ }
+ if !meta {
+ t.Fatalf("unexpected can meta context state")
+ }
+
+ err = h.Shutdown(nil)
+ if err != nil {
+ t.Fatalf("could not request shutdown: %s", err)
+ }
+}
--
2.37.3
Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 17/18] api: Add nbd_[aio_]opt_structured_reply()
Recent commits have lamented that we do not have a way to provoke
atypical ordering of option negotiation during server integration
testing. This commit adds nbd_opt_structured_reply() to make it
possible to fine-tune when structured replies are requested, and fills
in a couple of FIXME comments left in previous commits. The new API
can be called more than once; the testsuite is also enhanced with a
test that shows that nbdkit rejects the second attempt but libnbd
still remembers the first one succeeded (this requires moving the wipe
of h->structured_reply out of NBD_OPT_STRUCTURED_REPLY error handling,
and instead putting it into NBD_OPT_STARTTLS success, which becomes
important when a later patch adds nbd_opt_starttls()).
The C test changes depend on a new API, so they can't reasonably be
applied out of order. However, porting the FIXME changes of the
testsuite to other language bindings will be done in the next patch,
to ease review of this one. The test opt-structured-twice borders on
being an integration test (in fact, nbdkit commit 9418ec58 [upcoming
v1.34] changes nbdkit from accepting a second structured reply request
to instead diagnosing it as redundant; both behaviors are compatible
with the NBD spec), so that one will remain C only.
---
generator/API.ml | 70 ++++++++-
generator/states-newstyle-opt-starttls.c | 1 +
.../states-newstyle-opt-structured-reply.c | 22 ++-
generator/states-newstyle.c | 3 +
lib/opt.c | 44 ++++++
tests/Makefile.am | 5 +
tests/opt-list-meta.c | 29 +++-
tests/opt-set-meta.c | 50 ++++--
tests/opt-structured-twice.c | 145 ++++++++++++++++++
.gitignore | 1 +
10 files changed, 347 insertions(+), 23 deletions(-)
create mode 100644 tests/opt-structured-twice.c
diff --git a/generator/API.ml b/generator/API.ml
index 9ffe6516..1c9f890a 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -794,9 +794,12 @@ "set_request_structured_replies", {
L<nbd_can_meta_context(3)> or L<nbd_can_df(3)> can return true.
However,
for integration testing, it can be useful to clear this flag
rather than find a way to alter the server to fail the negotiation
-request.";
+request. It is also useful to set this to false prior to using
+L<nbd_set_opt_mode(3)> if it is desired to control when to send
+L<nbd_opt_structured_reply(3)> during negotiation.";
see_also = [Link "get_request_structured_replies";
Link "set_handshake_flags"; Link
"set_strict_mode";
+ Link "opt_structured_reply";
Link "get_structured_replies_negotiated";
Link "can_meta_context"; Link "can_df"];
};
@@ -824,9 +827,12 @@ "get_structured_replies_negotiated", {
shortdesc = "see if structured replies are in use";
longdesc = "\
After connecting you may call this to find out if the connection is
-using structured replies.";
+using structured replies. Note that this setting is sticky; this
+can return true even after a second L<nbd_opt_structured_reply(3)>
+returns false because the server detected a duplicate request.";
see_also = [Link "set_request_structured_replies";
Link "get_request_structured_replies";
+ Link "opt_structured_reply";
Link "get_protocol"];
};
@@ -1086,6 +1092,12 @@ "set_opt_mode", {
newstyle server. This setting has no effect when connecting to an
oldstyle server.
+Note that libnbd defaults to attempting
+C<NBD_OPT_STRUCTURED_REPLY> before letting you control remaining
+negotiation steps; if you need control over this step as well,
+first set L<nbd_set_request_structured_replies(3)> to false before
+starting the connection attempt.
+
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
@@ -1099,6 +1111,8 @@ "set_opt_mode", {
Link "opt_abort"; Link "opt_go"; Link
"opt_list";
Link "opt_info"; Link
"opt_list_meta_context";
Link "opt_set_meta_context";
+ Link "opt_structured_reply";
+ Link "set_request_structured_replies";
Link "aio_connect"];
};
@@ -1152,6 +1166,32 @@ "opt_abort", {
see_also = [Link "set_opt_mode"; Link "aio_opt_abort";
Link "opt_go"];
};
+ "opt_structured_reply", {
+ default_call with
+ args = []; ret = RBool;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server to enable structured replies";
+ longdesc = "\
+Request that the server use structured replies, by sending
+C<NBD_OPT_STRUCTURED_REPLY>. This can only be used if
+L<nbd_set_opt_mode(3)> enabled option mode; furthermore, libnbd
+defaults to automatically requesting this unless you use
+L<nbd_set_request_structured_replies(3)> prior to connecting.
+This function is mainly useful for integration testing of corner
+cases in server handling.
+
+This function returns true if the server replies with success,
+false if the server replies with an error, and fails only if
+the server does not reply (such as for a loss of connection).
+Note that some servers fail a second request as redundant;
+libnbd assumes that once one request has succeeded, then
+structured replies are supported (as visible by
+L<nbd_get_structured_replies_negotiated(3)>) regardless if
+later calls to this function return false.";
+ see_also = [Link "set_opt_mode"; Link
"aio_opt_structured_reply";
+ Link "opt_go"; Link
"set_request_structured_replies"]
+ };
+
"opt_list", {
default_call with
args = [ Closure list_closure ]; ret = RInt;
@@ -2775,6 +2815,30 @@ "aio_opt_abort", {
see_also = [Link "set_opt_mode"; Link "opt_abort"];
};
+ "aio_opt_structured_reply", {
+ default_call with
+ args = [];
+ optargs = [ OClosure completion_closure ];
+ ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server to enable structured replies";
+ longdesc = "\
+Request that the server use structured replies, by sending
+C<NBD_OPT_STRUCTURED_REPLY>. This behaves like the synchronous
+counterpart L<nbd_opt_structured_reply(3)>, except that it does
+not wait for the server's response.
+
+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_structured_reply"];
+ };
+
"aio_opt_list", {
default_call with
args = [ Closure list_closure ];
@@ -3678,6 +3742,8 @@ let first_version
"opt_set_meta_context_queries", (1, 16);
"aio_opt_set_meta_context", (1, 16);
"aio_opt_set_meta_context_queries", (1, 16);
+ "opt_structured_reply", (1, 16);
+ "aio_opt_structured_reply", (1, 16);
(* 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-starttls.c
b/generator/states-newstyle-opt-starttls.c
index a9383ced..afaad85a 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -72,6 +72,7 @@ NEWSTYLE.OPT_STARTTLS.CHECK_REPLY:
reply = be32toh (h->sbuf.or.option_reply.reply);
switch (reply) {
case NBD_REP_ACK:
+ h->structured_replies = false;
new_sock = nbd_internal_crypto_create_session (h, h->sock);
if (new_sock == NULL) {
SET_NEXT_STATE (%.DEAD);
diff --git a/generator/states-newstyle-opt-structured-reply.c
b/generator/states-newstyle-opt-structured-reply.c
index 0ea0fd43..300cf20b 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -21,12 +21,17 @@
STATE_MACHINE {
NEWSTYLE.OPT_STRUCTURED_REPLY.START:
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
- if (!h->request_sr) {
- if (h->opt_mode)
- SET_NEXT_STATE (%.NEGOTIATING);
- else
- SET_NEXT_STATE (%^OPT_META_CONTEXT.START);
- return 0;
+ if (h->opt_current == NBD_OPT_STRUCTURED_REPLY)
+ assert (h->opt_mode);
+ else {
+ assert (CALLBACK_IS_NULL(h->opt_cb.completion));
+ if (!h->request_sr) {
+ if (h->opt_mode)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%^OPT_META_CONTEXT.START);
+ return 0;
+ }
}
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
@@ -69,12 +74,14 @@ NEWSTYLE.OPT_STRUCTURED_REPLY.RECV_REPLY_PAYLOAD:
NEWSTYLE.OPT_STRUCTURED_REPLY.CHECK_REPLY:
uint32_t reply;
+ int err = ENOTSUP;
reply = be32toh (h->sbuf.or.option_reply.reply);
switch (reply) {
case NBD_REP_ACK:
debug (h, "negotiated structured replies on this connection");
h->structured_replies = true;
+ err = 0;
break;
default:
if (handle_reply_error (h) == -1) {
@@ -83,7 +90,6 @@ NEWSTYLE.OPT_STRUCTURED_REPLY.CHECK_REPLY:
}
debug (h, "structured replies are not supported by this server");
- h->structured_replies = false;
break;
}
@@ -92,6 +98,8 @@ NEWSTYLE.OPT_STRUCTURED_REPLY.CHECK_REPLY:
SET_NEXT_STATE (%.NEGOTIATING);
else
SET_NEXT_STATE (%^OPT_META_CONTEXT.START);
+ CALL_CALLBACK (h->opt_cb.completion, &err);
+ nbd_internal_free_option (h);
return 0;
} /* END STATE MACHINE */
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 60e0bc50..4652bc21 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -143,6 +143,9 @@ NEWSTYLE.START:
case NBD_OPT_SET_META_CONTEXT:
SET_NEXT_STATE (%OPT_META_CONTEXT.START);
return 0;
+ case NBD_OPT_STRUCTURED_REPLY:
+ SET_NEXT_STATE (%OPT_STRUCTURED_REPLY.START);
+ return 0;
case 0:
break;
default:
diff --git a/lib/opt.c b/lib/opt.c
index ee036235..1b18920f 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -127,6 +127,31 @@ nbd_unlocked_opt_abort (struct nbd_handle *h)
return wait_for_option (h);
}
+/* Issue NBD_OPT_STRUCTURED_REPLY and wait for the reply. */
+int
+nbd_unlocked_opt_structured_reply (struct nbd_handle *h)
+{
+ int err;
+ nbd_completion_callback c = { .callback = go_complete, .user_data = &err
};
+ int r = nbd_unlocked_aio_opt_structured_reply (h, &c);
+
+ if (r == -1)
+ return r;
+
+ r = wait_for_option (h);
+ if (r == 0) {
+ if (nbd_internal_is_state_negotiating (get_next_state (h)))
+ r = err == 0;
+ else {
+ assert (nbd_internal_is_state_dead (get_next_state (h)));
+ set_error (err,
+ "failed to get response to opt_structured_reply
request");
+ r = -1;
+ }
+ }
+ return r;
+}
+
struct list_helper {
int count;
nbd_list_callback list;
@@ -300,6 +325,25 @@ nbd_unlocked_aio_opt_abort (struct nbd_handle *h)
return 0;
}
+/* Issue NBD_OPT_STRUCTURED_REPLY without waiting. */
+int
+nbd_unlocked_aio_opt_structured_reply (struct nbd_handle *h,
+ nbd_completion_callback *complete)
+{
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
+ return -1;
+ }
+
+ h->opt_current = NBD_OPT_STRUCTURED_REPLY;
+ h->opt_cb.completion = *complete;
+ SET_CALLBACK_TO_NULL (*complete);
+
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ 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,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ec95495e..49ea6864 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -217,6 +217,7 @@ check_PROGRAMS += \
opt-list-meta-queries \
opt-set-meta \
opt-set-meta-queries \
+ opt-structured-twice \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -286,6 +287,7 @@ TESTS += \
opt-list-meta-queries \
opt-set-meta \
opt-set-meta-queries \
+ opt-structured-twice \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -557,6 +559,9 @@ opt_set_meta_LDADD = $(top_builddir)/lib/libnbd.la
opt_set_meta_queries_SOURCES = opt-set-meta-queries.c
opt_set_meta_queries_LDADD = $(top_builddir)/lib/libnbd.la
+opt_structured_twice_SOURCES = opt-structured-twice.c
+opt_structured_twice_LDADD = $(top_builddir)/lib/libnbd.la
+
connect_systemd_socket_activation_SOURCES = \
connect-systemd-socket-activation.c \
requires.c \
diff --git a/tests/opt-list-meta.c b/tests/opt-list-meta.c
index 05c1a5eb..b32dbf90 100644
--- a/tests/opt-list-meta.c
+++ b/tests/opt-list-meta.c
@@ -269,7 +269,34 @@ main (int argc, char *argv[])
}
}
- /* FIXME: Once nbd_opt_structured_reply() exists, use it here and retry. */
+ /* Now enable structured replies, and a retry should pass. */
+ r = nbd_opt_structured_reply (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "expecting structured replies to be
supported\n");
+ exit (EXIT_FAILURE);
+ }
+
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_list_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data =
&p});
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != p.count) {
+ fprintf (stderr, "inconsistent return value %d, expected %d\n",
+ r, p.count);
+ exit (EXIT_FAILURE);
+ }
+ if (r < 1 || !p.seen) {
+ fprintf (stderr, "server did not reply with base:allocation\n");
+ exit (EXIT_FAILURE);
+ }
nbd_opt_abort (nbd);
nbd_close (nbd);
diff --git a/tests/opt-set-meta.c b/tests/opt-set-meta.c
index a4366d24..48fce9bd 100644
--- a/tests/opt-set-meta.c
+++ b/tests/opt-set-meta.c
@@ -59,37 +59,61 @@ main (int argc, char *argv[])
char *args[] = { "nbdkit", "-s",
"--exit-with-parent", "-v",
"memory", "size=1M", NULL, NULL };
- /* First process, with structured replies. Get into negotiating state. */
+ /* First process, delay structured replies. Get into negotiating state. */
nbd = nbd_create ();
if (nbd == NULL ||
nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_set_request_structured_replies (nbd, false) == -1 ||
nbd_connect_command (nbd, args) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
/* No contexts negotiated yet; can_meta should be error if any requested */
+ if ((r = nbd_get_structured_replies_negotiated (nbd)) != 0) {
+ fprintf (stderr, "structured replies got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
+ fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != -1) {
+ fprintf (stderr, "can_meta_context got %d, expected -1\n", r);
+ exit (EXIT_FAILURE);
+ }
+
+ /* SET cannot succeed until SR is negotiated. */
+ p = (struct progress) { .count = 0 };
+ r = nbd_opt_set_meta_context (nbd,
+ (nbd_context_callback) { .callback = check,
+ .user_data = &p});
+ if (r != -1 || p.count || p.seen) {
+ fprintf (stderr, "expecting failure of set without SR\n");
+ exit (EXIT_FAILURE);
+ }
+ r = nbd_opt_structured_reply (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "expecting server support for SR\n");
+ exit (EXIT_FAILURE);
+ }
if ((r = nbd_get_structured_replies_negotiated (nbd)) != 1) {
fprintf (stderr, "structured replies got %d, expected 1\n", r);
exit (EXIT_FAILURE);
}
- if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) {
- fprintf (stderr, "can_meta_context got %d, expected 0\n", r);
- exit (EXIT_FAILURE);
- }
- if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- exit (EXIT_FAILURE);
- }
if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != -1) {
fprintf (stderr, "can_meta_context got %d, expected -1\n", r);
exit (EXIT_FAILURE);
}
- /* FIXME: Once nbd_opt_structured_reply exists, check that set before
- * SR fails server-side, then enable SR for rest of process.
- */
-
/* nbdkit does not match wildcard for SET, even though it does for LIST */
if (nbd_clear_meta_contexts (nbd) == -1 ||
nbd_add_meta_context (nbd, "base:") == -1) {
diff --git a/tests/opt-structured-twice.c b/tests/opt-structured-twice.c
new file mode 100644
index 00000000..5924d22e
--- /dev/null
+++ b/tests/opt-structured-twice.c
@@ -0,0 +1,145 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2022 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
+ */
+
+/* Demonstrate low-level use of nbd_opt_structured_reply(). */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/stat.h>
+
+#include <libnbd.h>
+
+
+static int
+check_extent (void *data, const char *metacontext, uint64_t offset,
+ uint32_t *entries, size_t nr_entries, int *error)
+{
+ /* If we got here, structured replies were negotiated. */
+ bool *seen = data;
+
+ *seen = true;
+ return 0;
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ const char *cmd[] = {
+ "nbdkit", "-s", "-v",
"--exit-with-parent", "memory", "1048576", NULL
+ };
+ int r;
+ bool extents_worked = false;
+
+ nbd = nbd_create ();
+ if (nbd == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ /* Connect to the server in opt mode, without structured replies. */
+ if (nbd_set_opt_mode (nbd, true) == -1 ||
+ nbd_set_request_structured_replies (nbd, false) == -1 ||
+ nbd_connect_command (nbd, (char **) cmd) == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ r = nbd_get_structured_replies_negotiated (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 0) {
+ fprintf (stderr, "%s: not expecting structured replies yet\n",
argv[0]);
+ exit (EXIT_FAILURE);
+ }
+
+ /* First request should succeed. */
+ r = nbd_opt_structured_reply (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "%s: expecting structured replies\n", argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ r = nbd_get_structured_replies_negotiated (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "%s: expecting structured replies\n", argv[0]);
+ exit (EXIT_FAILURE);
+ }
+
+ /* nbdkit 1.32 allows a second request, nbdkit 1.34 diagnoses it. */
+ r = nbd_opt_structured_reply (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ printf ("%s: server's response to second request: %s\n",
argv[0],
+ r ? "accepted" : "rejected");
+
+ /* Regardless of whether second request passed, structured replies were
+ * negotiated, so we should be able to do block status.
+ */
+ r = nbd_get_structured_replies_negotiated (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "%s: expecting structured replies\n", argv[0]);
+ exit (EXIT_FAILURE);
+ }
+
+ if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1 ||
+ nbd_opt_go (nbd) == -1 ||
+ (r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "%s: expecting base:allocation support\n",
argv[0]);
+ exit (EXIT_FAILURE);
+ }
+
+ if (nbd_block_status (nbd, 65536, 0,
+ (nbd_extent_callback) { .callback = check_extent,
+ .user_data =
&extents_worked },
+ 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (!extents_worked) {
+ fprintf (stderr, "%s: expecting block_status success\n",
argv[0]);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index 50e4616e..37166b73 100644
--- a/.gitignore
+++ b/.gitignore
@@ -236,6 +236,7 @@ Makefile.in
/tests/opt-list-meta-queries
/tests/opt-set-meta
/tests/opt-set-meta-queries
+/tests/opt-structured-twice
/tests/pki/
/tests/pread-initialize
/tests/private-data
--
2.37.3
Eric Blake
2022-Sep-26 22:06 UTC
[Libguestfs] [libnbd PATCH v3 18/18] tests: Language port of nbd_opt_structured_reply() tests
As promised in the previous patch, this addresses the FIXME comments
added in language port tests earlier in the series now that we can
control structured replies when we want.
---
python/t/240-opt-list-meta.py | 11 ++++++-
python/t/250-opt-set-meta.py | 20 ++++++++----
ocaml/tests/test_240_opt_list_meta.ml | 11 ++++++-
ocaml/tests/test_250_opt_set_meta.ml | 29 ++++++++++++++---
golang/libnbd_240_opt_list_meta_test.go | 21 ++++++++++++-
golang/libnbd_250_opt_set_meta_test.go | 41 ++++++++++++++++++++++---
6 files changed, 114 insertions(+), 19 deletions(-)
diff --git a/python/t/240-opt-list-meta.py b/python/t/240-opt-list-meta.py
index 8cafdd54..c9170689 100644
--- a/python/t/240-opt-list-meta.py
+++ b/python/t/240-opt-list-meta.py
@@ -124,6 +124,15 @@ def must_fail(f, *args, **kwds):
assert h.stats_bytes_sent() > bytes
print("ignoring failure from old server: %s" % ex.string)
-# FIXME: Once nbd_opt_structured_reply exists, use it here and retry.
+# Now enable structured replies, and a retry should pass.
+r = h.opt_structured_reply()
+assert r is True
+
+count = 0
+seen = False
+r = h.opt_list_meta_context(lambda *args: f(42, *args))
+assert r == count
+assert r >= 1
+assert seen is True
h.opt_abort()
diff --git a/python/t/250-opt-set-meta.py b/python/t/250-opt-set-meta.py
index c543a5d6..0d08bc0a 100644
--- a/python/t/250-opt-set-meta.py
+++ b/python/t/250-opt-set-meta.py
@@ -39,21 +39,29 @@ def must_fail(f, *args, **kwds):
pass
-# First process, with structured replies. Get into negotiating state.
+# First process, delay structured replies. Get into negotiating state.
h = nbd.NBD()
h.set_opt_mode(True)
+h.set_request_structured_replies(False)
h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
"memory", "size=1M"])
# No contexts negotiated yet; can_meta should be error if any requested
+assert h.get_structured_replies_negotiated() is False
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
+h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
+
+# SET cannot succeed until SR is negotiated.
+count = 0
+seen = False
+must_fail(h.opt_set_meta_context, lambda *args: f(42, *args))
+assert count == 0
+assert seen is False
+assert h.opt_structured_reply() is True
assert h.get_structured_replies_negotiated() is True
-assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False
-h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION)
-# FIXME: Once nbd_opt_structured_reply exists, check that set before
-# SR fails server-side, then enable SR for rest of process.
-
# nbdkit does not match wildcard for SET, even though it does for LIST
count = 0
seen = False
diff --git a/ocaml/tests/test_240_opt_list_meta.ml
b/ocaml/tests/test_240_opt_list_meta.ml
index 18582c44..f3487c27 100644
--- a/ocaml/tests/test_240_opt_list_meta.ml
+++ b/ocaml/tests/test_240_opt_list_meta.ml
@@ -135,7 +135,16 @@ let
printf "ignoring failure from old server %s\n" errstr
);
- (* FIXME: Once nbd_opt_structured_reply exists, use it here and retry. *)
+ (* Now enable structured replies, and a retry should pass. *)
+ let sr = NBD.opt_structured_reply nbd in
+ assert sr;
+
+ count := 0;
+ seen := false;
+ let r = NBD.opt_list_meta_context nbd (f 42) in
+ assert (r = !count);
+ assert (r >= 1);
+ assert !seen;
NBD.opt_abort nbd
diff --git a/ocaml/tests/test_250_opt_set_meta.ml
b/ocaml/tests/test_250_opt_set_meta.ml
index f35012fd..0e1c17b5 100644
--- a/ocaml/tests/test_250_opt_set_meta.ml
+++ b/ocaml/tests/test_250_opt_set_meta.ml
@@ -27,16 +27,17 @@ let
0
let () - (* First process, with structured replies. Get into negotiating
state. *)
+ (* First process, delay structured replies. Get into negotiating state. *)
let nbd = NBD.create () in
NBD.set_opt_mode nbd true;
+ NBD.set_request_structured_replies nbd false;
NBD.connect_command nbd
["nbdkit"; "-s";
"--exit-with-parent"; "-v";
"memory"; "size=1M"];
(* No contexts negotiated yet; can_meta should be error if any requested *)
let sr = NBD.get_structured_replies_negotiated nbd in
- assert sr;
+ assert (not sr);
let m = NBD.can_meta_context nbd NBD.context_base_allocation in
assert (not m);
NBD.add_meta_context nbd NBD.context_base_allocation;
@@ -47,9 +48,27 @@ let
NBD.Error (errstr, errno) -> ()
);
- (* FIXME: Once nbd_opt_structured_reply exists, check that set before
- * SR fails server-side, then enable SR for rest of process.
- *)
+ (* SET cannot succeed until SR is negotiated. *)
+ count := 0;
+ seen := false;
+ (try
+ let _ = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
+ assert (!count = 0);
+ assert (not !seen);
+ let sr = NBD.opt_structured_reply nbd in
+ assert sr;
+ let sr = NBD.get_structured_replies_negotiated nbd in
+ assert sr;
+ (try
+ let _ = NBD.can_meta_context nbd NBD.context_base_allocation in
+ assert false
+ with
+ NBD.Error (errstr, errno) -> ()
+ );
(* nbdkit does not match wildcard for SET, even though it does for LIST *)
count := 0;
diff --git a/golang/libnbd_240_opt_list_meta_test.go
b/golang/libnbd_240_opt_list_meta_test.go
index 9d6f6f28..9b0b4144 100644
--- a/golang/libnbd_240_opt_list_meta_test.go
+++ b/golang/libnbd_240_opt_list_meta_test.go
@@ -256,7 +256,26 @@ func Test240OptListMeta(t *testing.T) {
t.Fatalf("unexpected count after opt_list_meta_context")
}
- /* FIXME: Once nbd_opt_structured_reply exists, use it here and retry. */
+ /* Now enable structured replies, and a retry should pass. */
+ sr, err := h.OptStructuredReply()
+ if err != nil {
+ t.Fatalf("could not request opt_structured_reply: %s", err)
+ }
+ if !sr {
+ t.Fatalf("structured replies not enabled: %s", err)
+ }
+
+ list_count = 0
+ list_seen = false
+ r, err = h.OptListMetaContext(func(name string) int {
+ return listmetaf(42, name)
+ })
+ if err != nil {
+ t.Fatalf("could not request opt_list_meta_context: %s", err)
+ }
+ if r < 1 || r != list_count || !list_seen {
+ t.Fatalf("unexpected count after opt_list_meta_context")
+ }
err = h.OptAbort()
if err != nil {
diff --git a/golang/libnbd_250_opt_set_meta_test.go
b/golang/libnbd_250_opt_set_meta_test.go
index 1740d83e..cf036f45 100644
--- a/golang/libnbd_250_opt_set_meta_test.go
+++ b/golang/libnbd_250_opt_set_meta_test.go
@@ -35,7 +35,7 @@ func setmetaf(user_data int, name string) int {
}
func Test250OptSetMeta(t *testing.T) {
- /* First process, with structured replies. Get into negotiating state. */
+ /* First process, delay structured replies. Get into negotiating state. */
h, err := Create()
if err != nil {
t.Fatalf("could not create handle: %s", err)
@@ -46,6 +46,10 @@ func Test250OptSetMeta(t *testing.T) {
if err != nil {
t.Fatalf("could not set opt mode: %s", err)
}
+ err = h.SetRequestStructuredReplies(false)
+ if err != nil {
+ t.Fatalf("could not set opt mode: %s", err)
+ }
err = h.ConnectCommand([]string{
"nbdkit", "-s", "--exit-with-parent",
"-v",
@@ -60,7 +64,7 @@ func Test250OptSetMeta(t *testing.T) {
if err != nil {
t.Fatalf("could not check structured replies negotiated: %s", err)
}
- if !sr {
+ if sr {
t.Fatalf("unexpected structured replies state")
}
meta, err := h.CanMetaContext(context_base_allocation)
@@ -79,9 +83,36 @@ func Test250OptSetMeta(t *testing.T) {
t.Fatalf("expected error")
}
- /* FIXME: Once OptStructuredReply exists, check that set before
- * SR fails server-side, then enable SR for rest of process.
- */
+ /* SET cannot succeed until SR is negotiated. */
+ set_count = 0
+ set_seen = false
+ _, err = h.OptSetMetaContext(func(name string) int {
+ return setmetaf(42, name)
+ })
+ if err == nil {
+ t.Fatalf("expected error")
+ }
+ if set_count != 0 || set_seen {
+ t.Fatalf("unexpected set_count after opt_set_meta_context")
+ }
+ sr, err = h.OptStructuredReply()
+ if err != nil {
+ t.Fatalf("could not trigger opt_structured_reply: %s", err)
+ }
+ if !sr {
+ t.Fatalf("unexpected structured replies state")
+ }
+ sr, err = h.GetStructuredRepliesNegotiated()
+ if err != nil {
+ t.Fatalf("could not check structured replies negotiated: %s", err)
+ }
+ if !sr {
+ t.Fatalf("unexpected structured replies state")
+ }
+ _, err = h.CanMetaContext(context_base_allocation)
+ if err == nil {
+ t.Fatalf("expected error")
+ }
/* nbdkit does not match wildcard for SET, even though it does for LIST */
set_count = 0
--
2.37.3