Re: [Libguestfs] [libnbd PATCH 0/4] Work towards fixing nbdcopy silent corruption

On Wed, Feb 02, 2022 at 09:59:04AM +0100, Laszlo Ersek wrote: > However, in extent_wrapper() -> extent_wrapper_locked(), there is more > confusion for me: > > metacontextv = caml_copy_string (metacontext); > offsetv = caml_copy_int64 (offset); > entriesv = nbd_internal_ocaml_alloc_int64_from_uint32_array (entries, nr_entries); > errorv = caml_alloc_tuple (1); > Store_field (errorv, 0, Val_int (*error)); > args[0] = metacontextv; > args[1] = offsetv; > args[2] = entriesv; > args[3] = errorv; > rv = caml_callbackN_exn (data->fnv, 4, args); > *error = Int_val (Field (errorv, 0)); > if (Is_exception_result (rv)) { > nbd_internal_ocaml_exception_in_wrapper ("extent", rv); > CAMLreturnT (int, -1); > } > > r = Int_val (rv); > assert (r >= 0); > CAMLreturnT (int, r); > > If I understand correctly, if we returned (-1) from the (nameless) > extents callback, that would trip the assert(); would it not? Thus, we > would not reach the nbd_internal_ocaml_raise_error() in the above-quoted > nbd_internal_ocaml_nbd_block_status() function. There are five cases: - Callback returns > 1 => The program shouldn't do this, but it's treated the same as the next case. - Callback returns 1 => For completion callbacks this indicates that the callback was successful and you want to autoretire the command. For other types of callbacks the program shouldn't do this but it is the same as the next case. - Callback returns 0 => Indicates that the callback was successful. - Callback throws an exception => nbd_internal_ocaml_exception_in_wrapper is called => Callback returns -1 indicating (to libnbd) that there was an error. cmd->error is updated with !err || EPROTO. - Callback returns < 0 => assert fails! The assert is a bit harsh, but it indicates a bug in the OCaml code. It probably means the OCaml code thought that returning -1 indicates an error, whereas the OCaml code should throw an exception instead. For the history of why that assert was added see: https://gitlab.com/nbdkit/libnbd/-/commit/d881d160e1cd9c9964782300a7652ff...