Eric Blake
2022-Sep-26 22:05 UTC
[Libguestfs] [libnbd PATCH v3 14/18] tests: Add coverage for nbd_set_export_name fix
Add test coverage for the sequences fixed in the previous patch, where changing the export name should clean up anything remembered about a former export name. The tests in other language bindings are logically equivalent. Acked-by: Laszlo Ersek <lersek at redhat.com> --- python/t/230-opt-info.py | 11 ++++++++--- ocaml/tests/test_230_opt_info.ml | 11 ++++++++--- tests/opt-info.c | 24 ++++++++++++++++++------ golang/libnbd_230_opt_info_test.go | 23 +++++++++++++++++------ 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/python/t/230-opt-info.py b/python/t/230-opt-info.py index c326bc30..45682dd9 100644 --- a/python/t/230-opt-info.py +++ b/python/t/230-opt-info.py @@ -45,17 +45,22 @@ def must_fail(f, *args, **kwds): assert h.is_read_only() is True assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True -# info on something not present fails, wipes out prior info -h.set_export_name("a") -must_fail(h.opt_info) +# changing export wipes out prior info +h.set_export_name("b") must_fail(h.get_size) must_fail(h.is_read_only) must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION) +# info on something not present fails +h.set_export_name("a") +must_fail(h.opt_info) + # info for a different export, with automatic meta_context disabled h.set_export_name("b") h.set_request_meta_context(False) h.opt_info() +# idempotent name change is no-op +h.set_export_name("b") assert h.get_size() == 1 assert h.is_read_only() is False must_fail(h.can_meta_context, nbd.CONTEXT_BASE_ALLOCATION) diff --git a/ocaml/tests/test_230_opt_info.ml b/ocaml/tests/test_230_opt_info.ml index ec735ff1..0403d14e 100644 --- a/ocaml/tests/test_230_opt_info.ml +++ b/ocaml/tests/test_230_opt_info.ml @@ -62,17 +62,22 @@ let let meta = NBD.can_meta_context nbd NBD.context_base_allocation in assert meta; - (* info on something not present fails, wipes out prior info *) - NBD.set_export_name nbd "a"; - fail_unary NBD.opt_info nbd; + (* changing export wipes out prior info *) + NBD.set_export_name nbd "b"; fail_unary NBD.get_size nbd; fail_unary NBD.is_read_only nbd; fail_binary NBD.can_meta_context nbd NBD.context_base_allocation; + (* info on something not present fails *) + NBD.set_export_name nbd "a"; + fail_unary NBD.opt_info nbd; + (* info for a different export, with automatic meta_context disabled *) NBD.set_export_name nbd "b"; NBD.set_request_meta_context nbd false; NBD.opt_info nbd; + (* idempotent name change is no-op *) + NBD.set_export_name nbd "b"; let size = NBD.get_size nbd in assert (size = 1L); let ro = NBD.is_read_only nbd in diff --git a/tests/opt-info.c b/tests/opt-info.c index cf423d5c..737af2a4 100644 --- a/tests/opt-info.c +++ b/tests/opt-info.c @@ -17,6 +17,7 @@ */ /* Test behavior of nbd_opt_info. */ +/* See also unit test 230 in the various language ports. */ #include <config.h> @@ -80,15 +81,11 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* info on something not present fails, wipes out prior info */ - if (nbd_set_export_name (nbd, "a") == -1) { + /* changing export wipes out prior info */ + if (nbd_set_export_name (nbd, "b") == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } - if (nbd_opt_info (nbd) != -1) { - fprintf (stderr, "expecting error for opt_info\n"); - exit (EXIT_FAILURE); - } if (nbd_get_size (nbd) != -1) { fprintf (stderr, "expecting error for get_size\n"); exit (EXIT_FAILURE); @@ -102,6 +99,16 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } + /* info on something not present fails */ + if (nbd_set_export_name (nbd, "a") == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_opt_info (nbd) != -1) { + fprintf (stderr, "expecting error for opt_info\n"); + exit (EXIT_FAILURE); + } + /* info for a different export, with automatic meta_context disabled */ if (nbd_set_export_name (nbd, "b") == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); @@ -115,6 +122,11 @@ main (int argc, char *argv[]) fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } + /* idempotent name change is no-op */ + if (nbd_set_export_name (nbd, "b") == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } if ((r = nbd_get_size (nbd)) != 1) { fprintf (stderr, "expecting size of 1, got %" PRId64 "\n", r); exit (EXIT_FAILURE); diff --git a/golang/libnbd_230_opt_info_test.go b/golang/libnbd_230_opt_info_test.go index bc4cadfb..e16f6611 100644 --- a/golang/libnbd_230_opt_info_test.go +++ b/golang/libnbd_230_opt_info_test.go @@ -91,15 +91,11 @@ func Test230OptInfo(t *testing.T) { t.Fatalf("unexpected meta context") } - /* info on something not present fails, wipes out prior info */ - err = h.SetExportName("a") + /* changing export wipes out prior info */ + err = h.SetExportName("b") if err != nil { t.Fatalf("set export name failed unexpectedly: %s", err) } - err = h.OptInfo() - if err == nil { - t.Fatalf("expected error") - } _, err = h.GetSize() if err == nil { t.Fatalf("expected error") @@ -113,6 +109,16 @@ func Test230OptInfo(t *testing.T) { t.Fatalf("expected error") } + /* info on something not present fails */ + err = h.SetExportName("a") + if err != nil { + t.Fatalf("set export name failed unexpectedly: %s", err) + } + err = h.OptInfo() + if err == nil { + t.Fatalf("expected error") + } + /* info for a different export, with automatic meta_context disabled */ err = h.SetExportName("b") if err != nil { @@ -126,6 +132,11 @@ func Test230OptInfo(t *testing.T) { if err != nil { t.Fatalf("opt_info failed unexpectedly: %s", err) } + /* idempotent name change is no-op */ + err = h.SetExportName("b") + if err != nil { + t.Fatalf("set export name failed unexpectedly: %s", err) + } size, err = h.GetSize() if err != nil { t.Fatalf("size failed unexpectedly: %s", err) -- 2.37.3
Richard W.M. Jones
2022-Sep-27 11:32 UTC
[Libguestfs] [libnbd PATCH v3 14/18] tests: Add coverage for nbd_set_export_name fix
On Mon, Sep 26, 2022 at 05:05:56PM -0500, Eric Blake wrote: [...]> @@ -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); > + }While looking at this, I found this which seems like it may be wrong (lib/opt.c): /* Issue NBD_OPT_INFO and wait for the reply. */ int nbd_unlocked_opt_info (struct nbd_handle *h) ... r = wait_for_option (h); if (r == 0 && err) { assert (nbd_internal_is_state_negotiating (get_next_state (h)) || nbd_internal_is_state_dead (get_next_state (h))); set_error (err, "server replied with error to opt_info request"); I believe that r == 0 && err != 0 => the callback set *err; in which case the error message is wrong? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org