[Libguestfs] [libnbd PATCH v2 12/13] wip: api: Give aio_opt_go a completion callback
Richard W.M. Jones
rjones at redhat.com
Mon Aug 17 11:47:55 UTC 2020
On Fri, Aug 14, 2020 at 05:00:31PM -0500, Eric Blake wrote:
> Squash this into opt_go? into opt_info? Standalone?
>
> Testing: why does python not throw an exception:
> $ ./run nbdsh
>
> Welcome to nbdsh, the shell for interacting with
> Network Block Device (NBD) servers.
>
> The ‘nbd’ module has already been imported and there
> is an open NBD handle called ‘h’.
>
> h.connect_tcp ("remote", "10809") # Connect to a remote server.
> h.get_size () # Get size of the remote disk.
> buf = h.pread (512, 0, 0) # Read the first sector.
> exit() or Ctrl-D # Quit the shell
> help (nbd) # Display documentation
>
> nbd> h.set_opt_mode(True)
> nbd> h.connect_command(["nbdkit","-s","--exit-","eval",
> "open=echo ENOENT >&2; exit 1"])
> nbd> h.aio_is_negotiating()
> True
> nbd> h.set_export_name('a')
> nbd> h.opt_go()
> nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENT
> nbd> h.set_export_name('b')
> nbd> h.opt_go()
> nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENT
I don't know, but I would suggest first of all calling
h.set_debug(True) which will display what the C API nbd_opt_go is
actually returning.
If it's not returning an error then Python bindings won't throw an
exception (perhaps it just skips through the states if h->opt_mode is
false?). I'm fairly sure the Python bindings themselves should be
robust and always throw an exception if the underlying C returns an
error, but if you suspect a problem them have a look at the generated
code in python/methods.c for your new function.
The patch itself looks like a good idea.
Rich.
> nbd> h.set_opt_mode(True)
> Traceback (most recent call last):
> File "/usr/lib64/python3.8/code.py", line 90, in runcode
> exec(code, self.locals)
> File "<console>", line 1, in <module>
> File "/home/eblake/libnbd/python/nbd.py", line 570, in set_opt_mode
> return libnbdmod.set_opt_mode (self._o, enable)
> nbd.Error: nbd_set_opt_mode: invalid state: NEGOTIATING: the handle must be newly created: Invalid argument (EINVAL)
> nbd> quit()
> ---
> generator/API.ml | 12 ++++++++++--
> generator/states-newstyle-opt-go.c | 19 +++++++++++++------
> lib/opt.c | 18 ++++++++++++++++--
> 3 files changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/generator/API.ml b/generator/API.ml
> index 52970d3..bf50030 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -1876,7 +1876,9 @@ on the connection.";
>
> "aio_opt_go", {
> default_call with
> - args = []; ret = RErr;
> + args = [];
> + optargs = [ OClosure completion_closure ];
> + ret = RErr;
> permitted_states = [ Negotiating ];
> shortdesc = "end negotiation and move on to using an export";
> longdesc = "\
> @@ -1885,7 +1887,13 @@ export previously specified by L<nbd_set_export_name(3)>. This can only
> be used if L<nbd_set_opt_mode(3)> enabled option mode.
>
> To determine when the request completes, wait for
> -L<nbd_aio_is_connecting(3)> to return false.";
> +L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
> +C<completion_callback> which will be invoked as described in
> +L<libnbd(3)/Completion callbacks>, except that it is automatically
> +retired regardless of return value. Note that detecting whether the
> +server returns an error (as is done by the return value of the
> +synchronous counterpart) is only possible with a completion
> +callback.";
> see_also = [Link "set_opt_mode"; Link "opt_go"];
> };
>
> diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
> index 0568695..95cc041 100644
> --- a/generator/states-newstyle-opt-go.c
> +++ b/generator/states-newstyle-opt-go.c
> @@ -124,6 +124,7 @@ STATE_MACHINE {
> uint64_t exportsize;
> uint16_t eflags;
> uint32_t min, pref, max;
> + int err;
>
> reply = be32toh (h->sbuf.or.option_reply.reply);
> len = be32toh (h->sbuf.or.option_reply.replylen);
> @@ -241,15 +242,21 @@ STATE_MACHINE {
> reply);
> }
> nbd_internal_reset_size_and_flags (h);
> - if (h->opt_mode)
> - SET_NEXT_STATE (%.NEGOTIATING);
> - else
> - SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
> + err = nbd_get_errno ();
> break;
> case NBD_REP_ACK:
> + err = 0;
> + break;
> + }
> +
> + CALL_CALLBACK (h->opt_cb.completion, &err);
> + nbd_internal_free_option (h);
> + if (err == 0)
> SET_NEXT_STATE (%^FINISHED);
> - break;
> - }
> + else if (h->opt_mode)
> + SET_NEXT_STATE (%.NEGOTIATING);
> + else
> + SET_NEXT_STATE (%^PREPARE_OPT_ABORT);
> return 0;
>
> } /* END STATE MACHINE */
> diff --git a/lib/opt.c b/lib/opt.c
> index 1a5f645..17f2508 100644
> --- a/lib/opt.c
> +++ b/lib/opt.c
> @@ -60,16 +60,28 @@ wait_for_option (struct nbd_handle *h)
> return 0;
> }
>
> +static int
> +go_complete (void *opaque, int *err)
> +{
> + int *i = opaque;
> + *i = *err;
> + return 0;
> +}
> +
> /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) and wait for the reply. */
> int
> nbd_unlocked_opt_go (struct nbd_handle *h)
> {
> - int r = nbd_unlocked_aio_opt_go (h);
> + int err;
> + nbd_completion_callback c = { .callback = go_complete, .user_data = &err };
> + int r = nbd_unlocked_aio_opt_go (h, c);
>
> if (r == -1)
> return r;
>
> r = wait_for_option (h);
> + if (r == 0)
> + r = err;
> if (r == 0 && nbd_internal_is_state_negotiating (get_next_state (h)))
> return -1; /* NBD_OPT_GO failed, but can be attempted again */
> return r;
> @@ -132,9 +144,11 @@ nbd_unlocked_opt_list (struct nbd_handle *h, nbd_list_callback list)
>
> /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */
> int
> -nbd_unlocked_aio_opt_go (struct nbd_handle *h)
> +nbd_unlocked_aio_opt_go (struct nbd_handle *h,
> + nbd_completion_callback complete)
> {
> h->current_opt = NBD_OPT_GO;
> + h->opt_cb.completion = complete;
>
> if (nbd_internal_run (h, cmd_issue) == -1)
> debug (h, "option queued, ignoring state machine failure");
> --
> 2.28.0
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
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
More information about the Libguestfs
mailing list