* [PATCH preview 0/5] rust: allow minimum version of 1.83
@ 2025-05-05 10:08 Paolo Bonzini
2025-05-05 10:08 ` [PATCH 1/5] meson, cargo: require Rust 1.83.0 Paolo Bonzini
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-05 10:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust
This is the preview of moving the minimum supported Rust version
forward to 1.83.0, which is the target for QEMU due to its support
for the const_refs_to_static feature.
Being able to autogenerate all the reflection-like structs in qdev and
VMState improves the type safety, but also requires annotating the
types with the information needed to generate the structs. The
const_refs_to_static feature is needed because this information resides
in constants that refer to global variables (of types such as PropertyInfo,
VMStateField or VMStateDescription).
This series does not cover enabling the newer compiler in CI because,
while both Debian and Ubuntu have a new-enough Rust compiler to support
1.77, they pose problems for this further bump. For Debian, the bookworm
release probably will not have new compilers and is supported by QEMU
for roughly two more years. For Ubuntu, the situation is a bit weird
because while Ubuntu 22.04 had new Rust compilers added until the summer
of 2024, Ubuntu 24.04 is not adding packages for new versions.
A possible plan here is to split the configuration between "enable Rust"
and "enable all devices written in Rust" as soon as new devices are
contributed that are written in Rust. This way, the C versions of
the pl011 and HPET devices can be used but the new boards/devices would
only be available on Debian or Ubuntu by using rustup.
This series does not use *all* features enabled between 1.77 and 1.83;
in particular it does not replace addr_of!/addr_of_mut! with "&raw"
expressions.
Paolo
Paolo Bonzini (5):
meson, cargo: require Rust 1.83.0
rust: use inline const expressions
rust: vmstate: convert to use builder pattern
rust: vmstate: use const_refs_to_static
rust: qdev: const_refs_to_static
docs/devel/rust.rst | 30 +-
meson.build | 6 +-
rust/Cargo.toml | 2 +-
rust/clippy.toml | 2 +-
rust/hw/char/pl011/src/device.rs | 20 +-
rust/hw/char/pl011/src/device_class.rs | 123 +++----
rust/hw/timer/hpet/src/hpet.rs | 173 ++++------
rust/qemu-api/src/assertions.rs | 4 -
rust/qemu-api/src/callbacks.rs | 27 +-
rust/qemu-api/src/chardev.rs | 2 +-
rust/qemu-api/src/qdev.rs | 16 +-
rust/qemu-api/src/timer.rs | 2 +-
rust/qemu-api/src/vmstate.rs | 432 +++++++++++++++----------
rust/qemu-api/tests/tests.rs | 20 +-
rust/qemu-api/tests/vmstate_tests.rs | 155 +++++----
15 files changed, 517 insertions(+), 497 deletions(-)
--
2.49.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/5] meson, cargo: require Rust 1.83.0
2025-05-05 10:08 [PATCH preview 0/5] rust: allow minimum version of 1.83 Paolo Bonzini
@ 2025-05-05 10:08 ` Paolo Bonzini
2025-05-05 10:08 ` [PATCH 2/5] rust: use inline const expressions Paolo Bonzini
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-05 10:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 14 +++++---------
meson.build | 6 +++---
rust/Cargo.toml | 2 +-
rust/clippy.toml | 2 +-
4 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 4de86375021..139c298d462 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -71,21 +71,17 @@ Building Rust code with ``--enable-modules`` is not supported yet.
Supported tools
'''''''''''''''
-QEMU supports rustc version 1.77.0 and newer. Notably, the following features
-are missing:
+QEMU supports rustc version 1.83.0 and newer. The following features
+from relatively new versions of Rust are not used for historical reasons;
+patches are welcome:
* inline const expression (stable in 1.79.0), currently worked around with
associated constants in the ``FnCall`` trait.
-* associated constants have to be explicitly marked ``'static`` (`changed in
+* associated constants are still explicitly marked ``'static`` (`changed in
1.81.0`__)
-* ``&raw`` (stable in 1.82.0). Use ``addr_of!`` and ``addr_of_mut!`` instead,
- though hopefully the need for raw pointers will go down over time.
-
-* ``new_uninit`` (stable in 1.82.0). This is used internally by the ``pinned_init``
- crate, which is planned for inclusion in QEMU, but it can be easily patched
- out.
+* ``&raw`` (stable in 1.82.0).
* referencing statics in constants (stable in 1.83.0). For now use a const
function; this is an important limitation for QEMU's migration stream
diff --git a/meson.build b/meson.build
index e77da3f9b75..7afdae0ce90 100644
--- a/meson.build
+++ b/meson.build
@@ -94,12 +94,12 @@ have_rust = have_rust and add_languages('rust', native: true,
required: get_option('rust').disable_auto_if(not have_system))
if have_rust
rustc = meson.get_compiler('rust')
- if rustc.version().version_compare('<1.77.0')
+ if rustc.version().version_compare('<1.83.0')
if get_option('rust').enabled()
- error('rustc version ' + rustc.version() + ' is unsupported. Please upgrade to at least 1.77.0')
+ error('rustc version ' + rustc.version() + ' is unsupported. Please upgrade to at least 1.83.0')
else
warning('rustc version ' + rustc.version() + ' is unsupported, disabling Rust compilation.')
- message('Please upgrade to at least 1.77.0 to use Rust.')
+ message('Please upgrade to at least 1.83.0 to use Rust.')
have_rust = false
endif
endif
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 4f6fe17b50f..b0f779986ab 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -12,7 +12,7 @@ edition = "2021"
homepage = "https://www.qemu.org"
license = "GPL-2.0-or-later"
repository = "https://gitlab.com/qemu-project/qemu/"
-rust-version = "1.77.0"
+rust-version = "1.83.0"
[workspace.lints.rust]
unexpected_cfgs = { level = "deny", check-cfg = [
diff --git a/rust/clippy.toml b/rust/clippy.toml
index 933e46a2ffb..2fae76fb3f6 100644
--- a/rust/clippy.toml
+++ b/rust/clippy.toml
@@ -1,2 +1,2 @@
doc-valid-idents = ["PrimeCell", ".."]
-msrv = "1.77.0"
+msrv = "1.83.0"
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/5] rust: use inline const expressions
2025-05-05 10:08 [PATCH preview 0/5] rust: allow minimum version of 1.83 Paolo Bonzini
2025-05-05 10:08 ` [PATCH 1/5] meson, cargo: require Rust 1.83.0 Paolo Bonzini
@ 2025-05-05 10:08 ` Paolo Bonzini
2025-05-06 9:11 ` Zhao Liu
2025-05-05 10:08 ` [PATCH 3/5] rust: vmstate: convert to use builder pattern Paolo Bonzini
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-05 10:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust
They were stabilized in Rust 1.79.0.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 9 +++------
rust/qemu-api/src/callbacks.rs | 27 +--------------------------
rust/qemu-api/src/chardev.rs | 2 +-
rust/qemu-api/src/qdev.rs | 2 +-
rust/qemu-api/src/timer.rs | 2 +-
rust/qemu-api/src/vmstate.rs | 2 +-
6 files changed, 8 insertions(+), 36 deletions(-)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 139c298d462..cc52adcfdbe 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -75,9 +75,6 @@ QEMU supports rustc version 1.83.0 and newer. The following features
from relatively new versions of Rust are not used for historical reasons;
patches are welcome:
-* inline const expression (stable in 1.79.0), currently worked around with
- associated constants in the ``FnCall`` trait.
-
* associated constants are still explicitly marked ``'static`` (`changed in
1.81.0`__)
@@ -88,9 +85,9 @@ patches are welcome:
architecture (VMState). Right now, VMState lacks type safety because
it is hard to place the ``VMStateField`` definitions in traits.
-* associated const equality would be nice to have for some users of
- ``callbacks::FnCall``, but is still experimental. ``ASSERT_IS_SOME``
- replaces it.
+Associated const equality would be nice to have for some users of
+``callbacks::FnCall``, but is still experimental. Const assertions
+are used instead.
__ https://github.com/rust-lang/rust/pull/125258
diff --git a/rust/qemu-api/src/callbacks.rs b/rust/qemu-api/src/callbacks.rs
index 9642a16eb89..dbe2305f509 100644
--- a/rust/qemu-api/src/callbacks.rs
+++ b/rust/qemu-api/src/callbacks.rs
@@ -113,31 +113,6 @@
/// This is always true for zero-capture closures and function pointers, as long
/// as the code is able to name the function in the first place.
pub unsafe trait FnCall<Args, R = ()>: 'static + Sync + Sized {
- /// Referring to this internal constant asserts that the `Self` type is
- /// zero-sized. Can be replaced by an inline const expression in
- /// Rust 1.79.0+.
- const ASSERT_ZERO_SIZED: () = { assert!(mem::size_of::<Self>() == 0) };
-
- /// Referring to this constant asserts that the `Self` type is an actual
- /// function type, which can be used to catch incorrect use of `()`
- /// at compile time.
- ///
- /// # Examples
- ///
- /// ```compile_fail
- /// # use qemu_api::callbacks::FnCall;
- /// fn call_it<F: for<'a> FnCall<(&'a str,), String>>(_f: &F, s: &str) -> String {
- /// let _: () = F::ASSERT_IS_SOME;
- /// F::call((s,))
- /// }
- ///
- /// let s: String = call_it((), "hello world"); // does not compile
- /// ```
- ///
- /// Note that this can be more simply `const { assert!(F::IS_SOME) }` in
- /// Rust 1.79.0 or newer.
- const ASSERT_IS_SOME: () = { assert!(Self::IS_SOME) };
-
/// `true` if `Self` is an actual function type and not `()`.
///
/// # Examples
@@ -195,7 +170,7 @@ unsafe impl<F, $($args,)* R> FnCall<($($args,)*), R> for F
#[inline(always)]
fn call(a: ($($args,)*)) -> R {
- let _: () = Self::ASSERT_ZERO_SIZED;
+ const { assert!(mem::size_of::<Self>() == 0) };
// SAFETY: the safety of this method is the condition for implementing
// `FnCall`. As to the `NonNull` idiom to create a zero-sized type,
diff --git a/rust/qemu-api/src/chardev.rs b/rust/qemu-api/src/chardev.rs
index 6e0590d758e..cb27be52569 100644
--- a/rust/qemu-api/src/chardev.rs
+++ b/rust/qemu-api/src/chardev.rs
@@ -138,7 +138,7 @@ pub fn enable_handlers<
F::call((owner, event))
}
- let _: () = CanReceiveFn::ASSERT_IS_SOME;
+ const { assert!(CanReceiveFn::IS_SOME) };
let receive_cb: Option<unsafe extern "C" fn(*mut c_void, *const u8, c_int)> =
if ReceiveFn::is_some() {
Some(rust_receive_cb::<T, ReceiveFn>)
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 1279d7a58d5..6c93805a742 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -358,7 +358,7 @@ fn do_init_gpio_in(
}
}
- let _: () = F::ASSERT_IS_SOME;
+ const { assert!(F::IS_SOME) };
unsafe extern "C" fn rust_irq_handler<T, F: for<'a> FnCall<(&'a T, u32, u32)>>(
opaque: *mut c_void,
line: c_int,
diff --git a/rust/qemu-api/src/timer.rs b/rust/qemu-api/src/timer.rs
index 868bd88575f..66d39df37d8 100644
--- a/rust/qemu-api/src/timer.rs
+++ b/rust/qemu-api/src/timer.rs
@@ -56,7 +56,7 @@ pub fn init_full<'timer, 'opaque: 'timer, T, F>(
) where
F: for<'a> FnCall<(&'a T,)>,
{
- let _: () = F::ASSERT_IS_SOME;
+ const { assert!(F::IS_SOME) };
/// timer expiration callback
unsafe extern "C" fn rust_timer_handler<T, F: for<'a> FnCall<(&'a T,)>>(
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 9c8b2398e9d..c564bd70308 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -457,7 +457,7 @@ macro_rules! vmstate_exist_fn {
const fn test_cb_builder__<T, F: for<'a> $crate::callbacks::FnCall<(&'a T, u8), bool>>(
_phantom: ::core::marker::PhantomData<F>,
) -> $crate::vmstate::VMSFieldExistCb {
- let _: () = F::ASSERT_IS_SOME;
+ const { assert!(F::IS_SOME) };
$crate::vmstate::rust_vms_test_field_exists::<T, F>
}
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/5] rust: vmstate: convert to use builder pattern
2025-05-05 10:08 [PATCH preview 0/5] rust: allow minimum version of 1.83 Paolo Bonzini
2025-05-05 10:08 ` [PATCH 1/5] meson, cargo: require Rust 1.83.0 Paolo Bonzini
2025-05-05 10:08 ` [PATCH 2/5] rust: use inline const expressions Paolo Bonzini
@ 2025-05-05 10:08 ` Paolo Bonzini
2025-05-06 9:55 ` Zhao Liu
2025-05-05 10:08 ` [PATCH 4/5] rust: vmstate: use const_refs_to_static Paolo Bonzini
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-05 10:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, Zhao Liu
Similar to MemoryRegionOps, the builder pattern has two advantages:
1) it makes it possible to build a VMStateDescription that knows which
types it will be invoked on; 2) it provides a way to wrap the callbacks
and let devices avoid "unsafe".
Unfortunately, building a static VMStateDescription requires the
builder methods to be "const", and because the VMStateFields are
*also* static, this requires const_refs_to_static. So this requires
Rust 1.83.0.
Co-developed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 2 +-
rust/hw/char/pl011/src/device.rs | 16 +-
rust/hw/char/pl011/src/device_class.rs | 117 ++++++--------
rust/hw/timer/hpet/src/hpet.rs | 163 ++++++++-----------
rust/qemu-api/src/qdev.rs | 6 +-
rust/qemu-api/src/vmstate.rs | 214 ++++++++++++++++++++++++-
rust/qemu-api/tests/tests.rs | 16 +-
rust/qemu-api/tests/vmstate_tests.rs | 124 +++++++-------
8 files changed, 413 insertions(+), 245 deletions(-)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index cc52adcfdbe..ed1c765e722 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -152,7 +152,7 @@ module status
``qom`` stable
``sysbus`` stable
``timer`` stable
-``vmstate`` proof of concept
+``vmstate`` stable
``zeroable`` stable
================ ======================
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 7c563ade9cd..38373f54e7c 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -2,7 +2,7 @@
// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::{ffi::CStr, mem::size_of, ptr::addr_of_mut};
+use std::{ffi::CStr, io, mem::size_of, ptr::addr_of_mut};
use qemu_api::{
chardev::{CharBackend, Chardev, Event},
@@ -172,8 +172,8 @@ impl DeviceImpl for PL011State {
fn properties() -> &'static [Property] {
&device_class::PL011_PROPERTIES
}
- fn vmsd() -> Option<&'static VMStateDescription> {
- Some(&device_class::VMSTATE_PL011)
+ fn vmsd() -> Option<VMStateDescription<Self>> {
+ Some(device_class::VMSTATE_PL011)
}
const REALIZE: Option<fn(&Self)> = Some(Self::realize);
}
@@ -457,10 +457,10 @@ pub fn put_fifo(&mut self, value: registers::Data) -> bool {
false
}
- pub fn post_load(&mut self) -> Result<(), ()> {
+ pub fn post_load(&mut self) -> Result<(), io::ErrorKind> {
/* Sanity-check input state */
if self.read_pos >= self.read_fifo.len() || self.read_count > self.read_fifo.len() {
- return Err(());
+ return Err(io::ErrorKind::InvalidInput);
}
if !self.fifo_enabled() && self.read_count > 0 && self.read_pos > 0 {
@@ -524,6 +524,10 @@ const fn clock_update(&self, _event: ClockEvent) {
/* pl011_trace_baudrate_change(s); */
}
+ pub fn clock_needed(&self) -> bool {
+ self.migrate_clock
+ }
+
fn post_init(&self) {
self.init_mmio(&self.iomem);
for irq in self.interrupts.iter() {
@@ -629,7 +633,7 @@ fn update(&self) {
}
}
- pub fn post_load(&self, _version_id: u32) -> Result<(), ()> {
+ pub fn post_load(&self, _version_id: u8) -> Result<(), io::ErrorKind> {
self.regs.borrow_mut().post_load()
}
}
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index d328d846323..ed72bfad25f 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -2,86 +2,65 @@
// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::{
- ffi::{c_int, c_void},
- ptr::NonNull,
-};
-
use qemu_api::{
bindings::{qdev_prop_bool, qdev_prop_chr},
prelude::*,
- vmstate::VMStateDescription,
+ vmstate::{VMStateDescription, VMStateDescriptionBuilder},
vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_unused,
- zeroable::Zeroable,
};
use crate::device::{PL011Registers, PL011State};
-extern "C" fn pl011_clock_needed(opaque: *mut c_void) -> bool {
- let state = NonNull::new(opaque).unwrap().cast::<PL011State>();
- unsafe { state.as_ref().migrate_clock }
-}
-
/// Migration subsection for [`PL011State`] clock.
-static VMSTATE_PL011_CLOCK: VMStateDescription = VMStateDescription {
- name: c"pl011/clock".as_ptr(),
- version_id: 1,
- minimum_version_id: 1,
- needed: Some(pl011_clock_needed),
- fields: vmstate_fields! {
- vmstate_clock!(PL011State, clock),
- },
- ..Zeroable::ZERO
-};
+static VMSTATE_PL011_CLOCK: VMStateDescription<PL011State> =
+ VMStateDescriptionBuilder::<PL011State>::new()
+ .name(c"pl011/clock")
+ .version_id(1)
+ .minimum_version_id(1)
+ .needed(&PL011State::clock_needed)
+ .fields(vmstate_fields! {
+ vmstate_clock!(PL011State, clock),
+ })
+ .build();
-extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
- let state = NonNull::new(opaque).unwrap().cast::<PL011State>();
- let result = unsafe { state.as_ref().post_load(version_id as u32) };
- if result.is_err() {
- -1
- } else {
- 0
- }
-}
+static VMSTATE_PL011_REGS: VMStateDescription<PL011Registers> =
+ VMStateDescriptionBuilder::<PL011Registers>::new()
+ .name(c"pl011/regs")
+ .version_id(2)
+ .minimum_version_id(2)
+ .fields(vmstate_fields! {
+ vmstate_of!(PL011Registers, flags),
+ vmstate_of!(PL011Registers, line_control),
+ vmstate_of!(PL011Registers, receive_status_error_clear),
+ vmstate_of!(PL011Registers, control),
+ vmstate_of!(PL011Registers, dmacr),
+ vmstate_of!(PL011Registers, int_enabled),
+ vmstate_of!(PL011Registers, int_level),
+ vmstate_of!(PL011Registers, read_fifo),
+ vmstate_of!(PL011Registers, ilpr),
+ vmstate_of!(PL011Registers, ibrd),
+ vmstate_of!(PL011Registers, fbrd),
+ vmstate_of!(PL011Registers, ifl),
+ vmstate_of!(PL011Registers, read_pos),
+ vmstate_of!(PL011Registers, read_count),
+ vmstate_of!(PL011Registers, read_trigger),
+ })
+ .build();
-static VMSTATE_PL011_REGS: VMStateDescription = VMStateDescription {
- name: c"pl011/regs".as_ptr(),
- version_id: 2,
- minimum_version_id: 2,
- fields: vmstate_fields! {
- vmstate_of!(PL011Registers, flags),
- vmstate_of!(PL011Registers, line_control),
- vmstate_of!(PL011Registers, receive_status_error_clear),
- vmstate_of!(PL011Registers, control),
- vmstate_of!(PL011Registers, dmacr),
- vmstate_of!(PL011Registers, int_enabled),
- vmstate_of!(PL011Registers, int_level),
- vmstate_of!(PL011Registers, read_fifo),
- vmstate_of!(PL011Registers, ilpr),
- vmstate_of!(PL011Registers, ibrd),
- vmstate_of!(PL011Registers, fbrd),
- vmstate_of!(PL011Registers, ifl),
- vmstate_of!(PL011Registers, read_pos),
- vmstate_of!(PL011Registers, read_count),
- vmstate_of!(PL011Registers, read_trigger),
- },
- ..Zeroable::ZERO
-};
-
-pub static VMSTATE_PL011: VMStateDescription = VMStateDescription {
- name: c"pl011".as_ptr(),
- version_id: 2,
- minimum_version_id: 2,
- post_load: Some(pl011_post_load),
- fields: vmstate_fields! {
- vmstate_unused!(core::mem::size_of::<u32>()),
- vmstate_struct!(PL011State, regs, &VMSTATE_PL011_REGS, BqlRefCell<PL011Registers>),
- },
- subsections: vmstate_subsections! {
- VMSTATE_PL011_CLOCK
- },
- ..Zeroable::ZERO
-};
+pub const VMSTATE_PL011: VMStateDescription<PL011State> =
+ VMStateDescriptionBuilder::<PL011State>::new()
+ .name(c"pl011")
+ .version_id(2)
+ .minimum_version_id(2)
+ .post_load(&PL011State::post_load)
+ .fields(vmstate_fields! {
+ vmstate_unused!(core::mem::size_of::<u32>()),
+ vmstate_struct!(PL011State, regs, &VMSTATE_PL011_REGS, BqlRefCell<PL011Registers>),
+ })
+ .subsections(vmstate_subsections! {
+ VMSTATE_PL011_CLOCK
+ })
+ .build();
qemu_api::declare_properties! {
PL011_PROPERTIES,
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 779681d6509..1ed1cb7dcaf 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -3,7 +3,8 @@
// SPDX-License-Identifier: GPL-2.0-or-later
use std::{
- ffi::{c_int, c_void, CStr},
+ ffi::CStr,
+ io,
pin::Pin,
ptr::{addr_of_mut, null_mut, NonNull},
slice::from_ref,
@@ -25,9 +26,8 @@
qom_isa,
sysbus::{SysBusDevice, SysBusDeviceImpl},
timer::{Timer, CLOCK_VIRTUAL, NANOSECONDS_PER_SECOND},
- vmstate::VMStateDescription,
+ vmstate::{VMStateDescription, VMStateDescriptionBuilder},
vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_validate,
- zeroable::Zeroable,
};
use crate::fw_cfg::HPETFwConfig;
@@ -211,6 +211,10 @@ pub struct HPETTimer {
last: u64,
}
+// SAFETY: Sync is not automatically derived due to the `state` field,
+// which is always dereferenced to a shared reference.
+unsafe impl Sync for HPETTimer {}
+
impl HPETTimer {
fn init(&mut self, index: u8, state: &HPETState) {
*self = HPETTimer {
@@ -843,7 +847,7 @@ fn write(&self, addr: hwaddr, value: u64, size: u32) {
}
}
- fn pre_save(&self) -> i32 {
+ fn pre_save(&self) -> Result<(), io::ErrorKind> {
if self.is_hpet_enabled() {
self.counter.set(self.get_ticks());
}
@@ -854,10 +858,10 @@ fn pre_save(&self) -> i32 {
* that was configured.
*/
self.num_timers_save.set(self.num_timers.get());
- 0
+ Ok(())
}
- fn post_load(&self, _version_id: u8) -> i32 {
+ fn post_load(&self, _version_id: u8) -> Result<(), io::ErrorKind> {
for timer in self.timers.iter().take(self.get_num_timers()) {
let mut t = timer.borrow_mut();
@@ -871,7 +875,7 @@ fn post_load(&self, _version_id: u8) -> i32 {
.set(ticks_to_ns(self.counter.get()) - CLOCK_VIRTUAL.get_ns());
}
- 0
+ Ok(())
}
fn is_rtc_irq_level_needed(&self) -> bool {
@@ -941,105 +945,74 @@ impl ObjectImpl for HPETState {
),
}
-unsafe extern "C" fn hpet_rtc_irq_level_needed(opaque: *mut c_void) -> bool {
- // SAFETY:
- // the pointer is convertible to a reference
- let state: &HPETState = unsafe { NonNull::new(opaque.cast::<HPETState>()).unwrap().as_ref() };
- state.is_rtc_irq_level_needed()
-}
+static VMSTATE_HPET_RTC_IRQ_LEVEL: VMStateDescription<HPETState> =
+ VMStateDescriptionBuilder::<HPETState>::new()
+ .name(c"hpet/rtc_irq_level")
+ .version_id(1)
+ .minimum_version_id(1)
+ .needed(&HPETState::is_rtc_irq_level_needed)
+ .fields(vmstate_fields! {
+ vmstate_of!(HPETState, rtc_irq_level),
+ })
+ .build();
-unsafe extern "C" fn hpet_offset_needed(opaque: *mut c_void) -> bool {
- // SAFETY:
- // the pointer is convertible to a reference
- let state: &HPETState = unsafe { NonNull::new(opaque.cast::<HPETState>()).unwrap().as_ref() };
- state.is_offset_needed()
-}
+static VMSTATE_HPET_OFFSET: VMStateDescription<HPETState> =
+ VMStateDescriptionBuilder::<HPETState>::new()
+ .name(c"hpet/offset")
+ .version_id(1)
+ .minimum_version_id(1)
+ .needed(&HPETState::is_offset_needed)
+ .fields(vmstate_fields! {
+ vmstate_of!(HPETState, hpet_offset),
+ })
+ .build();
-unsafe extern "C" fn hpet_pre_save(opaque: *mut c_void) -> c_int {
- // SAFETY:
- // the pointer is convertible to a reference
- let state: &mut HPETState =
- unsafe { NonNull::new(opaque.cast::<HPETState>()).unwrap().as_mut() };
- state.pre_save() as c_int
-}
-
-unsafe extern "C" fn hpet_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
- // SAFETY:
- // the pointer is convertible to a reference
- let state: &mut HPETState =
- unsafe { NonNull::new(opaque.cast::<HPETState>()).unwrap().as_mut() };
- let version: u8 = version_id.try_into().unwrap();
- state.post_load(version) as c_int
-}
-
-static VMSTATE_HPET_RTC_IRQ_LEVEL: VMStateDescription = VMStateDescription {
- name: c"hpet/rtc_irq_level".as_ptr(),
- version_id: 1,
- minimum_version_id: 1,
- needed: Some(hpet_rtc_irq_level_needed),
- fields: vmstate_fields! {
- vmstate_of!(HPETState, rtc_irq_level),
- },
- ..Zeroable::ZERO
-};
-
-static VMSTATE_HPET_OFFSET: VMStateDescription = VMStateDescription {
- name: c"hpet/offset".as_ptr(),
- version_id: 1,
- minimum_version_id: 1,
- needed: Some(hpet_offset_needed),
- fields: vmstate_fields! {
- vmstate_of!(HPETState, hpet_offset),
- },
- ..Zeroable::ZERO
-};
-
-static VMSTATE_HPET_TIMER: VMStateDescription = VMStateDescription {
- name: c"hpet_timer".as_ptr(),
- version_id: 1,
- minimum_version_id: 1,
- fields: vmstate_fields! {
- vmstate_of!(HPETTimer, index),
- vmstate_of!(HPETTimer, config),
- vmstate_of!(HPETTimer, cmp),
- vmstate_of!(HPETTimer, fsb),
- vmstate_of!(HPETTimer, period),
- vmstate_of!(HPETTimer, wrap_flag),
- vmstate_of!(HPETTimer, qemu_timer),
- },
- ..Zeroable::ZERO
-};
+static VMSTATE_HPET_TIMER: VMStateDescription<HPETTimer> =
+ VMStateDescriptionBuilder::<HPETTimer>::new()
+ .name(c"hpet_timer")
+ .version_id(1)
+ .minimum_version_id(1)
+ .fields(vmstate_fields! {
+ vmstate_of!(HPETTimer, index),
+ vmstate_of!(HPETTimer, config),
+ vmstate_of!(HPETTimer, cmp),
+ vmstate_of!(HPETTimer, fsb),
+ vmstate_of!(HPETTimer, period),
+ vmstate_of!(HPETTimer, wrap_flag),
+ vmstate_of!(HPETTimer, qemu_timer),
+ })
+ .build();
const VALIDATE_TIMERS_NAME: &CStr = c"num_timers must match";
-static VMSTATE_HPET: VMStateDescription = VMStateDescription {
- name: c"hpet".as_ptr(),
- version_id: 2,
- minimum_version_id: 1,
- pre_save: Some(hpet_pre_save),
- post_load: Some(hpet_post_load),
- fields: vmstate_fields! {
- vmstate_of!(HPETState, config),
- vmstate_of!(HPETState, int_status),
- vmstate_of!(HPETState, counter),
- vmstate_of!(HPETState, num_timers_save).with_version_id(2),
- vmstate_validate!(HPETState, VALIDATE_TIMERS_NAME, HPETState::validate_num_timers),
- vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
- },
- subsections: vmstate_subsections! {
- VMSTATE_HPET_RTC_IRQ_LEVEL,
- VMSTATE_HPET_OFFSET,
- },
- ..Zeroable::ZERO
-};
+const VMSTATE_HPET: VMStateDescription<HPETState> =
+ VMStateDescriptionBuilder::<HPETState>::new()
+ .name(c"hpet")
+ .version_id(2)
+ .minimum_version_id(1)
+ .pre_save(&HPETState::pre_save)
+ .post_load(&HPETState::post_load)
+ .fields(vmstate_fields! {
+ vmstate_of!(HPETState, config),
+ vmstate_of!(HPETState, int_status),
+ vmstate_of!(HPETState, counter),
+ vmstate_of!(HPETState, num_timers_save).with_version_id(2),
+ vmstate_validate!(HPETState, VALIDATE_TIMERS_NAME, HPETState::validate_num_timers),
+ vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
+ })
+ .subsections(vmstate_subsections!(
+ VMSTATE_HPET_RTC_IRQ_LEVEL,
+ VMSTATE_HPET_OFFSET,
+ ))
+ .build();
impl DeviceImpl for HPETState {
fn properties() -> &'static [Property] {
&HPET_PROPERTIES
}
- fn vmsd() -> Option<&'static VMStateDescription> {
- Some(&VMSTATE_HPET)
+ fn vmsd() -> Option<VMStateDescription<Self>> {
+ Some(VMSTATE_HPET)
}
const REALIZE: Option<fn(&Self)> = Some(Self::realize);
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 6c93805a742..09555bbd0e7 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -120,7 +120,7 @@ fn properties() -> &'static [Property] {
/// A `VMStateDescription` providing the migration format for the device
/// Not a `const` because referencing statics in constants is unstable
/// until Rust 1.83.0.
- fn vmsd() -> Option<&'static VMStateDescription> {
+ fn vmsd() -> Option<VMStateDescription<Self>> {
None
}
}
@@ -169,7 +169,9 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
self.realize = Some(rust_realize_fn::<T>);
}
if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
- self.vmsd = vmsd;
+ // Give a 'static lifetime to the return value of vmsd().
+ // Temporary until vmsd() can be changed into a const.
+ self.vmsd = Box::leak(Box::new(vmsd.get()));
}
let prop = <T as DeviceImpl>::properties();
if !prop.is_empty() {
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index c564bd70308..228d748b6b7 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -25,11 +25,16 @@
//! functionality that is missing from `vmstate_of!`.
use core::{marker::PhantomData, mem, ptr::NonNull};
-use std::ffi::{c_int, c_void};
+use std::ffi::{c_int, c_void, CStr};
-pub use crate::bindings::{VMStateDescription, VMStateField};
+pub use crate::bindings::{MigrationPriority, VMStateField};
use crate::{
- bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
+ bindings::{self, VMStateFlags},
+ callbacks::FnCall,
+ errno::{into_neg_errno, Errno},
+ prelude::*,
+ qom::Owned,
+ zeroable::Zeroable,
};
/// This macro is used to call a function with a generic argument bound
@@ -440,7 +445,7 @@ pub extern "C" fn rust_vms_test_field_exists<T, F: for<'a> FnCall<(&'a T, u8), b
opaque: *mut c_void,
version_id: c_int,
) -> bool {
- // SAFETY: the opaque was passed as a reference to `T`.
+ // SAFETY: assumes vmstate_struct! is used correctly
let owner: &T = unsafe { &*(opaque.cast::<T>()) };
let version: u8 = version_id.try_into().unwrap();
F::call((owner, version))
@@ -490,7 +495,7 @@ macro_rules! vmstate_struct {
},
size: ::core::mem::size_of::<$type>(),
flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
- vmsd: $vmsd,
+ vmsd: $vmsd.as_ref(),
$(field_exists: $crate::vmstate_exist_fn!($struct_name, $test_fn),)?
..$crate::zeroable::Zeroable::ZERO
} $(.with_varray_flag_unchecked(
@@ -594,11 +599,206 @@ macro_rules! vmstate_subsections {
($($subsection:expr),*$(,)*) => {{
static _SUBSECTIONS: $crate::vmstate::VMStateSubsectionsWrapper = $crate::vmstate::VMStateSubsectionsWrapper(&[
$({
- static _SUBSECTION: $crate::bindings::VMStateDescription = $subsection;
+ static _SUBSECTION: $crate::bindings::VMStateDescription = $subsection.get();
::core::ptr::addr_of!(_SUBSECTION)
}),*,
::core::ptr::null()
]);
- _SUBSECTIONS.0.as_ptr()
+ &_SUBSECTIONS
}}
}
+
+pub struct VMStateDescription<T>(bindings::VMStateDescription, PhantomData<fn(&T)>);
+
+// SAFETY: When a *const T is passed to the callbacks, the call itself
+// is done in a thread-safe manner. The invocation is okay as long as
+// T itself is `Sync`.
+unsafe impl<T: Sync> Sync for VMStateDescription<T> {}
+
+#[derive(Clone)]
+pub struct VMStateDescriptionBuilder<T>(bindings::VMStateDescription, PhantomData<fn(&T)>);
+
+unsafe extern "C" fn vmstate_pre_load_cb<
+ T,
+ F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>,
+>(
+ opaque: *mut c_void,
+) -> c_int {
+ // SAFETY: assumes vmstate_struct! is used correctly
+ let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
+ into_neg_errno(result)
+}
+
+unsafe extern "C" fn vmstate_post_load_cb<
+ T,
+ F: for<'a> FnCall<(&'a T, u8), Result<(), impl Into<Errno>>>,
+>(
+ opaque: *mut c_void,
+ version_id: c_int,
+) -> c_int {
+ // SAFETY: assumes vmstate_struct! is used correctly
+ let owner: &T = unsafe { &*(opaque.cast::<T>()) };
+ let version: u8 = version_id.try_into().unwrap();
+ let result = F::call((owner, version));
+ into_neg_errno(result)
+}
+
+unsafe extern "C" fn vmstate_pre_save_cb<
+ T,
+ F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>,
+>(
+ opaque: *mut c_void,
+) -> c_int {
+ // SAFETY: assumes vmstate_struct! is used correctly
+ let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
+ into_neg_errno(result)
+}
+
+unsafe extern "C" fn vmstate_post_save_cb<
+ T,
+ F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>,
+>(
+ opaque: *mut c_void,
+) -> c_int {
+ // SAFETY: assumes vmstate_struct! is used correctly
+ let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
+ into_neg_errno(result)
+}
+
+unsafe extern "C" fn vmstate_needed_cb<T, F: for<'a> FnCall<(&'a T,), bool>>(
+ opaque: *mut c_void,
+) -> bool {
+ // SAFETY: assumes vmstate_struct! is used correctly
+ F::call((unsafe { &*(opaque.cast::<T>()) },))
+}
+
+unsafe extern "C" fn vmstate_dev_unplug_pending_cb<T, F: for<'a> FnCall<(&'a T,), bool>>(
+ opaque: *mut c_void,
+) -> bool {
+ // SAFETY: assumes vmstate_struct! is used correctly
+ F::call((unsafe { &*(opaque.cast::<T>()) },))
+}
+
+impl<T> VMStateDescriptionBuilder<T> {
+ #[must_use]
+ pub const fn name(mut self, name_str: &CStr) -> Self {
+ self.0.name = ::std::ffi::CStr::as_ptr(name_str);
+ self
+ }
+
+ #[must_use]
+ pub const fn unmigratable(mut self) -> Self {
+ self.0.unmigratable = true;
+ self
+ }
+
+ #[must_use]
+ pub const fn early_setup(mut self) -> Self {
+ self.0.early_setup = true;
+ self
+ }
+
+ #[must_use]
+ pub const fn version_id(mut self, version: u8) -> Self {
+ self.0.version_id = version as c_int;
+ self
+ }
+
+ #[must_use]
+ pub const fn minimum_version_id(mut self, min_version: u8) -> Self {
+ self.0.minimum_version_id = min_version as c_int;
+ self
+ }
+
+ #[must_use]
+ pub const fn priority(mut self, priority: MigrationPriority) -> Self {
+ self.0.priority = priority;
+ self
+ }
+
+ #[must_use]
+ pub const fn pre_load<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>(
+ mut self,
+ _f: &F,
+ ) -> Self {
+ self.0.pre_load = Some(vmstate_pre_load_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn post_load<F: for<'a> FnCall<(&'a T, u8), Result<(), impl Into<Errno>>>>(
+ mut self,
+ _f: &F,
+ ) -> Self {
+ self.0.post_load = Some(vmstate_post_load_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn pre_save<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>(
+ mut self,
+ _f: &F,
+ ) -> Self {
+ self.0.pre_save = Some(vmstate_pre_save_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn post_save<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>(
+ mut self,
+ _f: &F,
+ ) -> Self {
+ self.0.post_save = Some(vmstate_post_save_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn needed<F: for<'a> FnCall<(&'a T,), bool>>(mut self, _f: &F) -> Self {
+ self.0.needed = Some(vmstate_needed_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn unplug_pending<F: for<'a> FnCall<(&'a T,), bool>>(mut self, _f: &F) -> Self {
+ self.0.dev_unplug_pending = Some(vmstate_dev_unplug_pending_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn fields(mut self, fields: *const VMStateField) -> Self {
+ self.0.fields = fields;
+ self
+ }
+
+ #[must_use]
+ pub const fn subsections(mut self, subs: &'static VMStateSubsectionsWrapper) -> Self {
+ self.0.subsections = subs.0.as_ptr();
+ self
+ }
+
+ #[must_use]
+ pub const fn build(self) -> VMStateDescription<T> {
+ VMStateDescription::<T>(self.0, PhantomData)
+ }
+
+ #[must_use]
+ pub const fn new() -> Self {
+ Self(bindings::VMStateDescription::ZERO, PhantomData)
+ }
+}
+
+impl<T> Default for VMStateDescriptionBuilder<T> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+impl<T> VMStateDescription<T> {
+ pub const fn get(&self) -> bindings::VMStateDescription {
+ self.0
+ }
+
+ pub const fn as_ref(&self) -> &bindings::VMStateDescription {
+ &self.0
+ }
+}
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index a658a49fcfd..3264641d128 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -12,18 +12,16 @@
qdev::{DeviceImpl, DeviceState, Property, ResettablePhasesImpl},
qom::{ObjectImpl, ParentField},
sysbus::SysBusDevice,
- vmstate::VMStateDescription,
- zeroable::Zeroable,
+ vmstate::{VMStateDescription, VMStateDescriptionBuilder},
};
mod vmstate_tests;
// Test that macros can compile.
-pub static VMSTATE: VMStateDescription = VMStateDescription {
- name: c"name".as_ptr(),
- unmigratable: true,
- ..Zeroable::ZERO
-};
+pub const VMSTATE: VMStateDescription<DummyState> = VMStateDescriptionBuilder::<DummyState>::new()
+ .name(c"name")
+ .unmigratable()
+ .build();
#[repr(C)]
#[derive(qemu_api_macros::Object)]
@@ -72,8 +70,8 @@ impl DeviceImpl for DummyState {
fn properties() -> &'static [Property] {
&DUMMY_PROPERTIES
}
- fn vmsd() -> Option<&'static VMStateDescription> {
- Some(&VMSTATE)
+ fn vmsd() -> Option<VMStateDescription<Self>> {
+ Some(VMSTATE)
}
}
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index ad0fc5cd5dd..12f852ef703 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -16,9 +16,8 @@
},
cell::{BqlCell, Opaque},
impl_vmstate_forward,
- vmstate::{VMStateDescription, VMStateField},
+ vmstate::{VMStateDescription, VMStateDescriptionBuilder, VMStateField},
vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused, vmstate_validate,
- zeroable::Zeroable,
};
const FOO_ARRAY_MAX: usize = 3;
@@ -41,22 +40,22 @@ struct FooA {
elem: i8,
}
-static VMSTATE_FOOA: VMStateDescription = VMStateDescription {
- name: c"foo_a".as_ptr(),
- version_id: 1,
- minimum_version_id: 1,
- fields: vmstate_fields! {
+static VMSTATE_FOOA: VMStateDescription<FooA> = VMStateDescriptionBuilder::<FooA>::new()
+ .name(c"foo_a")
+ .version_id(1)
+ .minimum_version_id(1)
+ .fields(vmstate_fields! {
vmstate_of!(FooA, elem),
vmstate_unused!(size_of::<i64>()),
vmstate_of!(FooA, arr[0 .. num]).with_version_id(0),
vmstate_of!(FooA, arr_mul[0 .. num_mul * 16]),
- },
- ..Zeroable::ZERO
-};
+ })
+ .build();
#[test]
fn test_vmstate_uint16() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOA.as_ref().fields, 5) };
// 1st VMStateField ("elem") in VMSTATE_FOOA (corresponding to VMSTATE_UINT16)
assert_eq!(
@@ -76,7 +75,8 @@ fn test_vmstate_uint16() {
#[test]
fn test_vmstate_unused() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOA.as_ref().fields, 5) };
// 2nd VMStateField ("unused") in VMSTATE_FOOA (corresponding to VMSTATE_UNUSED)
assert_eq!(
@@ -96,7 +96,8 @@ fn test_vmstate_unused() {
#[test]
fn test_vmstate_varray_uint16_unsafe() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOA.as_ref().fields, 5) };
// 3rd VMStateField ("arr") in VMSTATE_FOOA (corresponding to
// VMSTATE_VARRAY_UINT16_UNSAFE)
@@ -117,7 +118,8 @@ fn test_vmstate_varray_uint16_unsafe() {
#[test]
fn test_vmstate_varray_multiply() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOA.as_ref().fields, 5) };
// 4th VMStateField ("arr_mul") in VMSTATE_FOOA (corresponding to
// VMSTATE_VARRAY_MULTIPLY)
@@ -171,24 +173,25 @@ fn validate_foob(_state: &FooB, _version_id: u8) -> bool {
true
}
-static VMSTATE_FOOB: VMStateDescription = VMStateDescription {
- name: c"foo_b".as_ptr(),
- version_id: 2,
- minimum_version_id: 1,
- fields: vmstate_fields! {
- vmstate_of!(FooB, val).with_version_id(2),
- vmstate_of!(FooB, wrap),
- vmstate_struct!(FooB, arr_a[0 .. num_a], &VMSTATE_FOOA, FooA).with_version_id(1),
- vmstate_struct!(FooB, arr_a_mul[0 .. num_a_mul * 32], &VMSTATE_FOOA, FooA).with_version_id(2),
- vmstate_of!(FooB, arr_i64),
- vmstate_struct!(FooB, arr_a_wrap[0 .. num_a_wrap], &VMSTATE_FOOA, FooA, validate_foob),
- },
- ..Zeroable::ZERO
-};
+static VMSTATE_FOOB: VMStateDescription<FooB> =
+ VMStateDescriptionBuilder::<FooB>::new()
+ .name(c"foo_b")
+ .version_id(2)
+ .minimum_version_id(1)
+ .fields(vmstate_fields! {
+ vmstate_of!(FooB, val).with_version_id(2),
+ vmstate_of!(FooB, wrap),
+ vmstate_struct!(FooB, arr_a[0 .. num_a], &VMSTATE_FOOA, FooA).with_version_id(1),
+ vmstate_struct!(FooB, arr_a_mul[0 .. num_a_mul * 32], &VMSTATE_FOOA, FooA).with_version_id(2),
+ vmstate_of!(FooB, arr_i64),
+ vmstate_struct!(FooB, arr_a_wrap[0 .. num_a_wrap], &VMSTATE_FOOA, FooA, validate_foob),
+ })
+ .build();
#[test]
fn test_vmstate_bool_v() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 7) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOB.as_ref().fields, 7) };
// 1st VMStateField ("val") in VMSTATE_FOOB (corresponding to VMSTATE_BOOL_V)
assert_eq!(
@@ -208,7 +211,8 @@ fn test_vmstate_bool_v() {
#[test]
fn test_vmstate_uint64() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 7) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOB.as_ref().fields, 7) };
// 2nd VMStateField ("wrap") in VMSTATE_FOOB (corresponding to VMSTATE_U64)
assert_eq!(
@@ -228,7 +232,8 @@ fn test_vmstate_uint64() {
#[test]
fn test_vmstate_struct_varray_uint8() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 7) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOB.as_ref().fields, 7) };
// 3rd VMStateField ("arr_a") in VMSTATE_FOOB (corresponding to
// VMSTATE_STRUCT_VARRAY_UINT8)
@@ -246,13 +251,14 @@ fn test_vmstate_struct_varray_uint8() {
foo_fields[2].flags.0,
VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_VARRAY_UINT8.0
);
- assert_eq!(foo_fields[2].vmsd, &VMSTATE_FOOA);
+ assert_eq!(foo_fields[2].vmsd, VMSTATE_FOOA.as_ref());
assert!(foo_fields[2].field_exists.is_none());
}
#[test]
fn test_vmstate_struct_varray_uint32_multiply() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 7) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOB.as_ref().fields, 7) };
// 4th VMStateField ("arr_a_mul") in VMSTATE_FOOB (corresponding to
// (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32)
@@ -272,13 +278,14 @@ fn test_vmstate_struct_varray_uint32_multiply() {
| VMStateFlags::VMS_VARRAY_UINT32.0
| VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
);
- assert_eq!(foo_fields[3].vmsd, &VMSTATE_FOOA);
+ assert_eq!(foo_fields[3].vmsd, VMSTATE_FOOA.as_ref());
assert!(foo_fields[3].field_exists.is_none());
}
#[test]
fn test_vmstate_macro_array() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 7) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOB.as_ref().fields, 7) };
// 5th VMStateField ("arr_i64") in VMSTATE_FOOB (corresponding to
// VMSTATE_ARRAY)
@@ -299,7 +306,8 @@ fn test_vmstate_macro_array() {
#[test]
fn test_vmstate_struct_varray_uint8_wrapper() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 7) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOB.as_ref().fields, 7) };
let mut foo_b: FooB = Default::default();
let foo_b_p = std::ptr::addr_of_mut!(foo_b).cast::<c_void>();
@@ -335,26 +343,28 @@ struct FooC {
arr_ptr_wrap: FooCWrapper,
}
-static VMSTATE_FOOC: VMStateDescription = VMStateDescription {
- name: c"foo_c".as_ptr(),
- version_id: 3,
- minimum_version_id: 1,
- fields: vmstate_fields! {
+unsafe impl Sync for FooC {}
+
+static VMSTATE_FOOC: VMStateDescription<FooC> = VMStateDescriptionBuilder::<FooC>::new()
+ .name(c"foo_c")
+ .version_id(3)
+ .minimum_version_id(1)
+ .fields(vmstate_fields! {
vmstate_of!(FooC, ptr).with_version_id(2),
// FIXME: Currently vmstate_struct doesn't support the pointer to structure.
// VMSTATE_STRUCT_POINTER: vmstate_struct!(FooC, ptr_a, VMSTATE_FOOA, NonNull<FooA>)
vmstate_unused!(size_of::<NonNull<FooA>>()),
vmstate_of!(FooC, arr_ptr),
vmstate_of!(FooC, arr_ptr_wrap),
- },
- ..Zeroable::ZERO
-};
+ })
+ .build();
const PTR_SIZE: usize = size_of::<*mut ()>();
#[test]
fn test_vmstate_pointer() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOC.as_ref().fields, 6) };
// 1st VMStateField ("ptr") in VMSTATE_FOOC (corresponding to VMSTATE_POINTER)
assert_eq!(
@@ -377,7 +387,8 @@ fn test_vmstate_pointer() {
#[test]
fn test_vmstate_macro_array_of_pointer() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOC.as_ref().fields, 6) };
// 3rd VMStateField ("arr_ptr") in VMSTATE_FOOC (corresponding to
// VMSTATE_ARRAY_OF_POINTER)
@@ -401,7 +412,8 @@ fn test_vmstate_macro_array_of_pointer() {
#[test]
fn test_vmstate_macro_array_of_pointer_wrapped() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOC.as_ref().fields, 6) };
// 4th VMStateField ("arr_ptr_wrap") in VMSTATE_FOOC (corresponding to
// VMSTATE_ARRAY_OF_POINTER)
@@ -450,21 +462,21 @@ fn validate_food_2(_state: &FooD, _version_id: u8) -> bool {
true
}
-static VMSTATE_FOOD: VMStateDescription = VMStateDescription {
- name: c"foo_d".as_ptr(),
- version_id: 3,
- minimum_version_id: 1,
- fields: vmstate_fields! {
+static VMSTATE_FOOD: VMStateDescription<FooD> = VMStateDescriptionBuilder::<FooD>::new()
+ .name(c"foo_d")
+ .version_id(3)
+ .minimum_version_id(1)
+ .fields(vmstate_fields! {
vmstate_validate!(FooD, c"foo_d_0", FooD::validate_food_0),
vmstate_validate!(FooD, c"foo_d_1", FooD::validate_food_1),
vmstate_validate!(FooD, c"foo_d_2", validate_food_2),
- },
- ..Zeroable::ZERO
-};
+ })
+ .build();
#[test]
fn test_vmstate_validate() {
- let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOD.fields, 4) };
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOD.as_ref().fields, 4) };
let mut foo_d = FooD;
let foo_d_p = std::ptr::addr_of_mut!(foo_d).cast::<c_void>();
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/5] rust: vmstate: use const_refs_to_static
2025-05-05 10:08 [PATCH preview 0/5] rust: allow minimum version of 1.83 Paolo Bonzini
` (2 preceding siblings ...)
2025-05-05 10:08 ` [PATCH 3/5] rust: vmstate: convert to use builder pattern Paolo Bonzini
@ 2025-05-05 10:08 ` Paolo Bonzini
2025-05-07 7:59 ` Zhao Liu
2025-05-05 10:08 ` [PATCH 5/5] rust: qdev: const_refs_to_static Paolo Bonzini
2025-05-06 8:56 ` [PATCH preview 0/5] rust: allow minimum version of 1.83 Zhao Liu
5 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-05 10:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust
The VMStateDescriptionBuilder already needs const_refs_to_static, so
use it to remove the need for vmstate_clock! and vmstate_struct!,
as well as to simplify the implementation for scalars.
If the consts in the VMState trait can reference to static
VMStateDescription, scalars do not need the info_enum_to_ref!
indirection and structs can implement the VMState trait themselves.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 5 -
rust/hw/char/pl011/src/device_class.rs | 14 +-
rust/hw/timer/hpet/src/hpet.rs | 8 +-
rust/qemu-api/src/assertions.rs | 4 -
rust/qemu-api/src/vmstate.rs | 232 +++++++------------------
rust/qemu-api/tests/vmstate_tests.rs | 65 ++++---
6 files changed, 116 insertions(+), 212 deletions(-)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index ed1c765e722..12c9bde4f5c 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -80,11 +80,6 @@ patches are welcome:
* ``&raw`` (stable in 1.82.0).
-* referencing statics in constants (stable in 1.83.0). For now use a const
- function; this is an important limitation for QEMU's migration stream
- architecture (VMState). Right now, VMState lacks type safety because
- it is hard to place the ``VMStateField`` definitions in traits.
-
Associated const equality would be nice to have for some users of
``callbacks::FnCall``, but is still experimental. Const assertions
are used instead.
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index ed72bfad25f..e4a9499bda7 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -4,9 +4,9 @@
use qemu_api::{
bindings::{qdev_prop_bool, qdev_prop_chr},
- prelude::*,
+ impl_vmstate_struct,
vmstate::{VMStateDescription, VMStateDescriptionBuilder},
- vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_unused,
+ vmstate_fields, vmstate_of, vmstate_subsections, vmstate_unused,
};
use crate::device::{PL011Registers, PL011State};
@@ -19,11 +19,12 @@
.minimum_version_id(1)
.needed(&PL011State::clock_needed)
.fields(vmstate_fields! {
- vmstate_clock!(PL011State, clock),
+ vmstate_of!(PL011State, clock),
})
.build();
-static VMSTATE_PL011_REGS: VMStateDescription<PL011Registers> =
+impl_vmstate_struct!(
+ PL011Registers,
VMStateDescriptionBuilder::<PL011Registers>::new()
.name(c"pl011/regs")
.version_id(2)
@@ -45,7 +46,8 @@
vmstate_of!(PL011Registers, read_count),
vmstate_of!(PL011Registers, read_trigger),
})
- .build();
+ .build()
+);
pub const VMSTATE_PL011: VMStateDescription<PL011State> =
VMStateDescriptionBuilder::<PL011State>::new()
@@ -55,7 +57,7 @@
.post_load(&PL011State::post_load)
.fields(vmstate_fields! {
vmstate_unused!(core::mem::size_of::<u32>()),
- vmstate_struct!(PL011State, regs, &VMSTATE_PL011_REGS, BqlRefCell<PL011Registers>),
+ vmstate_of!(PL011State, regs),
})
.subsections(vmstate_subsections! {
VMSTATE_PL011_CLOCK
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 1ed1cb7dcaf..be3b9afa316 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -16,6 +16,7 @@
qdev_prop_uint32, qdev_prop_uint8,
},
cell::{BqlCell, BqlRefCell},
+ impl_vmstate_struct,
irq::InterruptSource,
memory::{
hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder, MEMTXATTRS_UNSPECIFIED,
@@ -27,7 +28,7 @@
sysbus::{SysBusDevice, SysBusDeviceImpl},
timer::{Timer, CLOCK_VIRTUAL, NANOSECONDS_PER_SECOND},
vmstate::{VMStateDescription, VMStateDescriptionBuilder},
- vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_validate,
+ vmstate_fields, vmstate_of, vmstate_subsections, vmstate_validate,
};
use crate::fw_cfg::HPETFwConfig;
@@ -967,7 +968,7 @@ impl ObjectImpl for HPETState {
})
.build();
-static VMSTATE_HPET_TIMER: VMStateDescription<HPETTimer> =
+const VMSTATE_HPET_TIMER: VMStateDescription<HPETTimer> =
VMStateDescriptionBuilder::<HPETTimer>::new()
.name(c"hpet_timer")
.version_id(1)
@@ -982,6 +983,7 @@ impl ObjectImpl for HPETState {
vmstate_of!(HPETTimer, qemu_timer),
})
.build();
+impl_vmstate_struct!(HPETTimer, VMSTATE_HPET_TIMER);
const VALIDATE_TIMERS_NAME: &CStr = c"num_timers must match";
@@ -998,7 +1000,7 @@ impl ObjectImpl for HPETState {
vmstate_of!(HPETState, counter),
vmstate_of!(HPETState, num_timers_save).with_version_id(2),
vmstate_validate!(HPETState, VALIDATE_TIMERS_NAME, HPETState::validate_num_timers),
- vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
+ vmstate_of!(HPETState, timers[0 .. num_timers], HPETState::validate_num_timers).with_version_id(0),
})
.subsections(vmstate_subsections!(
VMSTATE_HPET_RTC_IRQ_LEVEL,
diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs
index a2d38c877df..80db0de099f 100644
--- a/rust/qemu-api/src/assertions.rs
+++ b/rust/qemu-api/src/assertions.rs
@@ -95,10 +95,6 @@ fn types_must_be_equal<T, U>(_: &T)
($t:ty, $i:tt, $ti:ty) => {
$crate::assert_field_type!(@internal v, $ti, $t, v.$i);
};
-
- ($t:ty, $i:tt, $ti:ty, num = $num:ident) => {
- $crate::assert_field_type!(@internal v, $ti, $t, v.$i[0]);
- };
}
/// Assert that an expression matches a pattern. This can also be
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 228d748b6b7..959d0a01fe3 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -11,10 +11,11 @@
//! migration format for a struct. This is based on the [`VMState`] trait,
//! which is defined by all migrateable types.
//!
-//! * [`impl_vmstate_forward`](crate::impl_vmstate_forward) and
+//! * [`impl_vmstate_forward`](crate::impl_vmstate_forward),
+//! * [`impl_vmstate_struct`](crate::impl_vmstate_struct),
//! [`impl_vmstate_bitsized`](crate::impl_vmstate_bitsized), which help with
-//! the definition of the [`VMState`] trait (respectively for transparent
-//! structs and for `bilge`-defined types)
+//! the definition of the [`VMState`] trait; they are respectively for
+//! transparent structs, nested structs and `bilge`-defined types)
//!
//! * helper macros to declare a device model state struct, in particular
//! [`vmstate_subsections`](crate::vmstate_subsections) and
@@ -24,7 +25,11 @@
//! `include/migration/vmstate.h`. These are not type-safe and only provide
//! functionality that is missing from `vmstate_of!`.
-use core::{marker::PhantomData, mem, ptr::NonNull};
+use core::{
+ marker::PhantomData,
+ mem,
+ ptr::{addr_of, NonNull},
+};
use std::ffi::{c_int, c_void, CStr};
pub use crate::bindings::{MigrationPriority, VMStateField};
@@ -33,6 +38,7 @@
callbacks::FnCall,
errno::{into_neg_errno, Errno},
prelude::*,
+ qdev,
qom::Owned,
zeroable::Zeroable,
};
@@ -74,70 +80,6 @@ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> { ::core::marker:
};
}
-/// Workaround for lack of `const_refs_static`: references to global variables
-/// can be included in a `static`, but not in a `const`; unfortunately, this
-/// is exactly what would go in the `VMStateField`'s `info` member.
-///
-/// This enum contains the contents of the `VMStateField`'s `info` member,
-/// but as an `enum` instead of a pointer.
-#[allow(non_camel_case_types)]
-pub enum VMStateFieldType {
- null,
- vmstate_info_bool,
- vmstate_info_int8,
- vmstate_info_int16,
- vmstate_info_int32,
- vmstate_info_int64,
- vmstate_info_uint8,
- vmstate_info_uint16,
- vmstate_info_uint32,
- vmstate_info_uint64,
- vmstate_info_timer,
-}
-
-/// Workaround for lack of `const_refs_static`. Converts a `VMStateFieldType`
-/// to a `*const VMStateInfo`, for inclusion in a `VMStateField`.
-#[macro_export]
-macro_rules! info_enum_to_ref {
- ($e:expr) => {
- unsafe {
- match $e {
- $crate::vmstate::VMStateFieldType::null => ::core::ptr::null(),
- $crate::vmstate::VMStateFieldType::vmstate_info_bool => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_bool)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_int8 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_int8)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_int16 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_int16)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_int32 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_int32)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_int64 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_int64)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_uint8 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint8)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_uint16 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint16)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_uint32 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint32)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_uint64 => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint64)
- }
- $crate::vmstate::VMStateFieldType::vmstate_info_timer => {
- ::core::ptr::addr_of!($crate::bindings::vmstate_info_timer)
- }
- }
- }
- };
-}
-
/// A trait for types that can be included in a device's migration stream. It
/// provides the base contents of a `VMStateField` (minus the name and offset).
///
@@ -148,12 +90,6 @@ macro_rules! info_enum_to_ref {
/// to implement it except via macros that do it for you, such as
/// `impl_vmstate_bitsized!`.
pub unsafe trait VMState {
- /// The `info` member of a `VMStateField` is a pointer and as such cannot
- /// yet be included in the [`BASE`](VMState::BASE) associated constant;
- /// this is only allowed by Rust 1.83.0 and newer. For now, include the
- /// member as an enum which is stored in a separate constant.
- const SCALAR_TYPE: VMStateFieldType = VMStateFieldType::null;
-
/// The base contents of a `VMStateField` (minus the name and offset) for
/// the type that is implementing the trait.
const BASE: VMStateField;
@@ -168,12 +104,6 @@ pub unsafe trait VMState {
};
}
-/// Internal utility function to retrieve a type's `VMStateFieldType`;
-/// used by [`vmstate_of!`](crate::vmstate_of).
-pub const fn vmstate_scalar_type<T: VMState>(_: PhantomData<T>) -> VMStateFieldType {
- T::SCALAR_TYPE
-}
-
/// Internal utility function to retrieve a type's `VMStateField`;
/// used by [`vmstate_of!`](crate::vmstate_of).
pub const fn vmstate_base<T: VMState>(_: PhantomData<T>) -> VMStateField {
@@ -201,7 +131,8 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
///
/// In order to support other types, the trait `VMState` must be implemented
/// for them. The macros
-/// [`impl_vmstate_bitsized!`](crate::impl_vmstate_bitsized)
+/// [`impl_vmstate_bitsized!`](crate::impl_vmstate_bitsized),
+/// [`impl_vmstate_struct!`](crate::impl_vmstate_struct),
/// and [`impl_vmstate_forward!`](crate::impl_vmstate_forward) help with this.
#[macro_export]
macro_rules! vmstate_of {
@@ -215,11 +146,6 @@ macro_rules! vmstate_of {
$(field_exists: $crate::vmstate_exist_fn!($struct_name, $test_fn),)?
// The calls to `call_func_with_field!` are the magic that
// computes most of the VMStateField from the type of the field.
- info: $crate::info_enum_to_ref!($crate::call_func_with_field!(
- $crate::vmstate::vmstate_scalar_type,
- $struct_name,
- $field_name
- )),
..$crate::call_func_with_field!(
$crate::vmstate::vmstate_base,
$struct_name,
@@ -320,8 +246,6 @@ macro_rules! impl_vmstate_forward {
// the first field of the tuple
($tuple:ty) => {
unsafe impl $crate::vmstate::VMState for $tuple {
- const SCALAR_TYPE: $crate::vmstate::VMStateFieldType =
- $crate::call_func_with_field!($crate::vmstate::vmstate_scalar_type, $tuple, 0);
const BASE: $crate::bindings::VMStateField =
$crate::call_func_with_field!($crate::vmstate::vmstate_base, $tuple, 0);
}
@@ -333,7 +257,6 @@ unsafe impl $crate::vmstate::VMState for $tuple {
macro_rules! impl_vmstate_transparent {
($type:ty where $base:tt: VMState $($where:tt)*) => {
unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
- const SCALAR_TYPE: VMStateFieldType = <$base as VMState>::SCALAR_TYPE;
const BASE: VMStateField = VMStateField {
size: mem::size_of::<$type>(),
..<$base as VMState>::BASE
@@ -354,10 +277,6 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
macro_rules! impl_vmstate_bitsized {
($type:ty) => {
unsafe impl $crate::vmstate::VMState for $type {
- const SCALAR_TYPE: $crate::vmstate::VMStateFieldType =
- <<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
- as ::bilge::prelude::Number>::UnderlyingType
- as $crate::vmstate::VMState>::SCALAR_TYPE;
const BASE: $crate::bindings::VMStateField =
<<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
as ::bilge::prelude::Number>::UnderlyingType
@@ -375,8 +294,8 @@ unsafe impl $crate::vmstate::VMState for $type {
macro_rules! impl_vmstate_scalar {
($info:ident, $type:ty$(, $varray_flag:ident)?) => {
unsafe impl VMState for $type {
- const SCALAR_TYPE: VMStateFieldType = VMStateFieldType::$info;
const BASE: VMStateField = VMStateField {
+ info: addr_of!(bindings::$info),
size: mem::size_of::<$type>(),
flags: VMStateFlags::VMS_SINGLE,
..Zeroable::ZERO
@@ -397,6 +316,21 @@ unsafe impl VMState for $type {
impl_vmstate_scalar!(vmstate_info_uint64, u64);
impl_vmstate_scalar!(vmstate_info_timer, crate::timer::Timer);
+macro_rules! impl_vmstate_c_struct {
+ ($type:ty, $vmsd:expr) => {
+ unsafe impl VMState for $type {
+ const BASE: VMStateField = $crate::bindings::VMStateField {
+ vmsd: addr_of!($vmsd),
+ size: mem::size_of::<$type>(),
+ flags: VMStateFlags::VMS_STRUCT,
+ ..Zeroable::ZERO
+ };
+ }
+ };
+}
+
+impl_vmstate_c_struct!(qdev::Clock, bindings::vmstate_clock);
+
// Pointer types using the underlying type's VMState plus VMS_POINTER
// Note that references are not supported, though references to cells
// could be allowed.
@@ -404,7 +338,6 @@ unsafe impl VMState for $type {
macro_rules! impl_vmstate_pointer {
($type:ty where $base:tt: VMState $($where:tt)*) => {
unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
- const SCALAR_TYPE: VMStateFieldType = <T as VMState>::SCALAR_TYPE;
const BASE: VMStateField = <$base as VMState>::BASE.with_pointer_flag();
}
};
@@ -423,7 +356,6 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
// VMS_ARRAY/VMS_ARRAY_OF_POINTER
unsafe impl<T: VMState, const N: usize> VMState for [T; N] {
- const SCALAR_TYPE: VMStateFieldType = <T as VMState>::SCALAR_TYPE;
const BASE: VMStateField = <T as VMState>::BASE.with_array_flag(N);
}
@@ -445,7 +377,7 @@ pub extern "C" fn rust_vms_test_field_exists<T, F: for<'a> FnCall<(&'a T, u8), b
opaque: *mut c_void,
version_id: c_int,
) -> bool {
- // SAFETY: assumes vmstate_struct! is used correctly
+ // SAFETY: the function is used in T's implementation of VMState
let owner: &T = unsafe { &*(opaque.cast::<T>()) };
let version: u8 = version_id.try_into().unwrap();
F::call((owner, version))
@@ -473,76 +405,6 @@ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
}};
}
-// FIXME: including the `vmsd` field in a `const` is not possible without
-// the const_refs_static feature (stabilized in Rust 1.83.0). Without it,
-// it is not possible to use VMS_STRUCT in a transparent manner using
-// `vmstate_of!`. While VMSTATE_CLOCK can at least try to be type-safe,
-// VMSTATE_STRUCT includes $type only for documentation purposes; it
-// is checked against $field_name and $struct_name, but not against $vmsd
-// which is what really would matter.
-#[doc(alias = "VMSTATE_STRUCT")]
-#[macro_export]
-macro_rules! vmstate_struct {
- ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])?, $vmsd:expr, $type:ty $(, $test_fn:expr)? $(,)?) => {
- $crate::bindings::VMStateField {
- name: ::core::concat!(::core::stringify!($field_name), "\0")
- .as_bytes()
- .as_ptr() as *const ::std::os::raw::c_char,
- $(num_offset: ::std::mem::offset_of!($struct_name, $num),)?
- offset: {
- $crate::assert_field_type!($struct_name, $field_name, $type $(, num = $num)?);
- ::std::mem::offset_of!($struct_name, $field_name)
- },
- size: ::core::mem::size_of::<$type>(),
- flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
- vmsd: $vmsd.as_ref(),
- $(field_exists: $crate::vmstate_exist_fn!($struct_name, $test_fn),)?
- ..$crate::zeroable::Zeroable::ZERO
- } $(.with_varray_flag_unchecked(
- $crate::call_func_with_field!(
- $crate::vmstate::vmstate_varray_flag,
- $struct_name,
- $num
- )
- )
- $(.with_varray_multiply($factor))?)?
- };
-}
-
-#[doc(alias = "VMSTATE_CLOCK")]
-#[macro_export]
-macro_rules! vmstate_clock {
- ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])?) => {{
- $crate::bindings::VMStateField {
- name: ::core::concat!(::core::stringify!($field_name), "\0")
- .as_bytes()
- .as_ptr() as *const ::std::os::raw::c_char,
- offset: {
- $crate::assert_field_type!(
- $struct_name,
- $field_name,
- $crate::qom::Owned<$crate::qdev::Clock> $(, num = $num)?
- );
- ::std::mem::offset_of!($struct_name, $field_name)
- },
- size: ::core::mem::size_of::<*const $crate::qdev::Clock>(),
- flags: $crate::bindings::VMStateFlags(
- $crate::bindings::VMStateFlags::VMS_STRUCT.0
- | $crate::bindings::VMStateFlags::VMS_POINTER.0,
- ),
- vmsd: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_clock) },
- ..$crate::zeroable::Zeroable::ZERO
- } $(.with_varray_flag_unchecked(
- $crate::call_func_with_field!(
- $crate::vmstate::vmstate_varray_flag,
- $struct_name,
- $num
- )
- )
- $(.with_varray_multiply($factor))?)?
- }};
-}
-
/// Helper macro to declare a list of
/// ([`VMStateField`](`crate::bindings::VMStateField`)) into a static and return
/// a pointer to the array of values it created.
@@ -577,6 +439,30 @@ macro_rules! vmstate_validate {
};
}
+/// Helper macro to allow using a struct in [`vmstate_field!`]
+///
+/// # Safety
+///
+/// The [`VMStateDescription`] constant `$vmsd` must be an accurate
+/// description of the struct.
+#[macro_export]
+macro_rules! impl_vmstate_struct {
+ ($type:ty, $vmsd:expr) => {
+ unsafe impl $crate::vmstate::VMState for $type {
+ const BASE: $crate::bindings::VMStateField = {
+ static VMSD: $crate::bindings::VMStateDescription = $vmsd.get();
+
+ $crate::bindings::VMStateField {
+ vmsd: ::core::ptr::addr_of!(VMSD),
+ size: ::core::mem::size_of::<$type>(),
+ flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
+ ..$crate::zeroable::Zeroable::ZERO
+ }
+ };
+ }
+ };
+}
+
/// A transparent wrapper type for the `subsections` field of
/// [`VMStateDescription`].
///
@@ -624,7 +510,7 @@ unsafe impl<T: Sync> Sync for VMStateDescription<T> {}
>(
opaque: *mut c_void,
) -> c_int {
- // SAFETY: assumes vmstate_struct! is used correctly
+ // SAFETY: the function is used in T's implementation of VMState
let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
into_neg_errno(result)
}
@@ -636,7 +522,7 @@ unsafe impl<T: Sync> Sync for VMStateDescription<T> {}
opaque: *mut c_void,
version_id: c_int,
) -> c_int {
- // SAFETY: assumes vmstate_struct! is used correctly
+ // SAFETY: the function is used in T's implementation of VMState
let owner: &T = unsafe { &*(opaque.cast::<T>()) };
let version: u8 = version_id.try_into().unwrap();
let result = F::call((owner, version));
@@ -649,7 +535,7 @@ unsafe impl<T: Sync> Sync for VMStateDescription<T> {}
>(
opaque: *mut c_void,
) -> c_int {
- // SAFETY: assumes vmstate_struct! is used correctly
+ // SAFETY: the function is used in T's implementation of VMState
let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
into_neg_errno(result)
}
@@ -660,7 +546,7 @@ unsafe impl<T: Sync> Sync for VMStateDescription<T> {}
>(
opaque: *mut c_void,
) -> c_int {
- // SAFETY: assumes vmstate_struct! is used correctly
+ // SAFETY: the function is used in T's implementation of VMState
let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
into_neg_errno(result)
}
@@ -668,14 +554,14 @@ unsafe impl<T: Sync> Sync for VMStateDescription<T> {}
unsafe extern "C" fn vmstate_needed_cb<T, F: for<'a> FnCall<(&'a T,), bool>>(
opaque: *mut c_void,
) -> bool {
- // SAFETY: assumes vmstate_struct! is used correctly
+ // SAFETY: the function is used in T's implementation of VMState
F::call((unsafe { &*(opaque.cast::<T>()) },))
}
unsafe extern "C" fn vmstate_dev_unplug_pending_cb<T, F: for<'a> FnCall<(&'a T,), bool>>(
opaque: *mut c_void,
) -> bool {
- // SAFETY: assumes vmstate_struct! is used correctly
+ // SAFETY: the function is used in T's implementation of VMState
F::call((unsafe { &*(opaque.cast::<T>()) },))
}
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index 12f852ef703..c73dba35fd3 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -15,9 +15,9 @@
vmstate_info_uint64, vmstate_info_uint8, vmstate_info_unused_buffer, VMStateFlags,
},
cell::{BqlCell, Opaque},
- impl_vmstate_forward,
+ impl_vmstate_forward, impl_vmstate_struct,
vmstate::{VMStateDescription, VMStateDescriptionBuilder, VMStateField},
- vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused, vmstate_validate,
+ vmstate_fields, vmstate_of, vmstate_unused, vmstate_validate,
};
const FOO_ARRAY_MAX: usize = 3;
@@ -52,6 +52,8 @@ struct FooA {
})
.build();
+impl_vmstate_struct!(FooA, &VMSTATE_FOOA);
+
#[test]
fn test_vmstate_uint16() {
let foo_fields: &[VMStateField] =
@@ -173,20 +175,19 @@ fn validate_foob(_state: &FooB, _version_id: u8) -> bool {
true
}
-static VMSTATE_FOOB: VMStateDescription<FooB> =
- VMStateDescriptionBuilder::<FooB>::new()
- .name(c"foo_b")
- .version_id(2)
- .minimum_version_id(1)
- .fields(vmstate_fields! {
- vmstate_of!(FooB, val).with_version_id(2),
- vmstate_of!(FooB, wrap),
- vmstate_struct!(FooB, arr_a[0 .. num_a], &VMSTATE_FOOA, FooA).with_version_id(1),
- vmstate_struct!(FooB, arr_a_mul[0 .. num_a_mul * 32], &VMSTATE_FOOA, FooA).with_version_id(2),
- vmstate_of!(FooB, arr_i64),
- vmstate_struct!(FooB, arr_a_wrap[0 .. num_a_wrap], &VMSTATE_FOOA, FooA, validate_foob),
- })
- .build();
+static VMSTATE_FOOB: VMStateDescription<FooB> = VMStateDescriptionBuilder::<FooB>::new()
+ .name(c"foo_b")
+ .version_id(2)
+ .minimum_version_id(1)
+ .fields(vmstate_fields! {
+ vmstate_of!(FooB, val).with_version_id(2),
+ vmstate_of!(FooB, wrap),
+ vmstate_of!(FooB, arr_a[0 .. num_a]).with_version_id(1),
+ vmstate_of!(FooB, arr_a_mul[0 .. num_a_mul * 32]).with_version_id(2),
+ vmstate_of!(FooB, arr_i64),
+ vmstate_of!(FooB, arr_a_wrap[0 .. num_a_wrap], validate_foob),
+ })
+ .build();
#[test]
fn test_vmstate_bool_v() {
@@ -351,9 +352,7 @@ unsafe impl Sync for FooC {}
.minimum_version_id(1)
.fields(vmstate_fields! {
vmstate_of!(FooC, ptr).with_version_id(2),
- // FIXME: Currently vmstate_struct doesn't support the pointer to structure.
- // VMSTATE_STRUCT_POINTER: vmstate_struct!(FooC, ptr_a, VMSTATE_FOOA, NonNull<FooA>)
- vmstate_unused!(size_of::<NonNull<FooA>>()),
+ vmstate_of!(FooC, ptr_a),
vmstate_of!(FooC, arr_ptr),
vmstate_of!(FooC, arr_ptr_wrap),
})
@@ -385,6 +384,31 @@ fn test_vmstate_pointer() {
assert!(foo_fields[0].field_exists.is_none());
}
+#[test]
+fn test_vmstate_struct_pointer() {
+ let foo_fields: &[VMStateField] =
+ unsafe { slice::from_raw_parts(VMSTATE_FOOC.as_ref().fields, 6) };
+
+ // 2st VMStateField ("ptr_a") in VMSTATE_FOOC (corresponding to
+ // VMSTATE_STRUCT_POINTER)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+ b"ptr\0"
+ );
+ assert_eq!(foo_fields[1].offset, PTR_SIZE);
+ assert_eq!(foo_fields[1].num_offset, 0);
+ assert_eq!(foo_fields[1].vmsd, VMSTATE_FOOA.as_ref());
+ assert_eq!(foo_fields[1].version_id, 0);
+ assert_eq!(foo_fields[1].size, PTR_SIZE);
+ assert_eq!(foo_fields[1].num, 0);
+ assert_eq!(
+ foo_fields[1].flags.0,
+ VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0
+ );
+ assert!(foo_fields[1].info.is_null());
+ assert!(foo_fields[1].field_exists.is_none());
+}
+
#[test]
fn test_vmstate_macro_array_of_pointer() {
let foo_fields: &[VMStateField] =
@@ -444,8 +468,7 @@ fn test_vmstate_macro_array_of_pointer_wrapped() {
// * VMSTATE_FOOD:
// - VMSTATE_VALIDATE
-// Add more member fields when vmstate_of/vmstate_struct support "test"
-// parameter.
+// Add more member fields when vmstate_of support "test" parameter.
struct FooD;
impl FooD {
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/5] rust: qdev: const_refs_to_static
2025-05-05 10:08 [PATCH preview 0/5] rust: allow minimum version of 1.83 Paolo Bonzini
` (3 preceding siblings ...)
2025-05-05 10:08 ` [PATCH 4/5] rust: vmstate: use const_refs_to_static Paolo Bonzini
@ 2025-05-05 10:08 ` Paolo Bonzini
2025-05-06 8:56 ` [PATCH preview 0/5] rust: allow minimum version of 1.83 Zhao Liu
5 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-05 10:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust
Now that const_refs_to_static can be assumed, convert the members of
the DeviceImpl trait from functions to constants. This lets the
compiler know that they have a 'static lifetime, and removes the
need for the weird "Box::leak()".
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 8 ++------
rust/hw/timer/hpet/src/hpet.rs | 10 ++--------
rust/qemu-api/src/qdev.rs | 16 +++++-----------
rust/qemu-api/tests/tests.rs | 8 ++------
4 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 38373f54e7c..34aa2bbabec 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -169,12 +169,8 @@ impl ObjectImpl for PL011State {
}
impl DeviceImpl for PL011State {
- fn properties() -> &'static [Property] {
- &device_class::PL011_PROPERTIES
- }
- fn vmsd() -> Option<VMStateDescription<Self>> {
- Some(device_class::VMSTATE_PL011)
- }
+ const PROPERTIES: &'static [Property] = &device_class::PL011_PROPERTIES;
+ const VMSTATE: Option<VMStateDescription<Self>> = Some(device_class::VMSTATE_PL011);
const REALIZE: Option<fn(&Self)> = Some(Self::realize);
}
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index be3b9afa316..48970a6f9c5 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -1009,14 +1009,8 @@ impl ObjectImpl for HPETState {
.build();
impl DeviceImpl for HPETState {
- fn properties() -> &'static [Property] {
- &HPET_PROPERTIES
- }
-
- fn vmsd() -> Option<VMStateDescription<Self>> {
- Some(VMSTATE_HPET)
- }
-
+ const PROPERTIES: &'static [Property] = &HPET_PROPERTIES;
+ const VMSTATE: Option<VMStateDescription<Self>> = Some(VMSTATE_HPET);
const REALIZE: Option<fn(&Self)> = Some(Self::realize);
}
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 09555bbd0e7..9b2bfabdad2 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -113,16 +113,12 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
/// An array providing the properties that the user can set on the
/// device. Not a `const` because referencing statics in constants
/// is unstable until Rust 1.83.0.
- fn properties() -> &'static [Property] {
- &[]
- }
+ const PROPERTIES: &'static [Property] = &[];
/// A `VMStateDescription` providing the migration format for the device
/// Not a `const` because referencing statics in constants is unstable
/// until Rust 1.83.0.
- fn vmsd() -> Option<VMStateDescription<Self>> {
- None
- }
+ const VMSTATE: Option<VMStateDescription<Self>> = None;
}
/// # Safety
@@ -168,12 +164,10 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
if <T as DeviceImpl>::REALIZE.is_some() {
self.realize = Some(rust_realize_fn::<T>);
}
- if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
- // Give a 'static lifetime to the return value of vmsd().
- // Temporary until vmsd() can be changed into a const.
- self.vmsd = Box::leak(Box::new(vmsd.get()));
+ if let Some(ref vmsd) = <T as DeviceImpl>::VMSTATE {
+ self.vmsd = vmsd.as_ref();
}
- let prop = <T as DeviceImpl>::properties();
+ let prop = <T as DeviceImpl>::PROPERTIES;
if !prop.is_empty() {
unsafe {
bindings::device_class_set_props_n(self, prop.as_ptr(), prop.len());
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 3264641d128..db0fd3de99b 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -67,12 +67,8 @@ impl ObjectImpl for DummyState {
impl ResettablePhasesImpl for DummyState {}
impl DeviceImpl for DummyState {
- fn properties() -> &'static [Property] {
- &DUMMY_PROPERTIES
- }
- fn vmsd() -> Option<VMStateDescription<Self>> {
- Some(VMSTATE)
- }
+ const PROPERTIES: &'static [Property] = &DUMMY_PROPERTIES;
+ const VMSTATE: Option<VMStateDescription<Self>> = Some(VMSTATE);
}
#[repr(C)]
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 8:56 ` [PATCH preview 0/5] rust: allow minimum version of 1.83 Zhao Liu
@ 2025-05-06 8:43 ` Paolo Bonzini
2025-05-06 9:26 ` Zhao Liu
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-06 8:43 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust
On Tue, May 6, 2025 at 10:35 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> > This series does not cover enabling the newer compiler in CI because,
> > while both Debian and Ubuntu have a new-enough Rust compiler to support
> > 1.77, they pose problems for this further bump. For Debian, the bookworm
> > release probably will not have new compilers and is supported by QEMU
> > for roughly two more years. For Ubuntu, the situation is a bit weird
> > because while Ubuntu 22.04 had new Rust compilers added until the summer
> > of 2024, Ubuntu 24.04 is not adding packages for new versions.
> >
> > A possible plan here is to split the configuration between "enable Rust"
> > and "enable all devices written in Rust" as soon as new devices are
> > contributed that are written in Rust. This way, the C versions of
> > the pl011 and HPET devices can be used but the new boards/devices would
> > only be available on Debian or Ubuntu by using rustup.
>
> "enable Rust" supports v1.77 and "enable all devices written in Rust"
> supports v1.83, correct?
Both support v1.83 only. However, if Rust is missing or old, "enable
all devices written in Rust" will fail compilation (e.g. Kconfig would
fail for ARM/x86 targets due to unsatisfiable CONFIG_PL011); "enable
Rust" will simply pick the C version of the PL011 and HPET devices.
> The current vmstate builder is excellent, but I'm concerned it might not
> land soon. Can we find a compromise?
Do you think the above would be a good compromise?
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-05 10:08 [PATCH preview 0/5] rust: allow minimum version of 1.83 Paolo Bonzini
` (4 preceding siblings ...)
2025-05-05 10:08 ` [PATCH 5/5] rust: qdev: const_refs_to_static Paolo Bonzini
@ 2025-05-06 8:56 ` Zhao Liu
2025-05-06 8:43 ` Paolo Bonzini
5 siblings, 1 reply; 20+ messages in thread
From: Zhao Liu @ 2025-05-06 8:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust
> This series does not cover enabling the newer compiler in CI because,
> while both Debian and Ubuntu have a new-enough Rust compiler to support
> 1.77, they pose problems for this further bump. For Debian, the bookworm
> release probably will not have new compilers and is supported by QEMU
> for roughly two more years. For Ubuntu, the situation is a bit weird
> because while Ubuntu 22.04 had new Rust compilers added until the summer
> of 2024, Ubuntu 24.04 is not adding packages for new versions.
>
> A possible plan here is to split the configuration between "enable Rust"
> and "enable all devices written in Rust" as soon as new devices are
> contributed that are written in Rust. This way, the C versions of
> the pl011 and HPET devices can be used but the new boards/devices would
> only be available on Debian or Ubuntu by using rustup.
"enable Rust" supports v1.77 and "enable all devices written in Rust"
supports v1.83, correct?
If so, do we need two versions of vmstate? one is for v1.77 (that HPET &
pl011 can use), and another one is for v1.83 (newer devices based on
v1.83 can use).
The current vmstate builder is excellent, but I'm concerned it might not
land soon. Can we find a compromise?
Thanks,
Zhao
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] rust: use inline const expressions
2025-05-05 10:08 ` [PATCH 2/5] rust: use inline const expressions Paolo Bonzini
@ 2025-05-06 9:11 ` Zhao Liu
2025-05-06 10:56 ` Paolo Bonzini
0 siblings, 1 reply; 20+ messages in thread
From: Zhao Liu @ 2025-05-06 9:11 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust
On Mon, May 05, 2025 at 12:08:51PM +0200, Paolo Bonzini wrote:
> Date: Mon, 5 May 2025 12:08:51 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 2/5] rust: use inline const expressions
> X-Mailer: git-send-email 2.49.0
>
> They were stabilized in Rust 1.79.0.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> docs/devel/rust.rst | 9 +++------
> rust/qemu-api/src/callbacks.rs | 27 +--------------------------
> rust/qemu-api/src/chardev.rs | 2 +-
> rust/qemu-api/src/qdev.rs | 2 +-
> rust/qemu-api/src/timer.rs | 2 +-
> rust/qemu-api/src/vmstate.rs | 2 +-
> 6 files changed, 8 insertions(+), 36 deletions(-)
...
> diff --git a/rust/qemu-api/src/chardev.rs b/rust/qemu-api/src/chardev.rs
> index 6e0590d758e..cb27be52569 100644
> --- a/rust/qemu-api/src/chardev.rs
> +++ b/rust/qemu-api/src/chardev.rs
> @@ -138,7 +138,7 @@ pub fn enable_handlers<
> F::call((owner, event))
> }
>
> - let _: () = CanReceiveFn::ASSERT_IS_SOME;
> + const { assert!(CanReceiveFn::IS_SOME) };
Do you think it's a good idea to warp this as a helper for easy
callback calls?
> let receive_cb: Option<unsafe extern "C" fn(*mut c_void, *const u8, c_int)> =
> if ReceiveFn::is_some() {
> Some(rust_receive_cb::<T, ReceiveFn>)
Thanks,
Zhao
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 8:43 ` Paolo Bonzini
@ 2025-05-06 9:26 ` Zhao Liu
2025-05-06 9:44 ` Daniel P. Berrangé
2025-05-06 9:48 ` Daniel P. Berrangé
0 siblings, 2 replies; 20+ messages in thread
From: Zhao Liu @ 2025-05-06 9:26 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust
> > "enable Rust" supports v1.77 and "enable all devices written in Rust"
> > supports v1.83, correct?
>
> Both support v1.83 only. However, if Rust is missing or old, "enable
> all devices written in Rust" will fail compilation (e.g. Kconfig would
> fail for ARM/x86 targets due to unsatisfiable CONFIG_PL011);
In this case, a brand new Rust device (without a corresponding C
version) would be unable to compile on the above platforms which don't
support v1.83. I'm not sure if this is an acceptable limitation or
policy. (Has there been a similar case in history?)
> "enable Rust" will simply pick the C version of the PL011 and HPET devices.
I support this, at least the compatibility with the old QEMU won't be
broken! Then all C devices rewritten in Rust can be covered by this
category.
> > The current vmstate builder is excellent, but I'm concerned it might not
> > land soon. Can we find a compromise?
>
> Do you think the above would be a good compromise?
Overall, I think it's OK (it's not even a compromise).
Thanks,
Zhao
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 9:26 ` Zhao Liu
@ 2025-05-06 9:44 ` Daniel P. Berrangé
2025-05-06 9:48 ` Daniel P. Berrangé
1 sibling, 0 replies; 20+ messages in thread
From: Daniel P. Berrangé @ 2025-05-06 9:44 UTC (permalink / raw)
To: Zhao Liu; +Cc: Paolo Bonzini, qemu-devel, qemu-rust
On Tue, May 06, 2025 at 05:26:12PM +0800, Zhao Liu wrote:
> > > "enable Rust" supports v1.77 and "enable all devices written in Rust"
> > > supports v1.83, correct?
> >
> > Both support v1.83 only. However, if Rust is missing or old, "enable
> > all devices written in Rust" will fail compilation (e.g. Kconfig would
> > fail for ARM/x86 targets due to unsatisfiable CONFIG_PL011);
>
> In this case, a brand new Rust device (without a corresponding C
> version) would be unable to compile on the above platforms which don't
> support v1.83. I'm not sure if this is an acceptable limitation or
> policy. (Has there been a similar case in history?)
Brand new features are not required to support all existing QEMU build
targets, they can set whatever baseline is appropriate given the external
dependencies they have.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 9:26 ` Zhao Liu
2025-05-06 9:44 ` Daniel P. Berrangé
@ 2025-05-06 9:48 ` Daniel P. Berrangé
2025-05-06 10:54 ` Paolo Bonzini
1 sibling, 1 reply; 20+ messages in thread
From: Daniel P. Berrangé @ 2025-05-06 9:48 UTC (permalink / raw)
To: Zhao Liu; +Cc: Paolo Bonzini, qemu-devel, qemu-rust
On Tue, May 06, 2025 at 05:26:12PM +0800, Zhao Liu wrote:
> > > "enable Rust" supports v1.77 and "enable all devices written in Rust"
> > > supports v1.83, correct?
> >
> > Both support v1.83 only. However, if Rust is missing or old, "enable
> > all devices written in Rust" will fail compilation (e.g. Kconfig would
> > fail for ARM/x86 targets due to unsatisfiable CONFIG_PL011);
>
> In this case, a brand new Rust device (without a corresponding C
> version) would be unable to compile on the above platforms which don't
> support v1.83. I'm not sure if this is an acceptable limitation or
> policy. (Has there been a similar case in history?)
>
> > "enable Rust" will simply pick the C version of the PL011 and HPET devices.
>
> I support this, at least the compatibility with the old QEMU won't be
> broken! Then all C devices rewritten in Rust can be covered by this
> category.
I don't really like this because it perpetuates a state where we have
parallel implementations of devices that have to be kept in sync.
If we're re-writing C devices in Rust, we need to be able to promptly
drop the C impl once the Rust impl is feature complete. Keeping 2 impls
is a general maint burden, as well as an ongoing vmstate compatibility
danger if a change in one impl is not matched by an identical change
in the other impl.
IMHO having Rust declared supported in QEMU should be aligned with being
able to drop C impls of any ported devices.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] rust: vmstate: convert to use builder pattern
2025-05-05 10:08 ` [PATCH 3/5] rust: vmstate: convert to use builder pattern Paolo Bonzini
@ 2025-05-06 9:55 ` Zhao Liu
2025-05-06 10:33 ` Paolo Bonzini
0 siblings, 1 reply; 20+ messages in thread
From: Zhao Liu @ 2025-05-06 9:55 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust
> +unsafe extern "C" fn vmstate_pre_load_cb<
> + T,
> + F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>,
> +>(
> + opaque: *mut c_void,
> +) -> c_int {
> + // SAFETY: assumes vmstate_struct! is used correctly
> + let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
> + into_neg_errno(result)
Thanks! Now I see why you used io::ErrorKind.
> +}
...
> + #[must_use]
> + pub const fn pre_load<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>(
> + mut self,
> + _f: &F,
> + ) -> Self {
Do we need to assert F is not ()?
const { assert!(F::IS_SOME) };
And I think all the other callbacks need to be checked as well.
> + self.0.pre_load = Some(vmstate_pre_load_cb::<T, F>);
> + self
> + }
Thank you :-) ! And though it's a preview,
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] rust: vmstate: convert to use builder pattern
2025-05-06 9:55 ` Zhao Liu
@ 2025-05-06 10:33 ` Paolo Bonzini
0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-06 10:33 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust
On Tue, May 6, 2025 at 11:35 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> > +unsafe extern "C" fn vmstate_pre_load_cb<
> > + T,
> > + F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>,
> > +>(
> > + opaque: *mut c_void,
> > +) -> c_int {
> > + // SAFETY: assumes vmstate_struct! is used correctly
> > + let result = F::call((unsafe { &*(opaque.cast::<T>()) },));
> > + into_neg_errno(result)
>
> Thanks! Now I see why you used io::ErrorKind.
>
> > +}
>
> ...
>
> > + #[must_use]
> > + pub const fn pre_load<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>(
> > + mut self,
> > + _f: &F,
> > + ) -> Self {
>
> Do we need to assert F is not ()?
>
> const { assert!(F::IS_SOME) };
A NULL callback (i.e. ".pre_load(())") is a bit weird but acceptable.
So the assertion if not needed, but indeed there should be an "if"
that stores "ptr::null()" instead of vmstate_pre_load_cb::<F>.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 9:48 ` Daniel P. Berrangé
@ 2025-05-06 10:54 ` Paolo Bonzini
2025-05-06 11:52 ` Daniel P. Berrangé
2025-05-06 15:56 ` Zhao Liu
0 siblings, 2 replies; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-06 10:54 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Zhao Liu, qemu-devel, qemu-rust
On Tue, May 6, 2025 at 11:49 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > [...] If Rust is missing or old, "enable
> > > all devices written in Rust" will fail compilation (e.g. Kconfig would
> > > fail for ARM/x86 targets due to unsatisfiable CONFIG_PL011);
> > > "enable Rust" will simply pick the C version of the PL011 and HPET devices.
>
> I don't really like this because it perpetuates a state where we have
> parallel implementations of devices that have to be kept in sync.
Me neither (see for example the Meson transition which avoided
parallel implementations at all costs).
On the other hand, this series shows that it's hard to have a baseline
version earlier than 1.83. The bindings got pretty far while
supporting older versions, and the few hacks needed were nice testbeds
for the build system and the procedural macro infrastructure, but the
improvements that const_refs_to_static provides for reflection are
just too big. And for Debian that means waiting until July 2027 before
making Rust mandatory, and for Ubuntu that's April 2028 based on the
current situation. I hope that the effort proves itself either valid
or unviable in less than 2-3 years. :)
Now, it's certainly not the only possibility:
1) If someone contributes devices that are written in Rust then we
could just drop the PL011 and/or HPET sample device. That's a pity but
they would survive in git history and could be resurrected later.
2) Using RUSTC_BOOTSTRAP[1] allows enabling unstable features even in
versions older than 1.83. Disadvantage: build system changes that will
be obsolete soon(ish), plus the relevant compiler code obviously
wasn't as tested as after stabilization. I'd prefer to avoid that, but
hey---Linux does it.
3) Affected distros could use RUSTC_BOOTSTRAP themselves if they want,
while upstream QEMU would only support rustup toolchains for Debian
bookworm and Ubuntu up to 24.10. This only requires
tests/lcitool/refresh changes, the disadvantage is that the project
would renege on the general promise that we make on platform support.
[1] https://rustc-dev-guide.rust-lang.org/building/bootstrapping/what-bootstrapping-does.html#complications-of-bootstrapping
> If we're re-writing C devices in Rust, we need to be able to promptly
> drop the C impl once the Rust impl is feature complete. Keeping 2 impls
> is a general maint burden, as well as an ongoing vmstate compatibility
> danger if a change in one impl is not matched by an identical change
> in the other impl.
I agree. One more reason why "Let's Rewrite It In Rust" is more of a
necessary evil to bootstrap the creation of bindings, and not a good
idea in general.
> IMHO having Rust declared supported in QEMU should be aligned with being
> able to drop C impls of any ported devices.
I agree in principle, though theory and practice may diverge.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] rust: use inline const expressions
2025-05-06 9:11 ` Zhao Liu
@ 2025-05-06 10:56 ` Paolo Bonzini
0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-06 10:56 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust
On Tue, May 6, 2025 at 10:51 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> > - let _: () = CanReceiveFn::ASSERT_IS_SOME;
> > + const { assert!(CanReceiveFn::IS_SOME) };
>
> Do you think it's a good idea to warp this as a helper for easy
> callback calls?
I think the wrapper would be very similar to what exists before this
patch, wouldn't it? The const { assert!(...) } seems clear enough and
not too verbose.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 10:54 ` Paolo Bonzini
@ 2025-05-06 11:52 ` Daniel P. Berrangé
2025-05-06 13:41 ` Paolo Bonzini
2025-05-06 15:56 ` Zhao Liu
1 sibling, 1 reply; 20+ messages in thread
From: Daniel P. Berrangé @ 2025-05-06 11:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Zhao Liu, qemu-devel, qemu-rust
On Tue, May 06, 2025 at 12:54:38PM +0200, Paolo Bonzini wrote:
> On Tue, May 6, 2025 at 11:49 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > [...] If Rust is missing or old, "enable
> > > > all devices written in Rust" will fail compilation (e.g. Kconfig would
> > > > fail for ARM/x86 targets due to unsatisfiable CONFIG_PL011);
> > > > "enable Rust" will simply pick the C version of the PL011 and HPET devices.
> >
> > I don't really like this because it perpetuates a state where we have
> > parallel implementations of devices that have to be kept in sync.
>
> Me neither (see for example the Meson transition which avoided
> parallel implementations at all costs).
>
> On the other hand, this series shows that it's hard to have a baseline
> version earlier than 1.83. The bindings got pretty far while
> supporting older versions, and the few hacks needed were nice testbeds
> for the build system and the procedural macro infrastructure, but the
> improvements that const_refs_to_static provides for reflection are
> just too big.
Admittedly I'm not actively working on the QEMU Rust code, but to
me to feels the opposite - we've shown it is possible to write useful
Rust code with the older version baseline. It may not be the ideal
way we want the code to look, but that's a tradeoff we can make.
I very much worry that at any point in time there is *always* going
to be something in a newer Rust that is very attractive to use, so
we end up on a slippery slope where we're always going to be chasing
the latest version to get a better way.
We've had the same situation with Meson where we initially set a
temporary newer baseline to get some critical features we could
not do without, and now have ended up in a situation where we
are continually pushing newer & newer versions, because there is
always something attractive in the new release.
> Now, it's certainly not the only possibility:
>
> 1) If someone contributes devices that are written in Rust then we
> could just drop the PL011 and/or HPET sample device. That's a pity but
> they would survive in git history and could be resurrected later.
IMHO for Rust in QEMU we should be targetting both new features
and existing feature ports - excluding existing feature ports
would be tieing one of our hands behind our back limiting the
potential benefits we can see.
> 2) Using RUSTC_BOOTSTRAP[1] allows enabling unstable features even in
> versions older than 1.83. Disadvantage: build system changes that will
> be obsolete soon(ish), plus the relevant compiler code obviously
> wasn't as tested as after stabilization. I'd prefer to avoid that, but
> hey---Linux does it.
>
> 3) Affected distros could use RUSTC_BOOTSTRAP themselves if they want,
> while upstream QEMU would only support rustup toolchains for Debian
> bookworm and Ubuntu up to 24.10. This only requires
> tests/lcitool/refresh changes, the disadvantage is that the project
> would renege on the general promise that we make on platform support.
Yes, this increasing defeats the benefit of defining our distro
target. We wanted to set a clear baseline that we could unambiguously
target, to give clarity to both users & contributors on when we could/
would impose new version requirements.
We've made exceptions for python, and then meson, and now Rust. We can
rationalize it is as "users only need to do x, y & z to get newer stuff",
but as we make more & more exceptions, this is a game of death by a 1000
cuts.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 11:52 ` Daniel P. Berrangé
@ 2025-05-06 13:41 ` Paolo Bonzini
0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2025-05-06 13:41 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Zhao Liu, qemu-devel, qemu-rust
[-- Attachment #1: Type: text/plain, Size: 5350 bytes --]
Il mar 6 mag 2025, 13:52 Daniel P. Berrangé <berrange@redhat.com> ha
scritto:
> > On the other hand, this series shows that it's hard to have a baseline
> > version earlier than 1.83. The bindings got pretty far while
> > supporting older versions, and the few hacks needed were nice testbeds
> > for the build system and the procedural macro infrastructure, but the
> > improvements that const_refs_to_static provides for reflection are
> > just too big.
>
> Admittedly I'm not actively working on the QEMU Rust code, but to
> me to feels the opposite - we've shown it is possible to write useful
> Rust code with the older version baseline. It may not be the ideal
> way we want the code to look, but that's a tradeoff we can make.
>
Yes it did go pretty far. But VMState is where you hit the barrier for a
very specific reason: the thing that you want to produce (structs with
pointers to other structs) isn't supported by compile time execution until
that version, so you're stuck writing C-style code for migration. These
structs have been very central to how QEMU does reflection, for more than
15 years, and there's no real workaround for that.
It's certainly possible to delay fully safe VMState some more, but perhaps
not 2-3 years.
We've had the same situation with Meson where we initially set a
> temporary newer baseline to get some critical features we could
> not do without, and now have ended up in a situation where we
> are continually pushing newer & newer versions, because there is
> always something attractive in the new release.
>
Most of the updates to Meson were done in the first year after switching,
because of new features in Meson that were contributed specifically for use
in QEMU. For example, "meson test" was improved to include all the features
of the preexisting "make check" (and many more), which benefited all users
of Meson. Since then we have only had one bleeding edge update in 2023,
when Apple messed up their linker's --version and we had to update in order
to help our macOS users (issue 1939).
Rust support does require more frequent updates to Meson because QEMU is
actually quite an early adopter among large C projects. Apart from
juggernauts like Firefox or Linux, I can only think of three: librsvg,
GStreamer and Mesa. Even Chromium (which builds with GN) only uses it for
some vendored third party libraries, with C++ bindings to Rust code. But I
can make these updates optional for people that don't --enable-rust; at the
same time I have no interest in building a pile of workarounds for those
that do.
Anyhow while I am contributing Rust-related improvements to Meson, I have
no plans, time or ability to contribute to Rust. :)
> Now, it's certainly not the only possibility:
> >
> > 1) If someone contributes devices that are written in Rust then we
> > could just drop the PL011 and/or HPET sample device. That's a pity but
> > they would survive in git history and could be resurrected later.
>
> IMHO for Rust in QEMU we should be targetting both new features
> and existing feature ports - excluding existing feature ports
> would be tieing one of our hands behind our back limiting the
> potential benefits we can see.
>
I agree.
Yes, this increasing defeats the benefit of defining our distro
> target. We wanted to set a clear baseline that we could unambiguously
> target, to give clarity to both users & contributors on when we could/
> would impose new version requirements.
>
> We've made exceptions for python, and then meson, and now Rust.
We've never made exceptions for Python in the sense of requiring a custom
Python interpreter. The interpreters we require have always been available
in all supported distros. What we did is not using the distro's *sphinx* on
enterprise distros whose default Python interpreter was way past EOL. (IMHO
shipping in /usr/bin anything that can run user Python code is a mistake,
because it conflicts with the idea that /usr/libexec/platform-python is
invisible to the user).
That came a few years *after* Meson and it happened because it's harder and
harder to support both very new and very old versions of Python, especially
after EOL. Our desires in that respect conflict very much with how the
Python community operates, unfortunately.
We can rationalize it is as "users only need to do x, y & z to get newer
> stuff",
> but as we make more & more exceptions, this is a game of death by a 1000
> cuts.
>
I agree. But the Python change is mostly water under the bridge, and the
bleeding edge Meson requirements could be made optional for those that
don't use Rust, so I don't think it's too bad.
For Rust, this submission shows exactly what the benefit would be and the
current status of the code shows how far you get without it. I don't want
to sneak in anything, I have posted it for transparency and it's up to the
community to discuss how to proceed. We can do so at the community call or
wait until KVM Forum.
Thanks,
Paolo
> With regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o-
> https://www.instagram.com/dberrange :|
>
>
[-- Attachment #2: Type: text/html, Size: 8621 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH preview 0/5] rust: allow minimum version of 1.83
2025-05-06 10:54 ` Paolo Bonzini
2025-05-06 11:52 ` Daniel P. Berrangé
@ 2025-05-06 15:56 ` Zhao Liu
1 sibling, 0 replies; 20+ messages in thread
From: Zhao Liu @ 2025-05-06 15:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Daniel P. Berrangé, qemu-devel, qemu-rust
> > If we're re-writing C devices in Rust, we need to be able to promptly
> > drop the C impl once the Rust impl is feature complete. Keeping 2 impls
> > is a general maint burden, as well as an ongoing vmstate compatibility
> > danger if a change in one impl is not matched by an identical change
> > in the other impl.
>
> I agree. One more reason why "Let's Rewrite It In Rust" is more of a
> necessary evil to bootstrap the creation of bindings, and not a good
> idea in general.
Hi Paolo and Daniel, about this point, what things can be done to help
the community move towards dropping duplicate impl?
For example, about HPET, add test cases (similar to tests/qtest/m48t59-test.c)
to check register read/write to prove that the two impls are consistent?
Thanks,
Zhao
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/5] rust: vmstate: use const_refs_to_static
2025-05-05 10:08 ` [PATCH 4/5] rust: vmstate: use const_refs_to_static Paolo Bonzini
@ 2025-05-07 7:59 ` Zhao Liu
0 siblings, 0 replies; 20+ messages in thread
From: Zhao Liu @ 2025-05-07 7:59 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust
> @@ -998,7 +1000,7 @@ impl ObjectImpl for HPETState {
> vmstate_of!(HPETState, counter),
> vmstate_of!(HPETState, num_timers_save).with_version_id(2),
> vmstate_validate!(HPETState, VALIDATE_TIMERS_NAME, HPETState::validate_num_timers),
> - vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
> + vmstate_of!(HPETState, timers[0 .. num_timers], HPETState::validate_num_timers).with_version_id(0),
> })
The unified vmstate_of is indeed clean... it's more concise and easier
to use than the C version.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-05-07 7:40 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-05 10:08 [PATCH preview 0/5] rust: allow minimum version of 1.83 Paolo Bonzini
2025-05-05 10:08 ` [PATCH 1/5] meson, cargo: require Rust 1.83.0 Paolo Bonzini
2025-05-05 10:08 ` [PATCH 2/5] rust: use inline const expressions Paolo Bonzini
2025-05-06 9:11 ` Zhao Liu
2025-05-06 10:56 ` Paolo Bonzini
2025-05-05 10:08 ` [PATCH 3/5] rust: vmstate: convert to use builder pattern Paolo Bonzini
2025-05-06 9:55 ` Zhao Liu
2025-05-06 10:33 ` Paolo Bonzini
2025-05-05 10:08 ` [PATCH 4/5] rust: vmstate: use const_refs_to_static Paolo Bonzini
2025-05-07 7:59 ` Zhao Liu
2025-05-05 10:08 ` [PATCH 5/5] rust: qdev: const_refs_to_static Paolo Bonzini
2025-05-06 8:56 ` [PATCH preview 0/5] rust: allow minimum version of 1.83 Zhao Liu
2025-05-06 8:43 ` Paolo Bonzini
2025-05-06 9:26 ` Zhao Liu
2025-05-06 9:44 ` Daniel P. Berrangé
2025-05-06 9:48 ` Daniel P. Berrangé
2025-05-06 10:54 ` Paolo Bonzini
2025-05-06 11:52 ` Daniel P. Berrangé
2025-05-06 13:41 ` Paolo Bonzini
2025-05-06 15:56 ` Zhao Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).