Re: [PATCH 04/11] rust/qemu-api: Add wrappers to run futures in QEMU

[Paolo Bonzini <pbonzini@redhat.com>]

On 2/11/25 22:43, Kevin Wolf wrote:
> +/// Use QEMU's event loops to run a Rust [`Future`] to completion and return its result.
> +///
> +/// This function must be called in coroutine context. If the future isn't ready yet, it yields.
> +pub fn qemu_co_run_future<F: Future>(future: F) -> F::Output {
> +    let waker = Arc::new(RunFutureWaker {
> +        co: unsafe { bindings::qemu_coroutine_self() },
> +    })
> +    .into();

into what? :)  Maybe you can add the type to the "let" for clarity.

> +    let mut cx = Context::from_waker(&waker);
> +
> +    let mut pinned_future = std::pin::pin!(future);
> +    loop {
> +        match pinned_future.as_mut().poll(&mut cx) {
> +            Poll::Ready(res) => return res,

Alternatively, "break res" (matter of taste).

> +            Poll::Pending => unsafe {
> +                bindings::qemu_coroutine_yield();
> +            },
> +        }
> +    }
> +}
> +/// Wrapper around [`qemu_co_run_future`] that can be called from C.
> +///
> +/// # Safety
> +///
> +/// `future` must be a valid pointer to an owned `F` (it will be freed in this function).  `output`
> +/// must be a valid pointer representing a mutable reference to an `F::Output` where the result can
> +/// be stored.
> +unsafe extern "C" fn rust_co_run_future<F: Future>(
> +    future: *mut bindings::RustBoxedFuture,
> +    output: *mut c_void,
> +) {
> +    let future = unsafe { Box::from_raw(future.cast::<F>()) };
> +    let output = output.cast::<F::Output>();
> +    let ret = qemu_co_run_future(*future);
> +    unsafe {
> +        *output = ret;

This should use output.write(ret), to ensure that the output is written 
without dropping the previous value.

Also, would qemu_co_run_future() and qemu_run_future() become methods on 
an Executor later?  Maybe it make sense to have already something like

pub trait QemuExecutor {
     fn run_until<F: Future>(future: F) -> F::Output;
}

pub struct Executor;
pub struct CoExecutor;

and pass an executor to Rust functions (&Executor for no_coroutine_fn, 
&CoExecutor for coroutine_fn, &dyn QemuExecutor for mixed).  Or would 
that be premature in your opinion?

Paolo