Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 00/12] Improve NBD_OPT_ control
v1 was here (under the subject Smarter nbd_opt_info) https://listman.redhat.com/archives/libguestfs/2022-August/029641.html Since then, I've done a lot. The original patch 1/2 is now split across 7/12 and 9/12; while original patch 2/2 is expanded across several patches to test more scenarios. Several new prerequisite patches were added to address smaller issues one at a time and with better commit messages. Where possible, I have kept tests separate from the patch introducing a fix to allow temporary reordering at that point in the series to prove the test catches the issue. It was not possible on tests for new APIs, nor on patch 2/12 where the only test I could come up with involves hacking a one-off nbdkit that does not behave like a normal server; it was also hard to justify on 3/12 where reordering the patches results in a test skip rather than failure, where distinguishing the difference requires reading logs to see whether an API failed client-side or server-side. Maybe we want to add an API to track how many packets are sent over the wire (client->server, and server->client), to make it easier to actually catch when things are filtered client-side, or how often a server uses multiple reply chunks to answer a single client request. I've also fixed a bug in nbd_can_meta_context that was discovered while responding to v1 reviews, and completed part of the work at adding new APIs hinted at earlier (but still to add: APIs for nbd_opt_structured_replies and nbd_opt_starttls). Eric Blake (12): internal: Use vector instead of linked list for meta_contexts api: Fix nbd_can_meta_context if NBD_OPT_SET_META_CONTEXT fails api: Allow nbd_opt_list_meta_context without SR 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 tests: Language port of nbd_opt_set_meta_context() tests lib/internal.h | 19 +- generator/API.ml | 174 ++++++++++++- generator/states-newstyle-opt-go.c | 1 + generator/states-newstyle-opt-meta-context.c | 77 +++--- generator/states-newstyle.c | 1 + generator/states-reply-structured.c | 11 +- lib/flags.c | 27 +- lib/handle.c | 26 ++ lib/opt.c | 53 +++- lib/rw.c | 2 +- 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 | 29 ++- python/t/250-opt-set-meta.py | 126 ++++++++++ ocaml/tests/Makefile.am | 5 +- 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 | 34 ++- ocaml/tests/test_250_opt_set_meta.ml | 150 +++++++++++ tests/Makefile.am | 9 + tests/opt-info.c | 91 ++++++- tests/opt-list-meta.c | 104 +++++++- tests/opt-set-meta | 210 ++++++++++++++++ tests/opt-set-meta.c | 236 ++++++++++++++++++ .gitignore | 1 + 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 | 106 ++++++-- golang/libnbd_250_opt_set_meta_test.go | 248 +++++++++++++++++++ info/list.c | 3 +- info/show.c | 3 +- 35 files changed, 1813 insertions(+), 154 deletions(-) create mode 100644 python/t/250-opt-set-meta.py create mode 100644 ocaml/tests/test_250_opt_set_meta.ml create mode 100755 tests/opt-set-meta create mode 100644 tests/opt-set-meta.c create mode 100644 golang/libnbd_250_opt_set_meta_test.go -- 2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 01/12] internal: Use vector instead of linked list for meta_contexts
Instead of open-coding a linked list, we can utilize our vector.h
helpers. No semantic change intended.
---
lib/internal.h | 17 +++++----
generator/states-newstyle-opt-meta-context.c | 36 ++++++++------------
generator/states-reply-structured.c | 10 +++---
lib/flags.c | 19 ++++-------
lib/rw.c | 2 +-
5 files changed, 35 insertions(+), 49 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 0a61b15..d601d5d 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -64,10 +64,15 @@
*/
#define MAX_REQUEST_SIZE (64 * 1024 * 1024)
-struct meta_context;
struct socket;
struct command;
+struct meta_context {
+ char *name; /* Name of meta context. */
+ uint32_t context_id; /* Context ID negotiated with the server. */
+};
+DEFINE_VECTOR_TYPE(meta_vector, struct meta_context);
+
struct export {
char *name;
char *description;
@@ -174,8 +179,8 @@ struct nbd_handle {
bool structured_replies; /* If we negotiated NBD_OPT_STRUCTURED_REPLY */
- /* Linked list of negotiated metadata contexts. */
- struct meta_context *meta_contexts;
+ /* Vector of negotiated metadata contexts. */
+ meta_vector meta_contexts;
/* The socket or a wrapper if using GnuTLS. */
struct socket *sock;
@@ -311,12 +316,6 @@ struct nbd_handle {
bool tls_shut_writes; /* Used by lib/crypto.c to track disconnect. */
};
-struct meta_context {
- struct meta_context *next; /* Linked list. */
- char *name; /* Name of meta context. */
- uint32_t context_id; /* Context ID negotiated with the server. */
-};
-
struct socket_ops {
ssize_t (*recv) (struct nbd_handle *h,
struct socket *sock, void *buf, size_t len);
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 30b9617..8f4dee2 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -1,5 +1,5 @@
/* nbd client library in userspace: state machine
- * 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
@@ -44,7 +44,7 @@ STATE_MACHINE {
}
}
- assert (h->meta_contexts == NULL);
+ assert (h->meta_contexts.len == 0);
/* Calculate the length of the option request data. */
len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
@@ -176,7 +176,7 @@ STATE_MACHINE {
uint32_t reply;
uint32_t len;
const size_t maxpayload = sizeof h->sbuf.or.payload.context;
- struct meta_context *meta_context;
+ struct meta_context meta_context;
uint32_t opt;
int err = 0;
@@ -202,33 +202,27 @@ STATE_MACHINE {
debug (h, "skipping too large meta context");
else {
assert (len > sizeof
h->sbuf.or.payload.context.context.context_id);
- meta_context = malloc (sizeof *meta_context);
- if (meta_context == NULL) {
- set_error (errno, "malloc");
- SET_NEXT_STATE (%.DEAD);
- return 0;
- }
- meta_context->context_id + meta_context.context_id
be32toh (h->sbuf.or.payload.context.context.context_id);
/* String payload is not NUL-terminated. */
- meta_context->name = strndup (h->sbuf.or.payload.context.str,
- len - sizeof meta_context->context_id);
- if (meta_context->name == NULL) {
+ meta_context.name = strndup (h->sbuf.or.payload.context.str,
+ len - sizeof meta_context.context_id);
+ if (meta_context.name == NULL) {
set_error (errno, "strdup");
SET_NEXT_STATE (%.DEAD);
- free (meta_context);
return 0;
}
debug (h, "negotiated %s with context ID %" PRIu32,
- meta_context->name, meta_context->context_id);
+ meta_context.name, meta_context.context_id);
if (opt == NBD_OPT_LIST_META_CONTEXT) {
- CALL_CALLBACK (h->opt_cb.fn.context, meta_context->name);
- free (meta_context->name);
- free (meta_context);
+ CALL_CALLBACK (h->opt_cb.fn.context, meta_context.name);
+ free (meta_context.name);
}
- else {
- meta_context->next = h->meta_contexts;
- h->meta_contexts = meta_context;
+ else if (meta_vector_append (&h->meta_contexts, meta_context) ==
-1) {
+ set_error (errno, "realloc");
+ free (meta_context.name);
+ SET_NEXT_STATE (%.DEAD);
+ return 0;
}
}
SET_NEXT_STATE (%PREPARE_FOR_REPLY);
diff --git a/generator/states-reply-structured.c
b/generator/states-reply-structured.c
index 19d9fb2..aaf75b8 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -454,18 +454,16 @@ STATE_MACHINE {
/* Look up the context ID. */
context_id = h->bs_entries[0];
- for (meta_context = h->meta_contexts;
- meta_context;
- meta_context = meta_context->next)
- if (context_id == meta_context->context_id)
+ for (i = 0; i < h->meta_contexts.len; ++i)
+ if (context_id == h->meta_contexts.ptr[i].context_id)
break;
- if (meta_context) {
+ if (i < h->meta_contexts.len) {
/* Call the caller's extent function. */
int error = cmd->error;
if (CALL_CALLBACK (cmd->cb.fn.extent,
- meta_context->name, cmd->offset,
+ h->meta_contexts.ptr[i].name, cmd->offset,
&h->bs_entries[1], (length-4) / 4,
&error) == -1)
if (cmd->error == 0)
diff --git a/lib/flags.c b/lib/flags.c
index 44d61c8..87c20ee 100644
--- a/lib/flags.c
+++ b/lib/flags.c
@@ -33,16 +33,13 @@
void
nbd_internal_reset_size_and_flags (struct nbd_handle *h)
{
- struct meta_context *m, *m_next;
+ size_t i;
h->exportsize = 0;
h->eflags = 0;
- for (m = h->meta_contexts; m != NULL; m = m_next) {
- m_next = m->next;
- free (m->name);
- free (m);
- }
- h->meta_contexts = NULL;
+ for (i = 0; i < h->meta_contexts.len; ++i)
+ free (h->meta_contexts.ptr[i].name);
+ meta_vector_reset (&h->meta_contexts);
h->block_minimum = 0;
h->block_preferred = 0;
h->block_maximum = 0;
@@ -195,7 +192,7 @@ nbd_unlocked_can_cache (struct nbd_handle *h)
int
nbd_unlocked_can_meta_context (struct nbd_handle *h, const char *name)
{
- struct meta_context *meta_context;
+ size_t i;
if (h->eflags == 0) {
set_error (EINVAL, "server has not returned export flags, "
@@ -203,10 +200,8 @@ nbd_unlocked_can_meta_context (struct nbd_handle *h, const
char *name)
return -1;
}
- for (meta_context = h->meta_contexts;
- meta_context;
- meta_context = meta_context->next)
- if (strcmp (meta_context->name, name) == 0)
+ for (i = 0; i < h->meta_contexts.len; ++i)
+ if (strcmp (h->meta_contexts.ptr[i].name, name) == 0)
return 1;
return 0;
}
diff --git a/lib/rw.c b/lib/rw.c
index f32481a..81f8f35 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -482,7 +482,7 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
return -1;
}
- if (h->meta_contexts == NULL) {
+ if (h->meta_contexts.len == 0) {
set_error (ENOTSUP, "did not negotiate any metadata contexts, "
"either you did not call nbd_add_meta_context before
"
"connecting or the server does not support it");
--
2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 02/12] api: Fix nbd_can_meta_context if NBD_OPT_SET_META_CONTEXT fails
If a server replies to NBD_OPT_SET_META_CONTEXT first with
NBD_REP_META_CONTEXT, then with NBD_REP_ERR_*, we have already
modified h->meta_contexts, but fail to clean it up before moving on to
OPT_GO in the OPT_META_CONTEXT.CHECK_REPLY's default: handler (this
corner-case bug has been present since we got meta context support
working back in commits a97e2779 and 1b560b62). Per the NBD spec,
because the overall SET_META_CONTEXT did not succeed, we should
therefore not attempt NBD_CMD_BLOCK_STATUS; but because
h->meta_contexts was left non-empty, we mistakenly report
nbd_can_meta_context(first_string) as true, and allow
nbd_block_status() to proceed; this forms a protocol violation not
pre-filtered by our usual litany of nbd_set_strict_mode() checks, and
therefore we can trigger unspecified server behavior.
Rather than open-code a cleanup loop on all error paths, I instead fix
the problem by adding a meta_valid witness that is only set to true in
select places. The obvious place is when handling NBD_REP_ACK; but it
also makes sense if we didn't call NBD_OPT_SET_META_CONTEXT at all: if
negotiating structured replies failed (including oldstyle servers), or
if the user never called nbd_add_meta_context() and therefore doesn't
care (blindly returning 0 to nbd_can_meta_context() in those cases is
fine; our existing tests/oldstyle and tests/newstyle-limited cover
that). However, whereas we previously always returned a 0/1 answer
for nbd_can_meta_context once we are in transmission phase, this new
witness now means that in the corner case of explicit server failure
to NBD_OPT_SET_META_CONTEXT, nbd_can_meta_context() returns -1 to call
attention to the earlier server failure (although it is something that
is unlikely enough in practice that I doubt anyone will experience it
in the wild).
The change in nbd_can_meta_context() to use h->meta_valid instead of
h->eflags has an additional latent semantic difference not observable
at this time (because both h->meta_valid and h->eflags are set in
tandem by successful nbd_opt_go()/nbd_opt_info() in their current
two-command sequence), but which matters to future patches adding new
APIs. On the one hand, once we add nbd_set_request_meta_context() to
stop nbd_opt_go() from performing NBD_OPT_SET_META_CONTEXT, we want
nbd_can_meta_context() to fail during transmission phase if the
context was not set elsewhere during negotiation, rather than blindly
returning 0. On the other hand, when manually calling
nbd_opt_set_meta_context() during negotiation, we do not want it to
touch h->eflags, but do want nbd_can_meta_context() to start working
right away.
Note that the use of a witness flag also allows me to slightly change
when the list of previously-negotiated contexts is freed - instead of
doing it in nbd_internal_reset_size_and_flags(), that function now
simply marks any existing vector as invalid; and the actual wipe is
done when starting a new NBD_OPT_SET_META_CONTEXT or when closing
struct nbd_handle. There are still some oddities to be cleaned up in
later patches by moving when the flag is marked invalid (for example,
we really want nbd_set_export_name() to wipe both flags and meta
contexts, but the act of NBD_OPT_GO should not wipe contexts, and the
act of NBD_OPT_SET_META_CONTEXT should not wipe flags), but I'm
leaving those further tweaks for later patches to make this one easier
to review.
Testing this is surprisingly tricky; compliant servers don't do this
(hence I don't really have anything automatic to add to libnbd's
testsuite). However, I came up with the following temporary hack to
nbdkit to demonstrate the problem:
| diff --git i/server/protocol-handshake-newstyle.c
w/server/protocol-handshake-newstyle.c
| index fedee48f..b3f34c98 100644
| --- i/server/protocol-handshake-newstyle.c
| +++ w/server/protocol-handshake-newstyle.c
| @@ -884,7 +884,7 @@ negotiate_handshake_newstyle_options (void)
| opt_index += querylen;
| nr_queries--;
| }
| - if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1)
| + if (send_newstyle_option_reply (option, NBD_REP_ERR_POLICY) == -1)
| return -1;
| }
| debug ("newstyle negotiation: %s: reply complete",
optname);
| diff --git i/server/protocol.c w/server/protocol.c
| index 2ac77055..97235bda 100644
| --- i/server/protocol.c
| +++ w/server/protocol.c
| @@ -191,7 +191,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t
offset, uint32_t count,
|
| /* Block status allowed? */
| if (cmd == NBD_CMD_BLOCK_STATUS) {
| - if (!conn->structured_replies) {
| + if (!conn->structured_replies || 1) {
| nbdkit_error ("invalid request: "
| "%s: structured replies was not negotiated",
| name_of_nbd_cmd (cmd));
Coupled with this command line:
$ ./run /path/to/nbdkit -U- memory 1 --run 'nbdsh --base -u "$uri"
-c "
try:
print(h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION))
except nbd.Error as ex:
print(ex.string)
def f(c, o, e, er):
pass
try:
print(h.block_status(1, 0, f))
except nbd.Error as ex:
print(ex.string)
"'
Pre-patch, we see can_meta_context succeed, and that we let
NBD_CMD_BLOCK_STATUS leak through causing an nbdkit error message:
True
nbdkit: memory.0: error: invalid request: NBD_CMD_BLOCK_STATUS: structured
replies was not negotiated
nbd_block_status: block-status: command failed: Invalid argument
Post-patch, can_meta_context diagnoses the problem, and block_status
is blocked client-side with no messages needed from nbdkit:
nbd_can_meta_context: need a successful server meta context request first:
Invalid argument
nbd_block_status: did not negotiate any metadata contexts, either you did not
call nbd_add_meta_context before connecting or the server does not support it:
Operation not supported
Fixes: a97e2779 ("states: Negotiate base:allocation with the server.",
v0.1)
---
lib/internal.h | 1 +
generator/API.ml | 4 +++-
generator/states-newstyle-opt-meta-context.c | 9 +++++++--
generator/states-reply-structured.c | 1 +
lib/flags.c | 14 ++++++++------
lib/handle.c | 5 +++++
lib/rw.c | 2 +-
7 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index d601d5d..8aaff15 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -180,6 +180,7 @@ struct nbd_handle {
bool structured_replies; /* If we negotiated NBD_OPT_STRUCTURED_REPLY */
/* Vector of negotiated metadata contexts. */
+ bool meta_valid;
meta_vector meta_contexts;
/* The socket or a wrapper if using GnuTLS. */
diff --git a/generator/API.ml b/generator/API.ml
index 3e948aa..62e2d54 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -1785,7 +1785,9 @@ "can_meta_context", {
longdesc = "\
Returns true if the server supports the given meta context
(see L<nbd_add_meta_context(3)>). Returns false if
-the server does not.
+the server does not. It is possible for this command to fail if
+meta contexts were requested but there is a missing or failed
+attempt at NBD_OPT_SET_META_CONTEXT during option negotiation.
The single parameter is the name of the metadata context,
for example C<LIBNBD_CONTEXT_BASE_ALLOCATION>.
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 8f4dee2..a6a5271 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -29,6 +29,9 @@ STATE_MACHINE {
*/
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 (h->structured_replies);
@@ -44,7 +47,7 @@ STATE_MACHINE {
}
}
- assert (h->meta_contexts.len == 0);
+ assert (!h->meta_valid);
/* Calculate the length of the option request data. */
len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
@@ -194,8 +197,10 @@ STATE_MACHINE {
CALL_CALLBACK (h->opt_cb.completion, &err);
nbd_internal_free_option (h);
}
- else
+ else {
SET_NEXT_STATE (%^OPT_GO.START);
+ h->meta_valid = true;
+ }
break;
case NBD_REP_META_CONTEXT: /* A context. */
if (len > maxpayload)
diff --git a/generator/states-reply-structured.c
b/generator/states-reply-structured.c
index aaf75b8..f18c999 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -445,6 +445,7 @@ STATE_MACHINE {
assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
assert (h->bs_entries);
assert (length >= 12);
+ assert (h->meta_valid);
/* Need to byte-swap the entries returned, but apart from that we
* don't validate them.
diff --git a/lib/flags.c b/lib/flags.c
index 87c20ee..91efc1a 100644
--- a/lib/flags.c
+++ b/lib/flags.c
@@ -37,9 +37,7 @@ nbd_internal_reset_size_and_flags (struct nbd_handle *h)
h->exportsize = 0;
h->eflags = 0;
- 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;
h->block_minimum = 0;
h->block_preferred = 0;
h->block_maximum = 0;
@@ -75,6 +73,11 @@ 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) {
+ assert (h->meta_contexts.len == 0);
+ h->meta_valid = true;
+ }
+
h->exportsize = exportsize;
h->eflags = eflags;
return 0;
@@ -194,9 +197,8 @@ nbd_unlocked_can_meta_context (struct nbd_handle *h, const
char *name)
{
size_t i;
- if (h->eflags == 0) {
- set_error (EINVAL, "server has not returned export flags, "
- "you need to connect to the server first");
+ if (h->request_meta_contexts.len && !h->meta_valid) {
+ set_error (EINVAL, "need a successful server meta context request
first");
return -1;
}
diff --git a/lib/handle.c b/lib/handle.c
index 8713e18..03f45a4 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -115,6 +115,8 @@ nbd_create (void)
void
nbd_close (struct nbd_handle *h)
{
+ size_t i;
+
nbd_internal_set_error_context ("nbd_close");
if (h == NULL)
@@ -127,6 +129,9 @@ nbd_close (struct nbd_handle *h)
free (h->bs_entries);
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);
nbd_internal_free_option (h);
free_cmd_list (h->cmds_to_issue);
free_cmd_list (h->cmds_in_flight);
diff --git a/lib/rw.c b/lib/rw.c
index 81f8f35..2ab85f3 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -482,7 +482,7 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
return -1;
}
- if (h->meta_contexts.len == 0) {
+ if (!h->meta_valid || h->meta_contexts.len == 0) {
set_error (ENOTSUP, "did not negotiate any metadata contexts, "
"either you did not call nbd_add_meta_context before
"
"connecting or the server does not support it");
--
2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 03/12] 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 testsuite changes pass
when using modern nbdkit; however, it skips[*] if we talk to an older
server. But since the test also skips with our pre-patch behavior,
it's not worth separating it into a separate patch.
For more history, qemu-nbd prior to commit da24597d [v6.2] and nbdkit
prior to commit c39cba73 [v1.27.3, also backported to 1.26.6] are
servers that chose to reject NBD_OPT_LIST_META_CONTEXT without a prior
NBD_OPT_STRUCTURED_REPLY.
[*]It's easier to skip on server failure than to try and write an
nbdkit patch to add yet another --config feature probe just to depend
on new-enough nbdkit to gracefully probe in advance if the server
should succeed.
---
generator/states-newstyle-opt-meta-context.c | 1 -
lib/opt.c | 6 +----
tests/opt-list-meta.c | 28 +++++++++++++++++---
3 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index a6a5271..5c65454 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -34,7 +34,6 @@ STATE_MACHINE {
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 e5802f4..d9114f4 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;
diff --git a/tests/opt-list-meta.c b/tests/opt-list-meta.c
index 9ad8e37..ccf58fc 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>
@@ -164,7 +165,10 @@ 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. Below this point,
+ * it is not worth porting this part of the test to non-C languages
+ * because of the potential to skip the rest of the test.
+ */
nbd = nbd_create ();
if (nbd == NULL ||
nbd_set_opt_mode (nbd, true) == -1 ||
@@ -174,15 +178,31 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- /* FIXME: For now, we reject this client-side, but it is overly strict. */
+ /* Older servers don't permit this, but there is no reliable indicator
+ * of whether nbdkit is new enough, so just skip the rest of the test
+ * if the attempt fails (then read the logs to see that the skip was
+ * indeed caused by the server, and not an accidental client-side bug).
+ */
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");
+ if (r == -1) {
+ fprintf (stderr, "Skipping test; server probably too old for listing
"
+ "without structured replies: %s\n", nbd_get_error ());
+ exit (77);
+ }
+ 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);
+
exit (EXIT_SUCCESS);
}
--
2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 04/12] 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
- 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.
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 | 71 ++++++-
generator/states-newstyle-opt-go.c | 1 +
generator/states-newstyle-opt-meta-context.c | 19 +-
lib/flags.c | 4 +-
lib/handle.c | 17 ++
tests/opt-info.c | 67 +++++-
tests/opt-set-meta | 210 +++++++++++++++++++
8 files changed, 369 insertions(+), 21 deletions(-)
create mode 100755 tests/opt-set-meta
diff --git a/lib/internal.h b/lib/internal.h
index 8aaff15..9d329f0 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 62e2d54..adafac6 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -747,6 +747,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;
@@ -994,12 +1034,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", {
@@ -1068,16 +1113,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", {
@@ -1797,7 +1849,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", {
@@ -3246,6 +3299,10 @@ let first_version "set_request_block_size",
(1, 12);
"get_request_block_size", (1, 12);
+ (* Added in 1.13.x development cycle, will be stable and supported in 1.14.
*)
+ "set_request_meta_context", (1, 14);
+ "get_request_meta_context", (1, 14);
+
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
"get_tls_certificates", (1, ??);
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index b7354ae..1ca5f09 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -269,6 +269,7 @@ STATE_MACHINE {
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 5c65454..35d3cbc 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -29,9 +29,6 @@ STATE_MACHINE {
*/
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));
@@ -40,13 +37,19 @@ STATE_MACHINE {
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) {
- SET_NEXT_STATE (%^OPT_GO.START);
- return 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;
}
}
-
- assert (!h->meta_valid);
+ if (opt != h->opt_current &&
+ (!h->request_meta || !h->structured_replies ||
+ h->request_meta_contexts.len == 0)) {
+ SET_NEXT_STATE (%^OPT_GO.START);
+ return 0;
+ }
/* Calculate the length of the option request data. */
len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
diff --git a/lib/flags.c b/lib/flags.c
index 91efc1a..c8c68ea 100644
--- a/lib/flags.c
+++ b/lib/flags.c
@@ -37,7 +37,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;
@@ -73,7 +72,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 03f45a4..4b373f5 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -64,6 +64,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;
@@ -232,6 +233,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;
}
@@ -391,6 +393,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 b9739a5..26de0ee 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,59 @@ 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);
}
diff --git a/tests/opt-set-meta b/tests/opt-set-meta
new file mode 100755
index 0000000..6c7b044
--- /dev/null
+++ b/tests/opt-set-meta
@@ -0,0 +1,210 @@
+#! /bin/sh
+
+# opt-set-meta - temporary wrapper script for .libs/opt-set-meta
+# Generated by libtool (GNU libtool) 2.4.6
+#
+# The opt-set-meta program cannot be directly executed until all the libtool
+# libraries that it depends on are installed.
+#
+# This wrapper script should never be moved out of the build directory.
+# If it is, it will not operate correctly.
+
+# Sed substitution that helps us do robust quoting. It backslashifies
+# metacharacters that are still active within double-quoted strings.
+sed_quote_subst='s|\([`"$\\]\)|\\\1|g'
+
+# Be Bourne compatible
+if test -n "${ZSH_VERSION+set}" && (emulate sh) >/dev/null
2>&1; then
+ emulate sh
+ NULLCMD=:
+ # Zsh 3.x and 4.x performs word splitting on ${1+"$@"}, which
+ # is contrary to our usage. Disable this feature.
+ alias -g '${1+"$@"}'='"$@"'
+ setopt NO_GLOB_SUBST
+else
+ case `(set -o) 2>/dev/null` in *posix*) set -o posix;; esac
+fi
+BIN_SH=xpg4; export BIN_SH # for Tru64
+DUALCASE=1; export DUALCASE # for MKS sh
+
+# The HP-UX ksh and POSIX shell print the target directory to stdout
+# if CDPATH is set.
+(unset CDPATH) >/dev/null 2>&1 && unset CDPATH
+
+relink_command=""
+
+# This environment variable determines our operation mode.
+if test "$libtool_install_magic" = "%%%MAGIC variable%%%";
then
+ # install mode needs the following variables:
+ generated_by_libtool_version='2.4.6'
+ notinst_deplibs=' ../lib/libnbd.la'
+else
+ # When we are sourced in execute mode, $file and $ECHO are already set.
+ if test "$libtool_execute_magic" != "%%%MAGIC
variable%%%"; then
+ file="$0"
+
+# A function that is used when there is no print builtin or printf.
+func_fallback_echo ()
+{
+ eval 'cat <<_LTECHO_EOF
+$1
+_LTECHO_EOF'
+}
+ ECHO="printf %s\\n"
+ fi
+
+# Very basic option parsing. These options are (a) specific to
+# the libtool wrapper, (b) are identical between the wrapper
+# /script/ and the wrapper /executable/ that is used only on
+# windows platforms, and (c) all begin with the string --lt-
+# (application programs are unlikely to have options that match
+# this pattern).
+#
+# There are only two supported options: --lt-debug and
+# --lt-dump-script. There is, deliberately, no --lt-help.
+#
+# The first argument to this parsing function should be the
+# script's ../libtool value, followed by no.
+lt_option_debug+func_parse_lt_options ()
+{
+ lt_script_arg0=$0
+ shift
+ for lt_opt
+ do
+ case "$lt_opt" in
+ --lt-debug) lt_option_debug=1 ;;
+ --lt-dump-script)
+ lt_dump_D=`$ECHO "X$lt_script_arg0" | /usr/bin/sed -e
's/^X//' -e 's%/[^/]*$%%'`
+ test "X$lt_dump_D" = "X$lt_script_arg0" &&
lt_dump_D=.
+ lt_dump_F=`$ECHO "X$lt_script_arg0" | /usr/bin/sed -e
's/^X//' -e 's%^.*/%%'`
+ cat "$lt_dump_D/$lt_dump_F"
+ exit 0
+ ;;
+ --lt-*)
+ $ECHO "Unrecognized --lt- option: '$lt_opt'"
1>&2
+ exit 1
+ ;;
+ esac
+ done
+
+ # Print the debug banner immediately:
+ if test -n "$lt_option_debug"; then
+ echo "opt-set-meta:opt-set-meta:$LINENO: libtool wrapper (GNU libtool)
2.4.6" 1>&2
+ fi
+}
+
+# Used when --lt-debug. Prints its arguments to stdout
+# (redirection is the responsibility of the caller)
+func_lt_dump_args ()
+{
+ lt_dump_args_N=1;
+ for lt_arg
+ do
+ $ECHO "opt-set-meta:opt-set-meta:$LINENO: newargv[$lt_dump_args_N]:
$lt_arg"
+ lt_dump_args_N=`expr $lt_dump_args_N + 1`
+ done
+}
+
+# Core function for launching the target application
+func_exec_program_core ()
+{
+
+ if test -n "$lt_option_debug"; then
+ $ECHO "opt-set-meta:opt-set-meta:$LINENO: newargv[0]:
$progdir/$program" 1>&2
+ func_lt_dump_args ${1+"$@"} 1>&2
+ fi
+ exec "$progdir/$program" ${1+"$@"}
+
+ $ECHO "$0: cannot exec $program $*" 1>&2
+ exit 1
+}
+
+# A function to encapsulate launching the target application
+# Strips options in the --lt-* namespace from $@ and
+# launches target application with the remaining arguments.
+func_exec_program ()
+{
+ case " $* " in
+ *\ --lt-*)
+ for lt_wr_arg
+ do
+ case $lt_wr_arg in
+ --lt-*) ;;
+ *) set x "$@" "$lt_wr_arg"; shift;;
+ esac
+ shift
+ done ;;
+ esac
+ func_exec_program_core ${1+"$@"}
+}
+
+ # Parse options
+ func_parse_lt_options "$0" ${1+"$@"}
+
+ # Find the directory that this script lives in.
+ thisdir=`$ECHO "$file" | /usr/bin/sed 's%/[^/]*$%%'`
+ test "x$thisdir" = "x$file" && thisdir=.
+
+ # Follow symbolic links until we get to the real thisdir.
+ file=`ls -ld "$file" | /usr/bin/sed -n 's/.*-> //p'`
+ while test -n "$file"; do
+ destdir=`$ECHO "$file" | /usr/bin/sed 's%/[^/]*$%%'`
+
+ # If there was a directory component, then change thisdir.
+ if test "x$destdir" != "x$file"; then
+ case "$destdir" in
+ [\\/]* | [A-Za-z]:[\\/]*) thisdir="$destdir" ;;
+ *) thisdir="$thisdir/$destdir" ;;
+ esac
+ fi
+
+ file=`$ECHO "$file" | /usr/bin/sed 's%^.*/%%'`
+ file=`ls -ld "$thisdir/$file" | /usr/bin/sed -n 's/.*->
//p'`
+ done
+
+ # Usually 'no', except on cygwin/mingw when embedded into
+ # the cwrapper.
+ WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no
+ if test "$WRAPPER_SCRIPT_BELONGS_IN_OBJDIR" = "yes"; then
+ # special case for '.'
+ if test "$thisdir" = "."; then
+ thisdir=`pwd`
+ fi
+ # remove .libs from thisdir
+ case "$thisdir" in
+ *[\\/].libs ) thisdir=`$ECHO "$thisdir" | /usr/bin/sed
's%[\\/][^\\/]*$%%'` ;;
+ .libs ) thisdir=. ;;
+ esac
+ fi
+
+ # Try to get the absolute directory name.
+ absdir=`cd "$thisdir" && pwd`
+ test -n "$absdir" && thisdir="$absdir"
+
+ program='opt-set-meta'
+ progdir="$thisdir/.libs"
+
+
+ if test -f "$progdir/$program"; then
+ # Add our own library path to LD_LIBRARY_PATH
+ LD_LIBRARY_PATH="/home/eblake/libnbd/lib/.libs:$LD_LIBRARY_PATH"
+
+ # Some systems cannot cope with colon-terminated LD_LIBRARY_PATH
+ # The second colon is a workaround for a bug in BeOS R4 sed
+ LD_LIBRARY_PATH=`$ECHO "$LD_LIBRARY_PATH" | /usr/bin/sed
's/::*$//'`
+
+ export LD_LIBRARY_PATH
+
+ if test "$libtool_execute_magic" != "%%%MAGIC
variable%%%"; then
+ # Run the actual program with our arguments.
+ func_exec_program ${1+"$@"}
+ fi
+ else
+ # The program doesn't exist.
+ $ECHO "$0: error: '$progdir/$program' does not exist"
1>&2
+ $ECHO "This script is just a wrapper for $program." 1>&2
+ $ECHO "See the libtool documentation for more information."
1>&2
+ exit 1
+ fi
+fi
--
2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 05/12] 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.
---
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 749c94f..6b62c8a 100644
--- a/python/t/110-defaults.py
+++ b/python/t/110-defaults.py
@@ -22,6 +22,7 @@ assert h.get_export_name() == ""
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 61a4160..8b48b4a 100644
--- a/python/t/120-set-non-defaults.py
+++ b/python/t/120-set-non-defaults.py
@@ -33,6 +33,8 @@ if h.supports_tls():
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 37db341..8aa47ae 100644
--- a/python/t/230-opt-info.py
+++ b/python/t/230-opt-info.py
@@ -52,12 +52,14 @@ must_fail(h.get_size)
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 @@ must_fail(h.set_export_name, "a")
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 ccec9c6..768ad63 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 092287e..76ff82f 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 693a663..ec735ff 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 f56c965..d7ad319 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 a4c411d..06bb29d 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 3dd231a..bc4cadf 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.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 06/12] 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.
---
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 7646bf1..c2741ba 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 79038b0..4bf5967 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.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 07/12] 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. Since we do not otherwise modify h->meta_contexts while
processing nbd_opt_list_meta_contexts(), we also should not wipe it;
similarly, while clearing h->exportsize makes sense if we know we are
going to attempt NBD_OPT_INFO or NBD_OPT_GO, it should not be
attempted merely because we are doing NBD_OPT_LIST_META_CONTEXTS.
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-meta-context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 35d3cbc..a281b05 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -28,7 +28,6 @@ STATE_MACHINE {
* contexts, when doing SET (but an empty LIST is okay).
*/
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));
@@ -37,6 +36,7 @@ STATE_MACHINE {
else {
assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
opt = NBD_OPT_SET_META_CONTEXT;
+ nbd_internal_reset_size_and_flags (h);
if (h->request_meta) {
for (i = 0; i < h->meta_contexts.len; ++i)
free (h->meta_contexts.ptr[i].name);
--
2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 08/12] 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.
---
python/t/240-opt-list-meta.py | 29 +++++++++-
ocaml/tests/test_240_opt_list_meta.ml | 34 ++++++++++-
tests/opt-list-meta.c | 76 +++++++++++++++++++++++--
golang/libnbd_240_opt_list_meta_test.go | 64 ++++++++++++++++++++-
4 files changed, 193 insertions(+), 10 deletions(-)
diff --git a/python/t/240-opt-list-meta.py b/python/t/240-opt-list-meta.py
index 8341f9c..591703a 100644
--- a/python/t/240-opt-list-meta.py
+++ b/python/t/240-opt-list-meta.py
@@ -1,5 +1,5 @@
# libnbd Python bindings
-# Copyright (C) 2010-2020 Red Hat Inc.
+# 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
@@ -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
+
+
h = nbd.NBD()
h.set_opt_mode(True)
h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
@@ -65,10 +73,27 @@ assert r == 1
assert count == 1
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
+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 not seen
+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 639d33c..afab084 100644
--- a/ocaml/tests/test_240_opt_list_meta.ml
+++ b/ocaml/tests/test_240_opt_list_meta.ml
@@ -64,10 +64,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 ccf58fc..12d6f51 100644
--- a/tests/opt-list-meta.c
+++ b/tests/opt-list-meta.c
@@ -138,13 +138,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 d732275..47df787 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
@@ -118,13 +118,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
+ */
count = 0
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 != count || 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" */
+ count = 0
+ seen = false
err = h.AddMetaContext("base:")
if err != nil {
t.Fatalf("could not request add_meta_context: %s", err)
--
2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 09/12] 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)
---
lib/handle.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/handle.c b/lib/handle.c
index 4b373f5..d8c0dfe 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -225,6 +225,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");
@@ -233,6 +236,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.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 10/12] 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.
---
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 8aa47ae..fc22a97 100644
--- a/python/t/230-opt-info.py
+++ b/python/t/230-opt-info.py
@@ -45,17 +45,22 @@ assert h.get_size() == 0
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 ec735ff..0403d14 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 26de0ee..70028ff 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 bc4cadf..e16f661 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.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 11/12] api: Add nbd_[aio_]opt_set_meta_context
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.
Overhaul the comment on OPT_META_CONTEXT.START to explain 5 different
scenarios that this state group can be reached in, to better document
which conditionals are in use later in the state machine:
- h->current_opt == opt is a witness of whether we are doing a
mandatory manual LIST/SET request with an expected callback and
transition to NEGOTIATIONG, or a conditional SET request dependent on
nbd_set_request_meta_context() and prior negotiation and transition to
OPT_GO
- h->current_opt == NBD_OPT_LIST_META_CONTEXT is a witness of whether
we are stateless or modifying h->meta_contexts
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.
---
Patches to add nbd_opt_starttls and nbd_opt_structured_reply are not
written yet...
---
generator/API.ml | 115 ++++++++-
generator/states-newstyle-opt-meta-context.c | 38 +--
generator/states-newstyle.c | 1 +
lib/opt.c | 47 +++-
tests/Makefile.am | 9 +
tests/opt-set-meta.c | 235 +++++++++++++++++++
.gitignore | 1 +
7 files changed, 420 insertions(+), 26 deletions(-)
create mode 100644 tests/opt-set-meta.c
diff --git a/generator/API.ml b/generator/API.ml
index adafac6..24cbabc 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -762,17 +762,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"];
};
@@ -1036,7 +1039,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;
@@ -1044,7 +1049,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", {
@@ -1179,7 +1185,64 @@ "opt_list_meta_context", {
a server may send a lengthy list.";
see_also = [Link "set_opt_mode"; Link
"aio_opt_list_meta_context";
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_set_meta_context", {
+ default_call with
+ args = [ Closure context_closure ]; ret = RInt;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server to select specific meta
contexts";
+ 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.
+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_go"; Link "set_export_name";
+ Link "opt_list_meta_context"; Link
"set_request_meta_context";
+ Link "can_meta_context"];
};
"add_meta_context", {
@@ -1850,7 +1913,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", {
@@ -2592,6 +2655,38 @@ "aio_opt_list_meta_context", {
see_also = [Link "set_opt_mode"; Link
"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 = "request the server to select specific meta
contexts";
+ 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"];
+ };
+
"aio_pread", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
@@ -3302,6 +3397,8 @@ let first_version (* Added in 1.13.x development cycle,
will be stable and supported in 1.14. *)
"set_request_meta_context", (1, 14);
"get_request_meta_context", (1, 14);
+ "opt_set_meta_context", (1, 14);
+ "aio_opt_set_meta_context", (1, 14);
(* 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 a281b05..03b63c6 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -34,10 +34,14 @@ STATE_MACHINE {
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));
+ nbd_internal_reset_size_and_flags (h);
+ }
opt = NBD_OPT_SET_META_CONTEXT;
- nbd_internal_reset_size_and_flags (h);
- 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);
@@ -194,15 +198,15 @@ STATE_MACHINE {
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)
@@ -221,10 +225,10 @@ STATE_MACHINE {
}
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);
@@ -235,25 +239,27 @@ STATE_MACHINE {
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 41e7cc3..62f2372 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -140,6 +140,7 @@ STATE_MACHINE {
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 d9114f4..2ad8c9e 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);
}
@@ -215,6 +216,28 @@ nbd_unlocked_opt_list_meta_context (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)
+{
+ 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 (h, &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,
@@ -301,3 +324,25 @@ nbd_unlocked_aio_opt_list_meta_context (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)
+{
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
+ return -1;
+ }
+
+ assert (CALLBACK_IS_NULL (h->opt_cb.fn.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 7fc7dec..1c6f137 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -213,6 +213,7 @@ check_PROGRAMS += \
opt-list \
opt-info \
opt-list-meta \
+ opt-set-meta \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -278,6 +279,7 @@ TESTS += \
opt-list \
opt-info \
opt-list-meta \
+ opt-set-meta \
connect-systemd-socket-activation \
connect-unix \
connect-tcp \
@@ -533,6 +535,13 @@ 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_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
+
connect_systemd_socket_activation_SOURCES = \
connect-systemd-socket-activation.c \
requires.c \
diff --git a/tests/opt-set-meta.c b/tests/opt-set-meta.c
new file mode 100644
index 0000000..01c9604
--- /dev/null
+++ b/tests/opt-set-meta.c
@@ -0,0 +1,235 @@
+/* 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 };
+ char *tmp;
+
+ /* 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 bd4650d..5603de5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -231,6 +231,7 @@ Makefile.in
/tests/opt-info
/tests/opt-list
/tests/opt-list-meta
+/tests/opt-set-meta
/tests/pki/
/tests/pread-initialize
/tests/private-data
--
2.37.2
Eric Blake
2022-Aug-31 14:39 UTC
[Libguestfs] [libnbd PATCH v2 12/12] 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.
The fact that Go slams all unit tests into the same namespace is
somewhat annoying; it required munging test 240 now that 250 wants to
use similar global variables.
---
python/t/250-opt-set-meta.py | 126 ++++++++++++
ocaml/tests/Makefile.am | 5 +-
ocaml/tests/test_250_opt_set_meta.ml | 150 ++++++++++++++
tests/opt-set-meta.c | 1 +
golang/Makefile.am | 3 +-
golang/libnbd_240_opt_list_meta_test.go | 54 +++---
golang/libnbd_250_opt_set_meta_test.go | 248 ++++++++++++++++++++++++
7 files changed, 558 insertions(+), 29 deletions(-)
create mode 100644 python/t/250-opt-set-meta.py
create mode 100644 ocaml/tests/test_250_opt_set_meta.ml
create mode 100644 golang/libnbd_250_opt_set_meta_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 0000000..eb27998
--- /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 not seen
+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 not seen
+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
+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 not seen
+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 not seen
+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/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am
index 24f626d..f8ce810 100644
--- a/ocaml/tests/Makefile.am
+++ b/ocaml/tests/Makefile.am
@@ -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
@@ -31,6 +31,7 @@ EXTRA_DIST = \
test_220_opt_list.ml \
test_230_opt_info.ml \
test_240_opt_list_meta.ml \
+ test_250_opt_set_meta.ml \
test_300_get_size.ml \
test_400_pread.ml \
test_405_pread_structured.ml \
@@ -59,6 +60,7 @@ tests_bc = \
test_220_opt_list.bc \
test_230_opt_info.bc \
test_240_opt_list_meta.bc \
+ test_250_opt_set_meta.bc \
test_300_get_size.bc \
test_400_pread.bc \
test_405_pread_structured.bc \
@@ -84,6 +86,7 @@ tests_opt = \
test_220_opt_list.opt \
test_230_opt_info.opt \
test_240_opt_list_meta.opt \
+ test_250_opt_set_meta.opt \
test_300_get_size.opt \
test_400_pread.opt \
test_405_pread_structured.opt \
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 0000000..d74e9e0
--- /dev/null
+++ b/ocaml/tests/test_250_opt_set_meta.ml
@@ -0,0 +1,150 @@
+(* 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 () + 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/tests/opt-set-meta.c b/tests/opt-set-meta.c
index 01c9604..bd00276 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 f170cbc..f6e6f91 100644
--- a/golang/Makefile.am
+++ b/golang/Makefile.am
@@ -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
@@ -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_250_opt_set_meta_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 47df787..4af1008 100644
--- a/golang/libnbd_240_opt_list_meta_test.go
+++ b/golang/libnbd_240_opt_list_meta_test.go
@@ -22,16 +22,16 @@ import (
"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
}
@@ -57,22 +57,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 {
- t.Fatalf("unexpected count after opt_list_meta_context")
+ if r != list_count || r < 1 || !list_seen {
+ t.Fatalf("unexpected list_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)
@@ -83,13 +83,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 {
- t.Fatalf("unexpected count after opt_list_meta_context")
+ if r != 0 || r != list_count || list_seen {
+ t.Fatalf("unexpected list_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)
@@ -114,15 +114,15 @@ 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 {
- t.Fatalf("unexpected count after opt_list_meta_context")
+ if r != 1 || r != list_count || !list_seen {
+ t.Fatalf("unexpected list_count after opt_list_meta_context")
}
/* Fourth pass: opt_list_meta_context is stateless, so it should
* not wipe status learned during opt_info
*/
- count = 0
- seen = false
+ list_count = 0
+ list_seen = false
_, err = h.GetSize()
if err == nil {
t.Fatalf("expected error")
@@ -147,7 +147,7 @@ func Test240OptListMeta(t *testing.T) {
t.Fatalf("can_meta_context failed unexpectedly: %s", err)
}
if !meta {
- t.Fatalf("unexpected count after can_meta_context")
+ t.Fatalf("unexpected list_count after can_meta_context")
}
err = h.ClearMetaContexts()
if err != nil {
@@ -163,8 +163,8 @@ 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 {
- t.Fatalf("unexpected count after opt_list_meta_context")
+ if r != 0 || r != list_count || list_seen {
+ t.Fatalf("unexpected list_count after opt_list_meta_context")
}
size, err = h.GetSize()
if err != nil {
@@ -178,13 +178,13 @@ func Test240OptListMeta(t *testing.T) {
t.Fatalf("can_meta_context failed unexpectedly: %s", err)
}
if !meta {
- t.Fatalf("unexpected count after can_meta_context")
+ t.Fatalf("unexpected list_count after can_meta_context")
}
err = h.ClearMetaContexts()
/* Final pass: "base:" query should get at least
"base:allocation" */
- count = 0
- seen = false
+ list_count = 0
+ list_seen = false
err = h.AddMetaContext("base:")
if err != nil {
t.Fatalf("could not request add_meta_context: %s", err)
@@ -195,8 +195,8 @@ 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 {
- t.Fatalf("unexpected count after opt_list_meta_context")
+ if r < 1 || r > max || r != list_count || !list_seen {
+ t.Fatalf("unexpected list_count after opt_list_meta_context")
}
err = h.OptAbort()
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 0000000..c29b7f4
--- /dev/null
+++ b/golang/libnbd_250_opt_set_meta_test.go
@@ -0,0 +1,248 @@
+/* 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) {
+ 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")
+ }
+
+ /* 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")
+ }
+}
--
2.37.2
Eric Blake
2022-Aug-31 17:01 UTC
[Libguestfs] [libnbd PATCH v2 00/12] Improve NBD_OPT_ control
On Wed, Aug 31, 2022 at 09:39:16AM -0500, Eric Blake wrote:> v1 was here (under the subject Smarter nbd_opt_info) > https://listman.redhat.com/archives/libguestfs/2022-August/029641.html >> tests/Makefile.am | 9 + > tests/opt-info.c | 91 ++++++- > tests/opt-list-meta.c | 104 +++++++- > tests/opt-set-meta | 210 ++++++++++++++++ > tests/opt-set-meta.c | 236 ++++++++++++++++++ > .gitignore | 1 +> 35 files changed, 1813 insertions(+), 154 deletions(-) > create mode 100644 python/t/250-opt-set-meta.py > create mode 100644 ocaml/tests/test_250_opt_set_meta.ml > create mode 100755 tests/opt-set-metaWhoops - this one is a libtool wrapper, and should not have been included (at least it was a libtool wrapper instead of a full-blown binary image). I'll scrub it out of the offending patch 4/12. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Eric Blake
2022-Sep-01 15:08 UTC
[Libguestfs] [libnbd PATCH v2 07/12] api: Make nbd_opt_list_meta_context stateless
On Wed, Aug 31, 2022 at 09:39:23AM -0500, Eric Blake wrote:> 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. Since we do not otherwise modify h->meta_contexts while > processing nbd_opt_list_meta_contexts(), we also should not wipe it; > similarly, while clearing h->exportsize makes sense if we know we are > going to attempt NBD_OPT_INFO or NBD_OPT_GO, it should not be > attempted merely because we are doing NBD_OPT_LIST_META_CONTEXTS. > > 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-meta-context.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)Given the review comments on 3/12, I'm thinking of squashing in: diff --git c/generator/states-newstyle-opt-go.c w/generator/states-newstyle-opt-go.c index 1ca5f099..9b05bec1 100644 --- c/generator/states-newstyle-opt-go.c +++ w/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 c/generator/states-newstyle-opt-meta-context.c w/generator/states-newstyle-opt-meta-context.c index 3e66b106..6149d61d 100644 --- c/generator/states-newstyle-opt-meta-context.c +++ w/generator/states-newstyle-opt-meta-context.c @@ -35,12 +35,12 @@ STATE_MACHINE { * 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. * 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. - * 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. * There is a callback if and only if the command is unconditional. */ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); @@ -52,7 +52,6 @@ STATE_MACHINE { else { assert (CALLBACK_IS_NULL (h->opt_cb.fn.context)); opt = NBD_OPT_SET_META_CONTEXT; - nbd_internal_reset_size_and_flags (h); if (h->request_meta) { for (i = 0; i < h->meta_contexts.len; ++i) free (h->meta_contexts.ptr[i].name); -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org