[Libguestfs] [libnbd PATCH v7 6/9] generator: Add `modifies_fd` flag to the [call] structure
Richard W.M. Jones
rjones at redhat.com
Tue Aug 15 08:54:56 UTC 2023
On Thu, Aug 10, 2023 at 11:24:33AM +0000, Tage Johansson wrote:
> Add a flag (modifies_fd) to the call structure in generator/API.ml
> which is set to true if the handle call may do something with the
> file descriptor. That is, it is true for all calls which are or may
> call aio_notify_*, including all synchronous commands like
> nbd_connect or nbd_opt_go.
>
> The motivation for this is that the asynchronous handle in the Rust
> bindings uses its own loop for polling, modifying the file descriptor
> outside of this loop may cause unexpected behaviour. Including that the
> handle may hang. All commands which set this flag to true will be
> excluded from that handle. The asynchronous (aio_*) functions will be
> used instead.
> ---
> generator/API.ml | 32 ++++++++++++++++++++++++++++++++
> generator/API.mli | 7 +++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/generator/API.ml b/generator/API.ml
> index 41a6dd1..ada2b06 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -33,6 +33,7 @@ type call = {
> is_locked : bool;
> may_set_error : bool;
> async_kind : async_kind option;
> + modifies_fd: bool;
> mutable first_version : int * int;
> }
> and arg =
> @@ -275,6 +276,7 @@ let default_call = { args = []; optargs = []; ret = RErr;
> permitted_states = [];
> is_locked = true; may_set_error = true;
> async_kind = None;
> + modifies_fd = false;
> first_version = (0, 0) }
>
> (* Calls.
> @@ -1182,6 +1184,7 @@ Return true if option negotiation mode was enabled on this handle.";
> default_call with
> args = []; ret = RErr;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "end negotiation and move on to using an export";
> longdesc = "\
> Request that the server finish negotiation and move on to serving the
> @@ -1209,6 +1212,7 @@ although older servers will instead have killed the connection.";
> default_call with
> args = []; ret = RErr;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "end negotiation and close the connection";
> longdesc = "\
> Request that the server finish negotiation, gracefully if possible, then
> @@ -1222,6 +1226,7 @@ enabled option mode.";
> default_call with
> args = []; ret = RBool;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "request the server to initiate TLS";
> longdesc = "\
> Request that the server initiate a secure TLS connection, by
> @@ -1260,6 +1265,7 @@ established, as reported by L<nbd_get_tls_negotiated(3)>.";
> default_call with
> args = []; ret = RBool;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "request the server to enable structured replies";
> longdesc = "\
> Request that the server use structured replies, by sending
> @@ -1286,6 +1292,7 @@ later calls to this function return false.";
> default_call with
> args = [ Closure list_closure ]; ret = RInt;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "request the server to list all exports during negotiation";
> longdesc = "\
> Request that the server list all exports that it supports. This can
> @@ -1327,6 +1334,7 @@ description is set with I<-D>.";
> default_call with
> args = []; ret = RErr;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "request the server for information about an export";
> longdesc = "\
> Request that the server supply information about the export name
> @@ -1358,6 +1366,7 @@ corresponding L<nbd_opt_go(3)> would succeed.";
> default_call with
> args = [ Closure context_closure ]; ret = RInt;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "list available meta contexts, using implicit query list";
> longdesc = "\
> Request that the server list available meta contexts associated with
> @@ -1413,6 +1422,7 @@ a server may send a lengthy list.";
> default_call with
> args = [ StringList "queries"; Closure context_closure ]; ret = RInt;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "list available meta contexts, using explicit query list";
> longdesc = "\
> Request that the server list available meta contexts associated with
> @@ -1463,6 +1473,7 @@ a server may send a lengthy list.";
> default_call with
> args = [ Closure context_closure ]; ret = RInt;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "select specific meta contexts, using implicit query list";
> longdesc = "\
> Request that the server supply all recognized meta contexts
> @@ -1522,6 +1533,7 @@ no contexts are reported, or may fail but have a non-empty list.";
> default_call with
> args = [ StringList "queries"; Closure context_closure ]; ret = RInt;
> permitted_states = [ Negotiating ];
> + modifies_fd = true;
> shortdesc = "select specific meta contexts, using explicit query list";
> longdesc = "\
> Request that the server supply all recognized meta contexts
> @@ -1751,6 +1763,7 @@ parameter in NBD URIs is allowed.";
> default_call with
> args = [ String "uri" ]; ret = RErr;
> permitted_states = [ Created ];
> + modifies_fd = true;
> shortdesc = "connect to NBD URI";
> longdesc = "\
> Connect (synchronously) to an NBD server and export by specifying
> @@ -1929,6 +1942,7 @@ See L<nbd_get_uri(3)>.";
> default_call with
> args = [ Path "unixsocket" ]; ret = RErr;
> permitted_states = [ Created ];
> + modifies_fd = true;
> shortdesc = "connect to NBD server over a Unix domain socket";
> longdesc = "\
> Connect (synchronously) over the named Unix domain socket (C<unixsocket>)
> @@ -1942,6 +1956,7 @@ to an NBD server running on the same machine.
> default_call with
> args = [ UInt32 "cid"; UInt32 "port" ]; ret = RErr;
> permitted_states = [ Created ];
> + modifies_fd = true;
> shortdesc = "connect to NBD server over AF_VSOCK protocol";
> longdesc = "\
> Connect (synchronously) over the C<AF_VSOCK> protocol from a
> @@ -1962,6 +1977,7 @@ built on a system with vsock support, see L<nbd_supports_vsock(3)>.
> default_call with
> args = [ String "hostname"; String "port" ]; ret = RErr;
> permitted_states = [ Created ];
> + modifies_fd = true;
> shortdesc = "connect to NBD server over a TCP port";
> longdesc = "\
> Connect (synchronously) to the NBD server listening on
> @@ -1976,6 +1992,7 @@ such as C<\"10809\">.
> default_call with
> args = [ Fd "sock" ]; ret = RErr;
> permitted_states = [ Created ];
> + modifies_fd = true;
> shortdesc = "connect directly to a connected socket";
> longdesc = "\
> Pass a connected socket C<sock> through which libnbd will talk
> @@ -1997,6 +2014,7 @@ handle is closed. The caller must not use the socket in any way.
> default_call with
> args = [ StringList "argv" ]; ret = RErr;
> permitted_states = [ Created ];
> + modifies_fd = true;
> shortdesc = "connect to NBD server command";
> longdesc = "\
> Run the command as a subprocess and connect to it over
> @@ -2032,6 +2050,7 @@ is killed.
> default_call with
> args = [ StringList "argv" ]; ret = RErr;
> permitted_states = [ Created ];
> + modifies_fd = true;
> shortdesc = "connect using systemd socket activation";
> longdesc = "\
> Run the command as a subprocess and connect to it using
> @@ -2405,6 +2424,7 @@ requests sizes.
> optargs = [ OFlags ("flags", cmd_flags, Some []) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "read from the NBD server";
> longdesc = "\
> Issue a read command to the NBD server for the range starting
> @@ -2444,6 +2464,7 @@ on failure."
> optargs = [ OFlags ("flags", cmd_flags, Some ["DF"]) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "read from the NBD server";
> longdesc = "\
> Issue a read command to the NBD server for the range starting
> @@ -2536,6 +2557,7 @@ on failure."
> optargs = [ OFlags ("flags", cmd_flags, Some ["FUA"; "PAYLOAD_LEN"]) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "write to the NBD server";
> longdesc = "\
> Issue a write command to the NBD server, writing the data in
> @@ -2572,6 +2594,7 @@ extended headers were negotiated."
> args = []; optargs = [ OFlags ("flags", shutdown_flags, None) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "disconnect from the NBD server";
> longdesc = "\
> Issue the disconnect command to the NBD server. This is
> @@ -2609,6 +2632,7 @@ A future version of the library may add new flags.";
> default_call with
> args = []; optargs = [ OFlags ("flags", cmd_flags, Some []) ]; ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "send flush command to the NBD server";
> longdesc = "\
> Issue the flush command to the NBD server. The function should
> @@ -2629,6 +2653,7 @@ protocol extensions)."
> optargs = [ OFlags ("flags", cmd_flags, Some ["FUA"]) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "send trim command to the NBD server";
> longdesc = "\
> Issue a trim command to the NBD server, which if supported
> @@ -2660,6 +2685,7 @@ L<nbd_can_fua(3)>)."
> optargs = [ OFlags ("flags", cmd_flags, Some []) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "send cache (prefetch) command to the NBD server";
> longdesc = "\
> Issue the cache (prefetch) command to the NBD server, which
> @@ -2689,6 +2715,7 @@ protocol extensions)."
> Some ["FUA"; "NO_HOLE"; "FAST_ZERO"]) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "send write zeroes command to the NBD server";
> longdesc = "\
> Issue a write zeroes command to the NBD server, which if supported
> @@ -2727,6 +2754,7 @@ cannot do this, see L<nbd_can_fast_zero(3)>)."
> optargs = [ OFlags ("flags", cmd_flags, Some ["REQ_ONE"]) ];
> ret = RErr;
> permitted_states = [ Connected ];
> + modifies_fd = true;
> shortdesc = "send block status command to the NBD server";
> longdesc = "\
> Issue the block status command to the NBD server. If
> @@ -2793,6 +2821,7 @@ validate that the server obeyed the flag."
> "poll", {
> default_call with
> args = [ Int "timeout" ]; ret = RInt;
> + modifies_fd = true;
> shortdesc = "poll the handle once";
> longdesc = "\
> This is a simple implementation of L<poll(2)> which is used
> @@ -2814,6 +2843,7 @@ intended as something you would use.";
> "poll2", {
> default_call with
> args = [Fd "fd"; Int "timeout" ]; ret = RInt;
> + modifies_fd = true;
> shortdesc = "poll the handle once, with fd";
> longdesc = "\
> This is the same as L<nbd_poll(3)>, but an additional
> @@ -3497,6 +3527,7 @@ and invalidate the need to write more commands.
> "aio_notify_read", {
> default_call with
> args = []; ret = RErr;
> + modifies_fd = true;
> shortdesc = "notify that the connection is readable";
> longdesc = "\
> Send notification to the state machine that the connection
> @@ -3508,6 +3539,7 @@ connection is readable.";
> "aio_notify_write", {
> default_call with
> args = []; ret = RErr;
> + modifies_fd = true;
> shortdesc = "notify that the connection is writable";
> longdesc = "\
> Send notification to the state machine that the connection
> diff --git a/generator/API.mli b/generator/API.mli
> index ff85849..4bf4468 100644
> --- a/generator/API.mli
> +++ b/generator/API.mli
> @@ -41,6 +41,13 @@ type call = {
> if the function is asynchronous and in that case how one can check if
> it has completed. *)
> async_kind : async_kind option;
> + (** A flag telling if the call may do something with the file descriptor.
> + Some bindings needs exclusive access to the file descriptor and can not
> + allow the user to call [aio_notify_read] or [aio_notify_write], neither
> + directly nor indirectly from another call. So all calls that might trigger
> + any of these functions to be called, including all synchronous commands
> + like [pread] or [connect], should set this to [true]. *)
> + modifies_fd : bool;
> (** The first stable version that the symbol appeared in, for
> example (1, 2) if the symbol was added in development cycle
> 1.1.x and thus the first stable version was 1.2. This is
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
More information about the Libguestfs
mailing list