Libguestfs - Aug 2019 - [libnbd PATCH 2/2] ocaml: Fix test 505

Rich found that I had been using bogus logic, but it was hidden by
other bugs in our language bindings until recently.

Eric Blake (2):
  python: Fix test 505
  ocaml: Fix test 505

 python/t/505-aio-pread-callback.py                 | 14 +++++++-------
 .../test_505_aio_pread_structured_callback.ml      | 14 +++++++-------
 2 files changed, 14 insertions(+), 14 deletions(-)

-- 
2.20.1

The logic in this test was wrong, but masked by other problems still
existing in our python bindings.  The fourth test claims to fail
during only chunk, but passed '42' as the user_data for callback which
requires that err.value be 0; if .chunk had actually failed, err.value
should be non-zero.

We're overloading a single user_data int to try and control two
orthogonal things (what to expect in err, and whether to fail); let's
instead make the callback user_data a tuple.

Fixes: e7d29712
---
 python/t/505-aio-pread-callback.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/python/t/505-aio-pread-callback.py
b/python/t/505-aio-pread-callback.py
index 8d71c38..3a90a0c 100644
--- a/python/t/505-aio-pread-callback.py
+++ b/python/t/505-aio-pread-callback.py
@@ -34,19 +34,19 @@ def chunk (user_data, buf2, offset, s, err):
     assert s == nbd.READ_DATA

 def callback (user_data, err):
-    print ("in callback, user_data %d" % user_data)
-    if user_data <= 43:
+    print ("in callback, user_data %d,%d" % user_data)
+    if user_data[0] == 42:
         assert err.value == 0
     else:
         assert err.value == errno.EPROTO
     err.value = errno.ENOMEM
-    assert user_data == 42
+    assert user_data[1] == 42

 # First try: succeed in both callbacks
 buf = nbd.Buffer (512)
 cookie = h.aio_pread_structured (buf, 0,
                                  lambda *args: chunk (42, *args),
-                                 lambda *args: callback (42, *args))
+                                 lambda *args: callback ((42, 42), *args))
 while not (h.aio_command_completed (cookie)):
     h.poll (-1)

@@ -60,7 +60,7 @@ assert buf == expected
 buf = nbd.Buffer (512)
 cookie = h.aio_pread_structured (buf, 0,
                                  lambda *args: chunk (42, *args),
-                                 lambda *args: callback (43, *args))
+                                 lambda *args: callback ((42, 43), *args))
 try:
     while not (h.aio_command_completed (cookie)):
         h.poll (-1)
@@ -72,7 +72,7 @@ except nbd.Error as ex:
 buf = nbd.Buffer (512)
 cookie = h.aio_pread_structured (buf, 0,
                                  lambda *args: chunk (43, *args),
-                                 lambda *args: callback (44, *args))
+                                 lambda *args: callback ((43, 43), *args))
 try:
     while not (h.aio_command_completed (cookie)):
         h.poll (-1)
@@ -84,7 +84,7 @@ except nbd.Error as ex:
 buf = nbd.Buffer (512)
 cookie = h.aio_pread_structured (buf, 0,
                                  lambda *args: chunk (43, *args),
-                                 lambda *args: callback (42, *args))
+                                 lambda *args: callback ((43, 42), *args))
 try:
     while not (h.aio_command_completed (cookie)):
         h.poll (-1)
-- 
2.20.1

The logic in this test was wrong (blindly copied from a similar test
in python), but masked by other problems until recently.  The fourth
test claims to fail during only chunk, but passed '42' as the
user_data for callback which requires that !err be 0; if chunk had
actually failed, !err should be non-zero.

We're overloading a single user_data int to try and control two
orthogonal things (what to expect in err, and whether to fail); let's
instead make the callback user_data a tuple.

Fixes: ce0f2126
---
 .../test_505_aio_pread_structured_callback.ml      | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/ocaml/tests/test_505_aio_pread_structured_callback.ml
b/ocaml/tests/test_505_aio_pread_structured_callback.ml
index 075ec85..433dc27 100644
--- a/ocaml/tests/test_505_aio_pread_structured_callback.ml
+++ b/ocaml/tests/test_505_aio_pread_structured_callback.ml
@@ -45,12 +45,12 @@ let chunk user_data buf2 offset s err    0

 let callback user_data err -  if user_data <= 43 then
+  if fst user_data = 42 then
     assert (!err = 0)
   else
     assert (!err = 100);
   err := 101;
-  assert (user_data = 42);
+  assert (snd user_data = 42);
   0

 let () @@ -61,7 +61,7 @@ let ()    (* First try: succeed in both callbacks *)
   let buf = NBD.Buffer.alloc 512 in
   let cookie = NBD.aio_pread_structured nbd buf 0_L (chunk 42)
-                 ~completion:(callback 42) in
+                 ~completion:(callback (42, 42)) in
   while not (NBD.aio_command_completed nbd cookie) do
     ignore (NBD.poll nbd (-1))
   done;
@@ -73,7 +73,7 @@ let ()    (* Second try: fail only during callback *)
   let buf = NBD.Buffer.alloc 512 in
   let cookie = NBD.aio_pread_structured nbd buf 0_L (chunk 42)
-                 ~completion:(callback 43) in
+                 ~completion:(callback (42, 43)) in
   try
     while not (NBD.aio_command_completed nbd cookie) do
       ignore (NBD.poll nbd (-1))
@@ -86,8 +86,8 @@ let () 
   (* Third try: fail during both *)
   let buf = NBD.Buffer.alloc 512 in
-  let cookie = NBD.aio_pread_structured nbd buf 0_L (chunk 43)
-                 ~completion:(callback 44) in
+  let cookie = NBD.aio_pread_structured nbd buf 0_L (chunk 44)
+                 ~completion:(callback (43, 43)) in
   try
     while not (NBD.aio_command_completed nbd cookie) do
       ignore (NBD.poll nbd (-1))
@@ -101,7 +101,7 @@ let ()    (* Fourth try: fail only during chunk *)
   let buf = NBD.Buffer.alloc 512 in
   let cookie = NBD.aio_pread_structured nbd buf 0_L (chunk 43)
-                 ~completion:(callback 42) in
+                 ~completion:(callback (43, 42)) in
   try
     while not (NBD.aio_command_completed nbd cookie) do
       ignore (NBD.poll nbd (-1))
-- 
2.20.1

ACK series, thanks.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Wed, Aug 14, 2019 at 01:54:34PM -0500, Eric Blake
wrote:
>  let callback user_data err > -  if user_data <= 43 then
> +  if fst user_data = 42 then
>      assert (!err = 0)
>    else
>      assert (!err = 100);
>    err := 101;
> -  assert (user_data = 42);
> +  assert (snd user_data = 42);
Actually I think this change is wrong?  It seems to need this I think.

Rich.

diff --git a/ocaml/tests/test_505_aio_pread_structured_callback.ml
b/ocaml/tests/test_505_aio_pread_structured_callback.ml
index 5474b2e..fed493d 100644
--- a/ocaml/tests/test_505_aio_pread_structured_callback.ml
+++ b/ocaml/tests/test_505_aio_pread_structured_callback.ml
@@ -50,7 +50,8 @@ let callback user_data err    else
     assert (!err = 100);
   err := 101;
-  assert (snd user_data = 42);
+  if snd user_data <> 42 then
+    invalid_arg "this should be turned into NBD.Error";
   0
 
 let () 

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/