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

On Wed, Feb 02, 2022 at 09:23:09AM +0100, Laszlo Ersek wrote: The documentation is not great, but I can tell you that they are called in the order: - extent_cb (usually once, but in theory several times) - completion_cb (once at the end) The *error parameter is also not well documented, but the behaviour is: - There is a single error value stored for the asynch block status command (cmd->error). - *error is initialized from cmd->error on entry to the callback, ie. *error != 0 if an error has been reported by a previous callback. - If the callback sets *error != 0 _and_ returns -1 then cmd->error is updated. - The callback can also fail by returning -1 and not setting *error, in which case cmd->error is set to EPROTO (but don't do this as it means your program is buggy - but see below about OCaml). - Since there's only one error value, updating it overwrites previous values. - Returning -1 doesn't stop future callbacks, because basically we're pulling data off the wire and there's no way to tell the server to stop. So extent and completion callbacks are still called. - Don't forget that its not just callbacks that can fail, commands can fail for other reasons, eg. network errors. In this case the whole connection will drop into what we call the %DEAD state (because NBD can't resynchronize). Every call returns -1 and you have to close and reopen the connection. It's possible to explicitly test for this (nbd_aio_is_dead). https://gitlab.com/nbdkit/libnbd/-/blob/1373d423f5ac42255fe606d2a7ab1caf1... https://gitlab.com/nbdkit/libnbd/-/blob/1373d423f5ac42255fe606d2a7ab1caf1... To find out if a command had an error you can do either (but not both): - Check for *error != 0 in completion_cb. Return 1 from completion_cb which auto-retires the command. - Call nbd_aio_command_completed which will return -1 and then call nbd_get_errno which will return cmd->error. This also retires the command. (Note in this case you also have to deal with nbd_aio_command_completed returning 0, meaning the command has not completed, or 1, meaning the command completed without error). The first method is what nbdcopy should be doing but isn't. The above applies to synch commands as well, with the difference that: - There's no completion callback (because there's no need for it in a synch call). - Once the command completes, it is retired and the error status is copied to nbd_get_errno. - The synch command returns -1 if there was an error or 0 if successful. See lib/rw.c: wait_for_command and nbd_unlocked_block_status. So in answer to your question, virt-v2v can ignore !err, unless there is a reason why you want to know if previous callbacks encountered an error (maybe you want to save some work). You'll still get an exception thrown by NBD.block_status. All OCaml wrappers check for the underlying nbd_* call failing and turn that into an exception which carries the errno and error string. See ocaml/nbd-c.c and ocaml/helpers.c:nbd_internal_ocaml_raise_error In addition, when calling back into OCaml code (ie. into the extent callback), if the callback throws an exception then that is automatically turned into setting *error from the err reference and returning -1. OCaml code ought to do this when encountering an error inside the callback: if some_error then ( err := errno; raise AnException ) But there are a number of difficulties with that in practice: - OCaml functions that you call might raise exceptions which you don't catch, so you don't end up setting !err on all exceptions. - OCaml code doesn't have access to C errno values! I wouldn't worry too much about this. What happens in practice is that nbd_internal_ocaml_exception_in_wrapper prints this on stderr: libnbd: <operation>: uncaught OCaml exception: <exception name> cmd->error is set to EPROTO and the function as a whole throws some NBD.Error exception. The error doesn't get lost in this case, it's just that the output is not ideal. See ocaml/nbd-c.c:extent_wrapper_locked and ocaml/helpers.c:nbd_internal_ocaml_exception_in_wrapper Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW