Re: [Libguestfs] [libnbd PATCH v3 14/18] tests: Add coverage for nbd_set_export_name fix
On Tue, Sep 27, 2022 at 12:32:15PM +0100, Richard W.M. Jones wrote:
On Mon, Sep 26, 2022 at 05:05:56PM -0500, Eric Blake wrote:
[...]
> @@ -102,6 +99,16 @@ main (int argc, char *argv[])
> exit (EXIT_FAILURE);
> }
>
> + /* info on something not present fails */
> + if (nbd_set_export_name (nbd, "a") == -1) {
> + fprintf (stderr, "%s\n", nbd_get_error ());
> + exit (EXIT_FAILURE);
> + }
> + if (nbd_opt_info (nbd) != -1) {
> + fprintf (stderr, "expecting error for opt_info\n");
> + exit (EXIT_FAILURE);
> + }
While looking at this, I found this which seems like it may be wrong
(lib/opt.c):
/* Issue NBD_OPT_INFO and wait for the reply. */
int
nbd_unlocked_opt_info (struct nbd_handle *h)
...
r = wait_for_option (h);
if (r == 0 && err) {
assert (nbd_internal_is_state_negotiating (get_next_state (h)) ||
nbd_internal_is_state_dead (get_next_state (h)));
set_error (err, "server replied with error to opt_info request");
I believe that r == 0 && err != 0 => the callback set *err; in which
case the error message is wrong?
You are correct that r==0 and err!=0 is only possible if the callback
set *err. But the error message is correct - the callback is passed a
non-zero parameter for its err argument in precisely the cases where
the server reported an error (NBD_REP_ERR*) rather than success
(NBD_REP_ACK) to the result of the NBD_OPT_INFO call (see
generator/states-newstyle-opt-go.c and search for CALLBACK), but where
the connection is still alive to try further negotiation commands.
The callback is taking that knowledge from the server and storing it
back in the stack variable 'err' that the synchronous code can then
use to flag the server's response.
See also commits bbf1c513 (where the completion callback was
implemented) and 517515e5 (where it was used to implement synchronous
nbd_opt_info).
It's easy to write a one-line hack to nbdkit to provoke this code, but
I don't know of any existing server that makes this easy to trigger
without a recompile.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org