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
Richard W.M. Jones
2019-Aug-14 19:33 UTC
Re: [Libguestfs] [libnbd PATCH 2/2] ocaml: Fix test 505
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
Richard W.M. Jones
2019-Aug-14 19:50 UTC
Re: [Libguestfs] [libnbd PATCH 2/2] ocaml: Fix test 505
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/