Re: [Libguestfs] [libnbd PATCH v2 2/5] ocaml: tests: Fix error handling

Like a lot of the C examples, the aio copy test ignores read and write errors in the completion callback, which can cause silent data corruption. The failure in the test is not critical, but this is a bad example that may be copied by developers to a real application. The test dies with an assertion failure now if completion callback fails. Tested with the temporary patch of: | diff --git i/ocaml/tests/test_590_aio_copy.ml w/ocaml/tests/test_590_aio_copy.ml | index a0339c8b..332bf31b 100644 | --- i/ocaml/tests/test_590_aio_copy.ml | +++ w/ocaml/tests/test_590_aio_copy.ml | @@ -120,7 +120,8 @@ let | let dst = NBD.create () in | NBD.set_handle_name dst "dst"; | NBD.connect_command src ["nbdkit"; "-s"; "--exit-with-parent"; "-r"; | - "pattern"; sprintf "size=%d" disk_size]; | + "--filter=error"; "pattern"; "error-pread-rate=1"; | + sprintf "size=%d" disk_size]; | NBD.connect_command dst ["nbdkit"; "-s"; "--exit-with-parent"; | "memory"; sprintf "size=%d" disk_size]; | asynch_copy src dst --- ocaml/tests/test_590_aio_copy.ml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ocaml/tests/test_590_aio_copy.ml b/ocaml/tests/test_590_aio_copy.ml index 11d89256..a0339c8b 100644 --- a/ocaml/tests/test_590_aio_copy.ml +++ b/ocaml/tests/test_590_aio_copy.ml @@ -1,6 +1,6 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* libnbd OCaml test case - * Copyright (C) 2013-2019 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 @@ -45,7 +45,8 @@ let * next iteration of the loop. *) let writes = ref [] in - let read_completed buf offset _ = + let read_completed buf offset err = + assert (!err = 0); bytes_read := !bytes_read + NBD.Buffer.size buf; (* Get ready to issue a write command. *) writes := (buf, offset) :: !writes; @@ -56,7 +57,8 @@ let (* This callback is called when any pwrite to the destination * has completed. *) - let write_completed buf _ = + let write_completed buf err = + assert (!err = 0); bytes_written := !bytes_written + NBD.Buffer.size buf; (* By returning 1 here we auto-retire the pwrite command. *) 1