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@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