On Thu, Aug 03, 2023 at 03:30:36PM +0000, Tage Johansson wrote:
On 8/2/2023 6:39 PM, Richard W.M. Jones wrote:
>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.
Kind of, it is true for all synchronous commands and the functions
aio_notify_read() and aio_notify_write(). I could mention those
explicitly though and rename the flag to something like
"is_synchronous", if you want?
No it'll be fine, thanks.
Rich.
Best regards,
Tage
>>---
>> 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
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW