On 7/21/2023 12:05 PM, Richard W.M. Jones wrote:
On Fri, Jul 21, 2023 at 09:58:52AM +0000, Tage Johansson wrote:
On 7/19/2023 4:47 PM, Richard W.M. Jones wrote:
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:
Interesting .. Is this used for all libnbd Rust handles?
Yes, both for `libnbd::Handle` and `libnbd::AsyncHandle`.
`libnbd::AsyncHandle` is built ontop of `libnbd::Handle`.
```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?
I'm a bit unclear if the debugging is necessary or not. In the other
bindings we don't set up a log handler by default, it just works the
same way as the C API.
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?
Yup, it was already clear to me why these wouldn't work after I
thought about it a bit more.
My issue for maintenance is that if we add a new "poll-like" function
to the API then the Rust bindings would need a special change.
Sorry, but what do you mean by "would need a special change"?
Best regards,
Tage
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.
Sure, thanks!
Rich.
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
