Libguestfs - Aug 2023 - [libnbd PATCH v7 9/9] rust: async: Add an example

So what do I think about the patch series as a whole ...  (in
particular, the patches I didn't add Reviewed-by tags to).

It would be much nicer IMHO if we didn't have to define callback
lifetimes in this way, since they were not intended to be classified
into async_kind / cblifetime / cbcount, and this might limit our
options for new ABIs in future.

I see two ways to go here:

(1) (Easier for now, problems in future) Rename async_kind, cblifetime
and cbcount as rust_async_kind, rust_cblifetime, rust_cbcount, which
would in some sense limit the scope of getting these right to the Rust
bindings.

This defers the pain til later (maybe never, if we never added an ABI
which didn't satisfy these constraints).

(2) (Harder for now, no problems in future) Use a reference count in
the Rust bindings, which is how the other bindings work.  It makes the
Rust bindings more awkward to use, but does more accurately express
the actual intention of the API.

Discuss ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On 8/15/2023 11:07 AM, Richard W.M. Jones wrote:
> So what do I think about the patch series as a whole ...  (in
> particular, the patches I didn't add Reviewed-by tags to).
>
> It would be much nicer IMHO if we didn't have to define callback
> lifetimes in this way, since they were not intended to be classified
> into async_kind / cblifetime / cbcount, and this might limit our
> options for new ABIs in future.
>
> I see two ways to go here:
>
> (1) (Easier for now, problems in future) Rename async_kind, cblifetime
> and cbcount as rust_async_kind, rust_cblifetime, rust_cbcount, which
> would in some sense limit the scope of getting these right to the Rust
> bindings.
>
> This defers the pain til later (maybe never, if we never added an ABI
> which didn't satisfy these constraints).
>
> (2) (Harder for now, no problems in future) Use a reference count in
> the Rust bindings, which is how the other bindings work.  It makes the
> Rust bindings more awkward to use, but does more accurately express
> he actual intention of the API.
>
> Discuss ...

I want to emphasize the different purposes of async_kind, cblifetime and 
cbcount:

- async_kind: Tells what calls are asynchronous or not and how they

mark completion. It has nothing to do with reference counting and is

 ? required for the asynchronous Rust bindings to work.

- cblifetime: Tells for how long a closure might be used. Without this

 ? the user would need to use a reference counter (Arc) and a Mutex. The

 ? API would be a bit more verbose/awkward to use, but it would

 ? definitely be possible.

- cbcount: Tells for how many times the closure might be called. Without

 ? this, all closures would be FnMut, including completion callbacks.

 ? I don't think it would hurt too much to remove this.


So unless someone comes up with a better solution, I think we have to 
keep the async_kind field. Although it would make sense to rename it to 
rust_async_kind.


Personally I think the other two makes sense as well, but they can 
definitely be removed if that's what the community wants.


--

Best regards,

Tage

> Rich.
>