On Wed, Jul 19, 2023 at 09:09:53AM +0000, Tage Johansson wrote:Create another handle type: `AsyncHandle`, which makes use of Rust's builtin asynchronous functions (see <https://doc.rust-lang.org/std/keyword.async.html>) and runs on top of the Tokio runtime (see <https://docs.rs/tokio>). For every asynchronous command, like `aio_connect()`, a corresponding `async` method is created on the handle. In this case it would be: async fn connect(...) -> Result<(), ...> When called, it will poll the file descriptor until the command is complete, and then return with a result. All the synchronous counterparts (like `nbd_connect()`) are excluded from this handle type as they are unnecessary and since they might interfear with the polling made by the Tokio runtime. For more details about how the asynchronous commands are executed, please see the comments in rust/src/async_handle.rs.This is an interesting (and good) approach, layering a more natural API for Rust users on top of the "low level" API.--- generator/Rust.ml | 232 +++++++++++++++++++++++++++++++++++++++ generator/Rust.mli | 2 + generator/generator.ml | 1 + rust/Cargo.toml | 1 + rust/Makefile.am | 1 + rust/src/async_handle.rs | 222 +++++++++++++++++++++++++++++++++++++ rust/src/lib.rs | 4 + 7 files changed, 463 insertions(+) create mode 100644 rust/src/async_handle.rs diff --git a/generator/Rust.ml b/generator/Rust.ml index 96095a9..1048831 100644 --- a/generator/Rust.ml +++ b/generator/Rust.ml @@ -601,3 +601,235 @@ let generate_rust_bindings () = pr "impl Handle {\n"; List.iter print_rust_handle_method handle_calls; pr "}\n\n" + +(*********************************************************) +(* The rest of the file conserns the asynchronous API. *) +(* *) +(* See the comments in rust/src/async_handle.rs for more *) +(* information about how it works. *) +(*********************************************************) + +let excluded_handle_calls : NameSet.t = + NameSet.of_list + [ + "aio_get_fd"; + "aio_get_direction"; + "aio_notify_read"; + "aio_notify_write"; + "clear_debug_callback"; + "get_debug"; + "poll"; + "poll2"; + "set_debug"; + "set_debug_callback"; + ]This is a "code smell" since all information that is specific to APIs should (usually) go in generator/API.ml. It's reasonable for aio_get_{fd,direction} aio_notify_{read,write} since those are very special APIs, but I don't understand why the poll and debug functions are listed here. What's the real meaning of this list of functions, ie. why can each one not be included in the tokio bindings?
The debug functions are used for setting up logging with the log crate in rust/src/handle.rs:
```Rust
impl Handle {
pub fn new() -> Result<Self> {
let handle = unsafe { sys::nbd_create() };
if handle.is_null() {
Err(unsafe {
Error::Fatal(ErrorKind::get_error().into()) })
} else {
// Set a debug callback communicating with any logging
// implementation as defined by the log crate.
#[allow(unused_mut)]
let mut nbd = Handle { handle };
#[cfg(feature = "log")]
{
nbd.set_debug_callback(|func_name, msg| {
log::debug!(
target:
func_name.to_string_lossy().as_ref(),
"{}",
msg.to_string_lossy()
);
0
})?;
nbd.set_debug(true)?;
}
Ok(nbd)
}
}
```
So if the user would set another debug callback this functionality would stop working. And since the documentation is automaticly generated for `nbd_set_debug_callback()`, the user might not expect that logging just stops working because they set a debug callback. But I guess that I can add a note about that to the documentation of the `Handle` struct and include the debug callbacks, if you want?
Regarding the other methods, it's a bit more complicated. The problem is that calling any `aio_notify_*` method might cause any ongoing command to finish. For example, the `connect()` method on `AsyncHandle` will not return until `aio_is_connecting()` returns false. In order not to have to check that in a busy loop, the function is woken up only when the polling task makes a call to any `aio_notify_*` function. So whenever the polling task calls `aio_notify_*`, it notifies the waiting command that the handle might has changed state. But if the user calls `aio_notify_*` (directly or indirectly) while `aio_connect` is in flight, it might happen that the connection completes without the call to `connect()` gets notified about that.
This means that any function that calls any `aio_notify_*` function should not be implemented on `AsyncHandle`. A better approach than the current would probably be to add a field to the `call`-structure in API.ml. Something like `notifies_fd : bool` (sure someone can come up with a better name). This would be set to true for any function that calls `aio_notify_*`, including `aio_notify_read()`, `poll()`, and all synchronous commands like `nbd_connect()`. What do you think about that?
I have written some more explonations about how the asynchronous handle works in rust/src/async_handle.rs, but it is somewhat complicated so please ask if there are more unclear things.
+ +(* A mapping with names as keys. *) +module NameMap = Map.Make (String) + +(* Strip "aio_" from the beginning of a string. *) +let strip_aio name : string = + if String.starts_with ~prefix:"aio_" name then + String.sub name 4 (String.length name - 4) + else failwith (sprintf "Asynchronous call %s must begin with aio_" name) + +(* A map with all asynchronous handle calls. The keys are names with "aio_" + stripped, the values are a tuple with the actual name (with "aio_"), the + [call] and the [async_kind]. *) +let async_handle_calls : ((string * call) * async_kind) NameMap.t = + handle_calls + |> List.filter (fun (n, _) -> not (NameSet.mem n excluded_handle_calls)) + |> List.filter_map (fun (name, call) -> + call.async_kind + |> Option.map (fun async_kind -> + (strip_aio name, ((name, call), async_kind)))) + |> List.to_seq |> NameMap.of_seq + +(* A mapping with all synchronous (not asynchronous) handle calls. Excluded + are also all synchronous calls that has an asynchronous counterpart. So if + "foo" is the name of a handle call and an asynchronous call "aio_foo" + exists, then "foo" will not b in this map. *) +let sync_handle_calls : call NameMap.t = + handle_calls + |> List.filter (fun (n, _) -> not (NameSet.mem n excluded_handle_calls)) + |> List.filter (fun (name, _) -> + (not (NameMap.mem name async_handle_calls)) + && not + (String.starts_with ~prefix:"aio_" name + && NameMap.mem (strip_aio name) async_handle_calls)) + |> List.to_seq |> NameMap.of_seq + +(* Get the Rust type for an argument in the asynchronous API. Like + [rust_arg_type] but no static lifetime on some closures and buffers. *) +let rust_async_arg_type : arg -> string = function + | Closure { cbargs; cbkind } -> + let lifetime = + match cbkind with + | CBOnceCommand | CBManyCommand -> None + | CBManyHandle -> Some "'static" + in + "impl " ^ rust_closure_trait ~lifetime cbargs cbkind + | BytesPersistIn _ -> "&[u8]" + | BytesPersistOut _ -> "&mut [u8]" + | x -> rust_arg_type x + +(* Get the Rust type for an optional argument in the asynchronous API. Like + [rust_optarg_type] but no static lifetime on some closures. *) +let rust_async_optarg_type : optarg -> string = function + | OClosure x -> sprintf "Option<%s>" (rust_async_arg_type (Closure x)) + | x -> rust_optarg_type x + +(* A string of the argument list for a method on the handle, with both + mandotory and optional arguments. *) +let rust_async_handle_call_args { args; optargs } : string = + let rust_args_names = + List.map rust_arg_name args @ List.map rust_optarg_name optargs + and rust_args_types = + List.map rust_async_arg_type args + @ List.map rust_async_optarg_type optargs + in + String.concat ", " + (List.map2 + (fun name typ -> name ^ ": " ^ typ)Often sprintf is nicer, and in this case it really works well: (List.map2 (sprintf "%s: %s") rust_args_names rust_args_types) OCaml will actually generate identical code for this as for your version, since sprintf is handled as a kind of macro. You will need "open Printf" at the top of the file, if it is not there already.
I will fix that.
Best regards,
Tage
+(* Print the Rust function for a not asynchronous handle call. *) +let print_rust_sync_handle_call (name : string) (call : call) = + print_rust_handle_call_comment call; + pr "pub fn %s(&self, %s) -> %s\n" name + (rust_async_handle_call_args call) + (rust_ret_type call); + print_ffi_call name "self.data.handle.handle" call; + pr "\n" + +(* Print the Rust function for an asynchronous handle call with a completion + callback. (Note that "callback" might be abbreviated with "cb" in the + following code. *) +let print_rust_async_handle_call_with_completion_cb name (aio_name, call) = + (* An array of all optional arguments. Useful because we need to deel with + the index of the completion callback. *) + let optargs = Array.of_list call.optargs in + (* The index of the completion callback in [optargs] *) + let completion_cb_index = + Array.find_map + (fun (i, optarg) -> + match optarg with + | OClosure { cbname } -> + if cbname = "completion" then Some i else None + | _ -> None) + (Array.mapi (fun x y -> (x, y)) optargs) + in + let completion_cb_index = + match completion_cb_index with + | Some x -> x + | None -> + failwith + (sprintf + "The handle call %s is claimed to have a completion callback \ + among its optional arguments by the async_kind field, but so \ + does not seem to be the case." + aio_name)Conversely, here you an just use "failwithf" (failwith + sprintf)! Rich.