Tage Johansson
2023-Aug-02 12:40 UTC
[Libguestfs] [libnbd PATCH v4 08/11] generator: Add `modifies_fd` flag to the [call] structure
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 42b9eec..6e1670d 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 @@ -274,6 +275,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. @@ -1181,6 +1183,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 @@ -1208,6 +1211,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 @@ -1221,6 +1225,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 @@ -1259,6 +1264,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 @@ -1285,6 +1291,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 @@ -1326,6 +1333,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 @@ -1357,6 +1365,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 @@ -1412,6 +1421,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 @@ -1462,6 +1472,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 @@ -1521,6 +1532,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 @@ -1750,6 +1762,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 @@ -1928,6 +1941,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>) @@ -1941,6 +1955,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 @@ -1961,6 +1976,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 @@ -1975,6 +1991,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 @@ -1996,6 +2013,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 @@ -2031,6 +2049,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 @@ -2404,6 +2423,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 @@ -2443,6 +2463,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 @@ -2535,6 +2556,7 @@ on failure." optargs = [ OFlags ("flags", cmd_flags, Some ["FUA"]) ]; 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 @@ -2568,6 +2590,7 @@ L<nbd_can_fua(3)>)." 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 @@ -2605,6 +2628,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 @@ -2625,6 +2649,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 @@ -2656,6 +2681,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 @@ -2685,6 +2711,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 @@ -2723,6 +2750,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 @@ -2789,6 +2817,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 @@ -2810,6 +2839,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 @@ -3493,6 +3523,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 @@ -3504,6 +3535,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 -- 2.41.0
Richard W.M. Jones
2023-Aug-02 16:39 UTC
[Libguestfs] [libnbd PATCH v4 08/11] generator: Add `modifies_fd` flag to the [call] structure
On Wed, Aug 02, 2023 at 12:40:53PM +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.Isn't this just saying that the call is synchronous? Rich.> --- > generator/API.ml | 32 ++++++++++++++++++++++++++++++++ > generator/API.mli | 7 +++++++ > 2 files changed, 39 insertions(+) > > diff --git a/generator/API.ml b/generator/API.ml > index 42b9eec..6e1670d 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 > @@ -274,6 +275,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. > @@ -1181,6 +1183,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 > @@ -1208,6 +1211,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 > @@ -1221,6 +1225,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 > @@ -1259,6 +1264,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 > @@ -1285,6 +1291,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 > @@ -1326,6 +1333,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 > @@ -1357,6 +1365,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 > @@ -1412,6 +1421,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 > @@ -1462,6 +1472,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 > @@ -1521,6 +1532,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 > @@ -1750,6 +1762,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 > @@ -1928,6 +1941,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>) > @@ -1941,6 +1955,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 > @@ -1961,6 +1976,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 > @@ -1975,6 +1991,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 > @@ -1996,6 +2013,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 > @@ -2031,6 +2049,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 > @@ -2404,6 +2423,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 > @@ -2443,6 +2463,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 > @@ -2535,6 +2556,7 @@ on failure." > optargs = [ OFlags ("flags", cmd_flags, Some ["FUA"]) ]; > 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 > @@ -2568,6 +2590,7 @@ L<nbd_can_fua(3)>)." > 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 > @@ -2605,6 +2628,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 > @@ -2625,6 +2649,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 > @@ -2656,6 +2681,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 > @@ -2685,6 +2711,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 > @@ -2723,6 +2750,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 > @@ -2789,6 +2817,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 > @@ -2810,6 +2839,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 > @@ -3493,6 +3523,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 > @@ -3504,6 +3535,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 > -- > 2.41.0-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v