| Message ID | 20250211214328.640374-5-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust/block: Add minimal block driver bindings | expand |
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
Am 12.02.2025 um 10:28 hat Paolo Bonzini geschrieben: > 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. Into Waker. Do we have any idea yet how explicit we want to be with type annotations that aren't strictly necessary? I didn't think of making it explicit here because the only thing that is done with it is building a Context from it, so it seemed obvious enough. If you think that being explicit is better, then Waker::from() might be better than adding a type name to let and keeping .into(). > > + 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. Oops, thanks. I need to learn that unsafe Rust still isn't C. :-) > 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? If we could get bindgen to actually do that for the C interface, then this could be interesting, though for most functions it also would remain unused boilerplate. If we have to get the executor manually on the Rust side for each function, that's probably the same function that will want to execute the future - in which case it just can directly call the correct function. The long term idea should be anyway that C coroutines disappear from the path and we integrate an executor into the QEMU main loop. But as long as the block layer core is written in C and uses coroutines and we want Rust futures to be able to call into coroutine_fns, that's still a long way to go. Kevin
On Wed, Feb 12, 2025 at 1:47 PM Kevin Wolf <kwolf@redhat.com> wrote: > > > +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. > > Into Waker. Do we have any idea yet how explicit we want to be with type > annotations that aren't strictly necessary? I didn't think of making it > explicit here because the only thing that is done with it is building a > Context from it, so it seemed obvious enough. > > If you think that being explicit is better, then Waker::from() might > be better than adding a type name to let and keeping .into(). I think this case threw me off because From<Arc<W: Wake>> is a bit more generic than your usual From implementation. Maybe it's obvious to anyone who has used futures in Rust (I haven't). I agree, something like let waker = Waker::from(waker); let mut cx = Context::from_waker(&waker); could be the best of both worlds. > > 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? > > If we could get bindgen to actually do that for the C interface, then > this could be interesting, though for most functions it also would > remain unused boilerplate. If we have to get the executor manually on > the Rust side for each function, that's probably the same function that > will want to execute the future - in which case it just can directly > call the correct function. The idea was that you don't call the correct function but the *only* function :) i.e. exec.run_until(), and it will do the right thing for coroutine vs. no coroutine. But yeah, there would be boilerplate to pass it all the way down so maybe it is not a great idea. I liked the concept that you just *couldn't* get _co_ wrong... but perhaps it is not necessary once more of "BlockDriver::open" is lifted into bdrv_open<D: BlockDriver>. Paolo > The long term idea should be anyway that C coroutines disappear from the > path and we integrate an executor into the QEMU main loop. But as long > as the block layer core is written in C and uses coroutines and we want > Rust futures to be able to call into coroutine_fns, that's still a long > way to go.
Am 12.02.2025 um 14:22 hat Paolo Bonzini geschrieben: > > > 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? > > > > If we could get bindgen to actually do that for the C interface, then > > this could be interesting, though for most functions it also would > > remain unused boilerplate. If we have to get the executor manually on > > the Rust side for each function, that's probably the same function that > > will want to execute the future - in which case it just can directly > > call the correct function. > > The idea was that you don't call the correct function but the *only* > function :) i.e. exec.run_until(), and it will do the right thing for > coroutine vs. no coroutine. > > But yeah, there would be boilerplate to pass it all the way down so > maybe it is not a great idea. I liked the concept that you just > *couldn't* get _co_ wrong... but perhaps it is not necessary once more > of "BlockDriver::open" > is lifted into bdrv_open<D: BlockDriver>. Yes, my assumption is that in the final state there is no "all the way down" because the function wanting to run a future will be the outermost Rust function. At any other level, the Rust function can just be async itself. That's why I said that calling the only function of the correct executor isn't really any better than directly calling the correct function. Kevin
diff --git a/include/qemu/coroutine-rust.h b/include/qemu/coroutine-rust.h new file mode 100644 index 0000000000..0c5cf42a6b --- /dev/null +++ b/include/qemu/coroutine-rust.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Helpers to run Rust futures using QEMU coroutines + * + * Copyright Red Hat + * + * Author: + * Kevin Wolf <kwolf@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#ifndef QEMU_COROUTINE_RUST_H +#define QEMU_COROUTINE_RUST_H + +typedef struct RustBoxedFuture RustBoxedFuture; +typedef void coroutine_fn RunFuture(RustBoxedFuture *future, void *opaque); + +void no_coroutine_fn rust_run_future(RustBoxedFuture *future, + RunFuture *entry, + void *opaque); + +#endif diff --git a/rust/wrapper.h b/rust/wrapper.h index c3e1e6f9cf..61afac0494 100644 --- a/rust/wrapper.h +++ b/rust/wrapper.h @@ -57,3 +57,4 @@ typedef enum memory_order { #include "block/block_int.h" #include "block/qdict.h" #include "qapi/qapi-visit-block-core.h" +#include "qemu/coroutine-rust.h" diff --git a/util/qemu-co-rust-async.c b/util/qemu-co-rust-async.c new file mode 100644 index 0000000000..d893dfb7bd --- /dev/null +++ b/util/qemu-co-rust-async.c @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Helpers to run Rust futures using QEMU coroutines + * + * Copyright Red Hat + * + * Author: + * Kevin Wolf <kwolf@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include "qemu/osdep.h" + +#include "block/aio-wait.h" +#include "qemu/coroutine.h" +#include "qemu/coroutine-rust.h" +#include "qemu/main-loop.h" + +typedef struct FutureCo { + RustBoxedFuture *future; + RunFuture *entry; + void *opaque; + bool done; +} FutureCo; + +static void coroutine_fn rust_co_run_future_entry(void *opaque) +{ + FutureCo *data = opaque; + + data->entry(data->future, data->opaque); + data->done = true; + aio_wait_kick(); +} + +void no_coroutine_fn rust_run_future(RustBoxedFuture *future, + RunFuture *entry, + void *opaque) +{ + AioContext *ctx = qemu_get_current_aio_context(); + Coroutine *co; + FutureCo data = { + .future = future, + .entry = entry, + .opaque = opaque, + .done = false, + }; + + GLOBAL_STATE_CODE(); + + co = qemu_coroutine_create(rust_co_run_future_entry, &data); + aio_co_enter(ctx, co); + AIO_WAIT_WHILE(ctx, !data.done); +} diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index acac384936..713812bc2f 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -18,6 +18,7 @@ sources_core = [ 'src/callbacks.rs', 'src/cell.rs', 'src/c_str.rs', + 'src/futures.rs', 'src/module.rs', 'src/offset_of.rs', 'src/prelude.rs', diff --git a/rust/qemu-api/src/futures.rs b/rust/qemu-api/src/futures.rs new file mode 100644 index 0000000000..485041fd98 --- /dev/null +++ b/rust/qemu-api/src/futures.rs @@ -0,0 +1,78 @@ +use crate::bindings; +use std::ffi::c_void; +use std::future::Future; +use std::mem::MaybeUninit; +use std::sync::Arc; +use std::task::{Context, Poll, Wake}; + +struct RunFutureWaker { + co: *mut bindings::Coroutine, +} +unsafe impl Send for RunFutureWaker {} +unsafe impl Sync for RunFutureWaker {} + +impl Wake for RunFutureWaker { + fn wake(self: Arc<Self>) { + unsafe { + bindings::aio_co_wake(self.co); + } + } +} + +/// 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(); + 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, + 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; + } +} + +/// Use QEMU's event loops to run a Rust [`Future`] to completion and return its result. +/// +/// This function must be called outside of coroutine context to avoid deadlocks. It blocks and +/// runs a nested even loop until the future is ready and returns a result. +pub fn qemu_run_future<F: Future>(future: F) -> F::Output { + let future_ptr = Box::into_raw(Box::new(future)); + let mut output = MaybeUninit::<F::Output>::uninit(); + unsafe { + bindings::rust_run_future( + future_ptr.cast::<bindings::RustBoxedFuture>(), + #[allow(clippy::as_underscore)] + Some(rust_co_run_future::<F> as _), + output.as_mut_ptr().cast::<c_void>(), + ); + output.assume_init() + } +} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 3c6c154f3d..9b8f5fa4f1 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -18,6 +18,7 @@ pub mod c_str; pub mod callbacks; pub mod cell; +pub mod futures; #[cfg(feature = "system")] pub mod irq; pub mod module; diff --git a/util/meson.build b/util/meson.build index 780b5977a8..14a2ae17fd 100644 --- a/util/meson.build +++ b/util/meson.build @@ -101,6 +101,9 @@ if have_block util_ss.add(files('qemu-coroutine-sleep.c')) util_ss.add(files('qemu-co-shared-resource.c')) util_ss.add(files('qemu-co-timeout.c')) + if have_rust + util_ss.add(files('qemu-co-rust-async.c')) + endif util_ss.add(files('readline.c')) util_ss.add(files('throttle.c')) util_ss.add(files('timed-average.c'))
This adds helper functions that allow running Rust futures to completion using QEMU's event loops. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/qemu/coroutine-rust.h | 24 +++++++++++ rust/wrapper.h | 1 + util/qemu-co-rust-async.c | 55 ++++++++++++++++++++++++ rust/qemu-api/meson.build | 1 + rust/qemu-api/src/futures.rs | 78 +++++++++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 1 + util/meson.build | 3 ++ 7 files changed, 163 insertions(+) create mode 100644 include/qemu/coroutine-rust.h create mode 100644 util/qemu-co-rust-async.c create mode 100644 rust/qemu-api/src/futures.rs