[Libguestfs] [libnbd PATCH v2 08/12] tests: Add coverage for stateless nbd_opt_list_meta_context
Laszlo Ersek
lersek at redhat.com
Fri Sep 9 06:52:20 UTC 2022
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 at 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)
>
More information about the Libguestfs
mailing list