On 08/31/22 16:39, Eric Blake wrote:
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)
So thus far we've only submitted LIST, with "x-nosuch" and
"base:allocation" in request_meta_contexts. LIST does not change
meta_contexts, which is why the above can_meta_context is expected to
fail (AIUI).
+h.opt_info()
This is where we submit SET.
+assert h.get_size() == 1024*1024
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
h.clear_meta_contexts()
So this is the mind-boggling part: it says "clear meta contexts"
nominally, but it only clears the *requested* meta contexts (e.g. for
the next LIST op); it does not clear the negotiated 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
This makes sense, it basically repeats "Second pass: bogus query has no
response" (just with meta_contexts already containing the negotiated
base:allocation context).
+assert h.get_size() == 1048576
+assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True
Right.
I think the test case is valid; I just find it nearly impossible to
understand the library state and the transitions.
Basically, for the sake of easily generating the language wrappers, the
libnbd API commits to a programming style that is otherwise widely
considered really bad: namely, to keeping everything in (effectively)
global variables. By not limiting the *apparent scope* of opcode
parameters (i.e. by turning everything into internal globals), it's
super difficult to see what matters and what matters *not*, at any
particular point.
I understand the importance of auto-generating the wrappers, but man,
tracking this mentally is super exhausting.
I think the patch is good.
Reviewed-by: Laszlo Ersek <lersek(a)redhat.com>
Laszlo
+
+# 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)