* [PATCH 1/7] rust: bql: add BqlRefCell::get_mut()
2025-09-20 14:29 [RFC PATCH 0/7] rust: migration: add high-level migration wrappers Paolo Bonzini
@ 2025-09-20 14:29 ` Paolo Bonzini
2025-09-23 15:12 ` Zhao Liu
2025-09-20 14:29 ` [PATCH 2/7] rust: move VMState from bql to migration Paolo Bonzini
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-20 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, manos.pitsidianakis
This method is rarely useful in QEMU due to the pervasiveness of
shared references, but add it for when a &mut BqlRefCell<> is used.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/bql/src/cell.rs | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/rust/bql/src/cell.rs b/rust/bql/src/cell.rs
index 24ab294b60d..8a0c8c14ad4 100644
--- a/rust/bql/src/cell.rs
+++ b/rust/bql/src/cell.rs
@@ -580,6 +580,25 @@ pub fn borrow_mut(&self) -> BqlRefMut<'_, T> {
}
}
+ /// Returns a mutable reference to the underlying data in this cell,
+ /// while the owner already has a mutable reference to the cell.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use bql::BqlRefCell;
+ ///
+ /// let mut c = BqlRefCell::new(5);
+ ///
+ /// *c.get_mut() = 10;
+ /// ```
+ #[inline]
+ pub const fn get_mut(&mut self) -> &mut T {
+ // SAFETY: there cannot be any outstanding borrow,
+ // since `self` is mutably owned.
+ unsafe { &mut *self.as_ptr() }
+ }
+
/// Returns a raw pointer to the underlying data in this cell.
///
/// # Examples
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/7] rust: bql: add BqlRefCell::get_mut()
2025-09-20 14:29 ` [PATCH 1/7] rust: bql: add BqlRefCell::get_mut() Paolo Bonzini
@ 2025-09-23 15:12 ` Zhao Liu
2025-09-24 12:14 ` Paolo Bonzini
0 siblings, 1 reply; 21+ messages in thread
From: Zhao Liu @ 2025-09-23 15:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On Sat, Sep 20, 2025 at 04:29:52PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:52 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 1/7] rust: bql: add BqlRefCell::get_mut()
> X-Mailer: git-send-email 2.51.0
>
> This method is rarely useful in QEMU due to the pervasiveness of
> shared references, but add it for when a &mut BqlRefCell<> is used.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/bql/src/cell.rs | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/rust/bql/src/cell.rs b/rust/bql/src/cell.rs
> index 24ab294b60d..8a0c8c14ad4 100644
> --- a/rust/bql/src/cell.rs
> +++ b/rust/bql/src/cell.rs
> @@ -580,6 +580,25 @@ pub fn borrow_mut(&self) -> BqlRefMut<'_, T> {
> }
> }
>
> + /// Returns a mutable reference to the underlying data in this cell,
> + /// while the owner already has a mutable reference to the cell.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// use bql::BqlRefCell;
> + ///
> + /// let mut c = BqlRefCell::new(5);
> + ///
> + /// *c.get_mut() = 10;
> + /// ```
> + #[inline]
> + pub const fn get_mut(&mut self) -> &mut T {
> + // SAFETY: there cannot be any outstanding borrow,
> + // since `self` is mutably owned.
> + unsafe { &mut *self.as_ptr() }
Why not use UnsafeCell::get_mut?
self.value.get_mut()
Regards,
Zhao
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/7] rust: bql: add BqlRefCell::get_mut()
2025-09-23 15:12 ` Zhao Liu
@ 2025-09-24 12:14 ` Paolo Bonzini
0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-24 12:14 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, Emmanouil Pitsidianakis
[-- Attachment #1: Type: text/plain, Size: 451 bytes --]
On Tue, Sep 23, 2025, 16:50 Zhao Liu <zhao1.liu@intel.com> wrote:
> > + pub const fn get_mut(&mut self) -> &mut T {
> > + // SAFETY: there cannot be any outstanding borrow,
> > + // since `self` is mutably owned.
> > + unsafe { &mut *self.as_ptr() }
>
> Why not use UnsafeCell::get_mut?
>
> self.value.get_mut()
>
Uh, of course. I thought it was too new, but it's even available as const
in 1.83.
Paolo
Regards,
> Zhao
>
>
[-- Attachment #2: Type: text/html, Size: 1173 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/7] rust: move VMState from bql to migration
2025-09-20 14:29 [RFC PATCH 0/7] rust: migration: add high-level migration wrappers Paolo Bonzini
2025-09-20 14:29 ` [PATCH 1/7] rust: bql: add BqlRefCell::get_mut() Paolo Bonzini
@ 2025-09-20 14:29 ` Paolo Bonzini
2025-09-23 14:46 ` Zhao Liu
2025-09-20 14:29 ` [PATCH 3/7] rust: migration: extract vmstate_fields_ref Paolo Bonzini
` (4 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-20 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, manos.pitsidianakis
The high-level wrapper Migratable<T> will contain a BqlCell,
which would introduce a circular dependency betwen the bql and
migration crates. Move the implementation of VMState for cells
to "migration", together with the implementation for std types.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.lock | 4 +---
rust/bql/Cargo.toml | 3 ---
rust/bql/meson.build | 1 -
rust/bql/src/cell.rs | 6 ------
rust/meson.build | 2 +-
rust/migration/Cargo.toml | 1 +
rust/migration/meson.build | 4 ++--
rust/migration/src/vmstate.rs | 2 ++
8 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 2826c4d027b..960f603cedb 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -57,9 +57,6 @@ dependencies = [
[[package]]
name = "bql"
version = "0.1.0"
-dependencies = [
- "migration",
-]
[[package]]
name = "chardev"
@@ -141,6 +138,7 @@ checksum = "18d287de67fe55fd7e1581fe933d965a5a9477b38e949cfa9f8574ef01506398"
name = "migration"
version = "0.1.0"
dependencies = [
+ "bql",
"common",
"util",
]
diff --git a/rust/bql/Cargo.toml b/rust/bql/Cargo.toml
index 1041bd4ea93..d87edf4c302 100644
--- a/rust/bql/Cargo.toml
+++ b/rust/bql/Cargo.toml
@@ -12,9 +12,6 @@ license.workspace = true
repository.workspace = true
rust-version.workspace = true
-[dependencies]
-migration = { path = "../migration" }
-
[features]
default = ["debug_cell"]
debug_cell = []
diff --git a/rust/bql/meson.build b/rust/bql/meson.build
index 7214d944089..305d7111897 100644
--- a/rust/bql/meson.build
+++ b/rust/bql/meson.build
@@ -36,7 +36,6 @@ _bql_rs = static_library(
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
rust_args: _bql_cfg,
- link_with: [_migration_rs],
)
bql_rs = declare_dependency(link_with: [_bql_rs],
diff --git a/rust/bql/src/cell.rs b/rust/bql/src/cell.rs
index 8a0c8c14ad4..defa45a3294 100644
--- a/rust/bql/src/cell.rs
+++ b/rust/bql/src/cell.rs
@@ -151,8 +151,6 @@
ptr::NonNull,
};
-use migration::impl_vmstate_transparent;
-
/// A mutable memory location that is protected by the Big QEMU Lock.
///
/// # Memory layout
@@ -364,8 +362,6 @@ pub fn take(&self) -> T {
}
}
-impl_vmstate_transparent!(crate::cell::BqlCell<T> where T: VMState);
-
/// A mutable memory location with dynamically checked borrow rules,
/// protected by the Big QEMU Lock.
///
@@ -693,8 +689,6 @@ fn from(t: T) -> BqlRefCell<T> {
}
}
-impl_vmstate_transparent!(crate::cell::BqlRefCell<T> where T: VMState);
-
struct BorrowRef<'b> {
borrow: &'b Cell<BorrowFlag>,
}
diff --git a/rust/meson.build b/rust/meson.build
index 0bb2a9630a1..f4fb7daf05a 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -29,8 +29,8 @@ subdir('qemu-macros')
subdir('common')
subdir('bits')
subdir('util')
-subdir('migration')
subdir('bql')
+subdir('migration')
subdir('qom')
subdir('system')
subdir('chardev')
diff --git a/rust/migration/Cargo.toml b/rust/migration/Cargo.toml
index 708bfaaa682..f4a86275152 100644
--- a/rust/migration/Cargo.toml
+++ b/rust/migration/Cargo.toml
@@ -13,6 +13,7 @@ repository.workspace = true
rust-version.workspace = true
[dependencies]
+bql = { path = "../bql" }
common = { path = "../common" }
util = { path = "../util" }
diff --git a/rust/migration/meson.build b/rust/migration/meson.build
index 2a49bd1633e..2f38da9220f 100644
--- a/rust/migration/meson.build
+++ b/rust/migration/meson.build
@@ -36,12 +36,12 @@ _migration_rs = static_library(
),
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
- link_with: [_util_rs],
+ link_with: [_util_rs, _bql_rs],
dependencies: [common_rs],
)
migration_rs = declare_dependency(link_with: [_migration_rs],
- dependencies: [migration, qemuutil])
+ dependencies: [bql_rs, migration, qemuutil])
# Doctests are essentially integration tests, so they need the same dependencies.
# Note that running them requires the object files for C code, so place them
diff --git a/rust/migration/src/vmstate.rs b/rust/migration/src/vmstate.rs
index e04b19b3c9f..05a833a8b7d 100644
--- a/rust/migration/src/vmstate.rs
+++ b/rust/migration/src/vmstate.rs
@@ -275,6 +275,8 @@ unsafe impl<$base> $crate::vmstate::VMState for $type where $base: $crate::vmsta
};
}
+impl_vmstate_transparent!(bql::BqlCell<T> where T: VMState);
+impl_vmstate_transparent!(bql::BqlRefCell<T> where T: VMState);
impl_vmstate_transparent!(std::cell::Cell<T> where T: VMState);
impl_vmstate_transparent!(std::cell::UnsafeCell<T> where T: VMState);
impl_vmstate_transparent!(std::pin::Pin<T> where T: VMState);
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 2/7] rust: move VMState from bql to migration
2025-09-20 14:29 ` [PATCH 2/7] rust: move VMState from bql to migration Paolo Bonzini
@ 2025-09-23 14:46 ` Zhao Liu
0 siblings, 0 replies; 21+ messages in thread
From: Zhao Liu @ 2025-09-23 14:46 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On Sat, Sep 20, 2025 at 04:29:53PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:53 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 2/7] rust: move VMState from bql to migration
> X-Mailer: git-send-email 2.51.0
>
> The high-level wrapper Migratable<T> will contain a BqlCell,
> which would introduce a circular dependency betwen the bql and
> migration crates. Move the implementation of VMState for cells
> to "migration", together with the implementation for std types.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/Cargo.lock | 4 +---
> rust/bql/Cargo.toml | 3 ---
> rust/bql/meson.build | 1 -
> rust/bql/src/cell.rs | 6 ------
> rust/meson.build | 2 +-
> rust/migration/Cargo.toml | 1 +
> rust/migration/meson.build | 4 ++--
> rust/migration/src/vmstate.rs | 2 ++
> 8 files changed, 7 insertions(+), 16 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/7] rust: migration: extract vmstate_fields_ref
2025-09-20 14:29 [RFC PATCH 0/7] rust: migration: add high-level migration wrappers Paolo Bonzini
2025-09-20 14:29 ` [PATCH 1/7] rust: bql: add BqlRefCell::get_mut() Paolo Bonzini
2025-09-20 14:29 ` [PATCH 2/7] rust: move VMState from bql to migration Paolo Bonzini
@ 2025-09-20 14:29 ` Paolo Bonzini
2025-09-24 15:21 ` Zhao Liu
2025-09-20 14:29 ` [PATCH 4/7] rust: migration: add high-level migration wrappers Paolo Bonzini
` (3 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-20 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, manos.pitsidianakis
This is useful when building a VMState for generic structs, because you have
to avoid nested statics. Using vmstate_fields! will fail in the likely case
where the _FIELDS static uses Self from an outer item, because that is
forbidden.
I also looked into returning a &'static [VMStateField] from vmstate_fields!,
but that also fails; the fields are not promoted to static, which is
unfortunate but intentional (https://github.com/rust-lang/rust/issues/60502):
error[E0716]: temporary value dropped while borrowed
--> rust/hw/char/pl011/libpl011.rlib.p/structured/device.rs:743:17
|
738 | / VMStateDescriptionBuilder::<PL011State>::new()
739 | | .name(c"pl011/clock")
740 | | .version_id(1)
741 | | .minimum_version_id(1)
742 | | .needed(&PL011State::clock_needed)
743 | | .fields(vmstate_fields! {
| | _________________^
744 | || vmstate_of!(PL011State, clock),
745 | || })
| ||_________^- argument requires that borrow lasts for `'static`
| |_________|
| creates a temporary value which is freed while still in use
746 | .build();
| - temporary value is freed at the end of this statement
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/migration/src/vmstate.rs | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/rust/migration/src/vmstate.rs b/rust/migration/src/vmstate.rs
index 05a833a8b7d..421a236194d 100644
--- a/rust/migration/src/vmstate.rs
+++ b/rust/migration/src/vmstate.rs
@@ -413,19 +413,30 @@ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
}};
}
+/// Add a terminator to the fields in the arguments, and return
+/// a reference to the resulting array of values.
+#[macro_export]
+macro_rules! vmstate_fields_ref {
+ ($($field:expr),*$(,)*) => {
+ &[
+ $($field),*,
+ $crate::bindings::VMStateField {
+ flags: $crate::bindings::VMStateFlags::VMS_END,
+ ..::common::zeroable::Zeroable::ZERO
+ }
+ ]
+ }
+}
+
/// 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.
#[macro_export]
macro_rules! vmstate_fields {
($($field:expr),*$(,)*) => {{
- static _FIELDS: &[$crate::bindings::VMStateField] = &[
+ static _FIELDS: &[$crate::bindings::VMStateField] = $crate::vmstate_fields_ref!(
$($field),*,
- $crate::bindings::VMStateField {
- flags: $crate::bindings::VMStateFlags::VMS_END,
- ..::common::zeroable::Zeroable::ZERO
- }
- ];
+ );
_FIELDS.as_ptr()
}}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 3/7] rust: migration: extract vmstate_fields_ref
2025-09-20 14:29 ` [PATCH 3/7] rust: migration: extract vmstate_fields_ref Paolo Bonzini
@ 2025-09-24 15:21 ` Zhao Liu
2025-09-24 15:27 ` Zhao Liu
0 siblings, 1 reply; 21+ messages in thread
From: Zhao Liu @ 2025-09-24 15:21 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
Hi Paolo,
On Sat, Sep 20, 2025 at 04:29:54PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:54 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 3/7] rust: migration: extract vmstate_fields_ref
> X-Mailer: git-send-email 2.51.0
>
> This is useful when building a VMState for generic structs, because you have
> to avoid nested statics. Using vmstate_fields! will fail in the likely case
> where the _FIELDS static uses Self from an outer item, because that is
> forbidden.
vmstate_fields_ref is good for me.
> I also looked into returning a &'static [VMStateField] from vmstate_fields!,
> but that also fails; the fields are not promoted to static, which is
> unfortunate but intentional (https://github.com/rust-lang/rust/issues/60502):
>
> error[E0716]: temporary value dropped while borrowed
> --> rust/hw/char/pl011/libpl011.rlib.p/structured/device.rs:743:17
> |
> 738 | / VMStateDescriptionBuilder::<PL011State>::new()
> 739 | | .name(c"pl011/clock")
> 740 | | .version_id(1)
> 741 | | .minimum_version_id(1)
> 742 | | .needed(&PL011State::clock_needed)
> 743 | | .fields(vmstate_fields! {
> | | _________________^
> 744 | || vmstate_of!(PL011State, clock),
> 745 | || })
> | ||_________^- argument requires that borrow lasts for `'static`
> | |_________|
> | creates a temporary value which is freed while still in use
> 746 | .build();
> | - temporary value is freed at the end of this statement
And I'm curious what change caused this error... I guess you're
indicating the following case?
diff --git a/rust/migration/src/vmstate.rs b/rust/migration/src/vmstate.rs
index c05c4a1fd665..bff6ba502e02 100644
--- a/rust/migration/src/vmstate.rs
+++ b/rust/migration/src/vmstate.rs
@@ -417,14 +417,13 @@ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
#[macro_export]
macro_rules! vmstate_fields {
($($field:expr),*$(,)*) => {{
- static _FIELDS: &[$crate::bindings::VMStateField] = &[
+ &[
$($field),*,
$crate::bindings::VMStateField {
flags: $crate::bindings::VMStateFlags::VMS_END,
..::common::zeroable::Zeroable::ZERO
}
- ];
- _FIELDS.as_ptr()
+ ]
}}
}
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/migration/src/vmstate.rs | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/rust/migration/src/vmstate.rs b/rust/migration/src/vmstate.rs
> index 05a833a8b7d..421a236194d 100644
> --- a/rust/migration/src/vmstate.rs
> +++ b/rust/migration/src/vmstate.rs
> @@ -413,19 +413,30 @@ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
> }};
> }
>
> +/// Add a terminator to the fields in the arguments, and return
> +/// a reference to the resulting array of values.
> +#[macro_export]
> +macro_rules! vmstate_fields_ref {
> + ($($field:expr),*$(,)*) => {
> + &[
> + $($field),*,
> + $crate::bindings::VMStateField {
> + flags: $crate::bindings::VMStateFlags::VMS_END,
> + ..::common::zeroable::Zeroable::ZERO
> + }
> + ]
> + }
> +}
> +
> /// 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.
> #[macro_export]
> macro_rules! vmstate_fields {
> ($($field:expr),*$(,)*) => {{
> - static _FIELDS: &[$crate::bindings::VMStateField] = &[
> + static _FIELDS: &[$crate::bindings::VMStateField] = $crate::vmstate_fields_ref!(
> $($field),*,
> - $crate::bindings::VMStateField {
> - flags: $crate::bindings::VMStateFlags::VMS_END,
> - ..::common::zeroable::Zeroable::ZERO
> - }
> - ];
> + );
> _FIELDS.as_ptr()
Considerring the idea about returing &'static [VMStateField], I think at
least we could return `_FIELDS.as_ptr()` directly from vmstate_fields!,
and convert &'static [VMStateField] to *const [VMStateField] in
fields(), just like subsections() did.
Thanks,
Zhao
> }}
> }
> --
> 2.51.0
>
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 3/7] rust: migration: extract vmstate_fields_ref
2025-09-24 15:21 ` Zhao Liu
@ 2025-09-24 15:27 ` Zhao Liu
2025-09-25 9:24 ` Paolo Bonzini
0 siblings, 1 reply; 21+ messages in thread
From: Zhao Liu @ 2025-09-24 15:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
> Considerring the idea about returing &'static [VMStateField], I think at
> least we could return `_FIELDS.as_ptr()` directly from vmstate_fields!,
^^^^^^^^^^^^^^^^
Sorry for copy error :(, s/_FIELDS.as_ptr()/_FIELDS/
> and convert &'static [VMStateField] to *const [VMStateField] in
> fields(), just like subsections() did.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/7] rust: migration: extract vmstate_fields_ref
2025-09-24 15:27 ` Zhao Liu
@ 2025-09-25 9:24 ` Paolo Bonzini
0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-25 9:24 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, Emmanouil Pitsidianakis
[-- Attachment #1: Type: text/plain, Size: 590 bytes --]
On Wed, Sep 24, 2025, 17:06 Zhao Liu <zhao1.liu@intel.com> wrote:
> > Considerring the idea about returing &'static [VMStateField], I think at
> > least we could return `_FIELDS.as_ptr()` directly from vmstate_fields!,
> ^^^^^^^^^^^^^^^^
> Sorry for copy error :(, s/_FIELDS.as_ptr()/_FIELDS/
>
> > and convert &'static [VMStateField] to *const [VMStateField] in
> > fields(), just like subsections() did.
>
Yes, that's a good idea. Though subsections() can also be cleaned up a lot
to avoid the wrapper. I will include everything in the next version.
Paolo
>
[-- Attachment #2: Type: text/html, Size: 1262 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/7] rust: migration: add high-level migration wrappers
2025-09-20 14:29 [RFC PATCH 0/7] rust: migration: add high-level migration wrappers Paolo Bonzini
` (2 preceding siblings ...)
2025-09-20 14:29 ` [PATCH 3/7] rust: migration: extract vmstate_fields_ref Paolo Bonzini
@ 2025-09-20 14:29 ` Paolo Bonzini
2025-09-25 9:05 ` Zhao Liu
2025-09-20 14:29 ` [PATCH 5/7] rust: qemu-macros: add ToMigrationState derive macro Paolo Bonzini
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-20 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, manos.pitsidianakis
Instead of dealing with pre/post callbacks, allow devices to
implement a snapshot/restore mechanism; this has two main
advantages:
- it can be easily implemented via procedural macros
- there can be generic implementations to deal with various
kinds of interior-mutable containers, from BqlRefCell to Mutex,
so that C code does not see Rust concepts such as Mutex<>.
Using it is easy; you can implement the snapshot/restore trait
ToMigrationState and declare your state like:
regs: Migratable<Mutex<MyDeviceRegisters>>
Migratable<> allows dereferencing to the underlying object with
no run-time cost.
Note that Migratable<> actually does not accept ToMigrationState,
only the similar ToMigrationStateShared trait that the user will mostly
not care about. This is required by the fact that pre/post callbacks
take a &self, and ensures that the argument is a Mutex or BqlRefCell
(including an array or Arc<> thereof).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.lock | 1 +
rust/migration/Cargo.toml | 1 +
rust/migration/meson.build | 1 +
rust/migration/src/lib.rs | 3 +
rust/migration/src/migratable.rs | 430 +++++++++++++++++++++++++++++++
5 files changed, 436 insertions(+)
create mode 100644 rust/migration/src/migratable.rs
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 960f603cedb..32db90066f1 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -140,6 +140,7 @@ version = "0.1.0"
dependencies = [
"bql",
"common",
+ "qemu_macros",
"util",
]
diff --git a/rust/migration/Cargo.toml b/rust/migration/Cargo.toml
index f4a86275152..8efce7a72cb 100644
--- a/rust/migration/Cargo.toml
+++ b/rust/migration/Cargo.toml
@@ -15,6 +15,7 @@ rust-version.workspace = true
[dependencies]
bql = { path = "../bql" }
common = { path = "../common" }
+qemu_macros = { path = "../qemu-macros" }
util = { path = "../util" }
[lints]
diff --git a/rust/migration/meson.build b/rust/migration/meson.build
index 2f38da9220f..c258881790d 100644
--- a/rust/migration/meson.build
+++ b/rust/migration/meson.build
@@ -30,6 +30,7 @@ _migration_rs = static_library(
[
'src/lib.rs',
'src/bindings.rs',
+ 'src/migratable.rs',
'src/vmstate.rs',
],
{'.' : _migration_bindings_inc_rs},
diff --git a/rust/migration/src/lib.rs b/rust/migration/src/lib.rs
index 5f51dde4406..efe9896b619 100644
--- a/rust/migration/src/lib.rs
+++ b/rust/migration/src/lib.rs
@@ -2,5 +2,8 @@
pub mod bindings;
+pub mod migratable;
+pub use migratable::*;
+
pub mod vmstate;
pub use vmstate::*;
diff --git a/rust/migration/src/migratable.rs b/rust/migration/src/migratable.rs
new file mode 100644
index 00000000000..d09eeb35f11
--- /dev/null
+++ b/rust/migration/src/migratable.rs
@@ -0,0 +1,430 @@
+// Copyright 2025 Red Hat, Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::{
+ fmt,
+ mem::size_of,
+ ptr::{self, addr_of, NonNull},
+ sync::{Arc, Mutex},
+};
+
+use bql::{BqlCell, BqlRefCell};
+use common::Zeroable;
+
+use crate::{
+ bindings, vmstate_fields_ref, vmstate_of, InvalidError, VMState, VMStateDescriptionBuilder,
+};
+
+/// Enables QEMU migration support even when a type is wrapped with
+/// synchronization primitives (like `Mutex`) that the C migration
+/// code cannot directly handle. The trait provides methods to
+/// extract essential state for migration and restore it after
+/// migration completes.
+///
+/// On top of extracting data from synchronization wrappers during save
+/// and restoring it during load, it's also possible to use `ToMigrationState`
+/// to convert runtime representations to migration-safe formats.
+///
+/// # Examples
+///
+/// ```
+/// use bql::BqlCell;
+/// use migration::{InvalidError, ToMigrationState, VMState};
+/// # use migration::VMStateField;
+///
+/// # #[derive(Debug, PartialEq, Eq)]
+/// struct DeviceState {
+/// counter: BqlCell<u32>,
+/// enabled: bool,
+/// }
+///
+/// # #[derive(Debug)]
+/// #[derive(Default)]
+/// struct DeviceMigrationState {
+/// counter: u32,
+/// enabled: bool,
+/// }
+///
+/// # unsafe impl VMState for DeviceMigrationState {
+/// # const BASE: VMStateField = ::common::Zeroable::ZERO;
+/// # }
+/// impl ToMigrationState for DeviceState {
+/// type Migrated = DeviceMigrationState;
+///
+/// fn snapshot_migration_state(
+/// &self,
+/// target: &mut Self::Migrated,
+/// ) -> Result<(), InvalidError> {
+/// target.counter = self.counter.get();
+/// target.enabled = self.enabled;
+/// Ok(())
+/// }
+///
+/// fn restore_migrated_state_mut(
+/// &mut self,
+/// source: Self::Migrated,
+/// _version_id: u8,
+/// ) -> Result<(), InvalidError> {
+/// self.counter.set(source.counter);
+/// self.enabled = source.enabled;
+/// Ok(())
+/// }
+/// }
+/// # bql::start_test();
+/// # let dev = DeviceState { counter: 10.into(), enabled: true };
+/// # let mig = dev.to_migration_state().unwrap();
+/// # assert!(matches!(*mig, DeviceMigrationState { counter: 10, enabled: true }));
+/// # let mut dev2 = DeviceState { counter: 42.into(), enabled: false };
+/// # dev2.restore_migrated_state_mut(*mig, 1).unwrap();
+/// # assert_eq!(dev2, dev);
+/// ```
+pub trait ToMigrationState {
+ /// The type used to represent the migrated state.
+ type Migrated: Default + VMState;
+
+ /// Capture the current state into a migration-safe format, failing
+ /// if the state cannot be migrated.
+ fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), InvalidError>;
+
+ /// Restores state from a migrated representation, failing if the
+ /// state cannot be restored.
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError>;
+
+ /// Convenience method to combine allocation and state capture
+ /// into a single operation.
+ fn to_migration_state(&self) -> Result<Box<Self::Migrated>, InvalidError> {
+ let mut migrated = Box::<Self::Migrated>::default();
+ self.snapshot_migration_state(&mut migrated)?;
+ Ok(migrated)
+ }
+}
+
+// Implementations for primitive types. Do not use a blanket implementation
+// for all Copy types, because [T; N] is Copy if T is Copy; that would conflict
+// with the below implementation for arrays.
+macro_rules! impl_for_primitive {
+ ($($t:ty),*) => {
+ $(
+ impl ToMigrationState for $t {
+ type Migrated = Self;
+
+ fn snapshot_migration_state(
+ &self,
+ target: &mut Self::Migrated,
+ ) -> Result<(), InvalidError> {
+ *target = *self;
+ Ok(())
+ }
+
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ _version_id: u8,
+ ) -> Result<(), InvalidError> {
+ *self = source;
+ Ok(())
+ }
+ }
+ )*
+ };
+}
+
+impl_for_primitive!(u8, u16, u32, u64, i8, i16, i32, i64, bool);
+
+impl<T: ToMigrationState, const N: usize> ToMigrationState for [T; N]
+where
+ [T::Migrated; N]: Default,
+{
+ type Migrated = [T::Migrated; N];
+
+ fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), InvalidError> {
+ for (item, target_item) in self.iter().zip(target.iter_mut()) {
+ item.snapshot_migration_state(target_item)?;
+ }
+ Ok(())
+ }
+
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ for (item, source_item) in self.iter_mut().zip(source) {
+ item.restore_migrated_state_mut(source_item, version_id)?;
+ }
+ Ok(())
+ }
+}
+
+/// Extension trait for types that support migration state restoration
+/// through interior mutability.
+///
+/// This trait extends [`ToMigrationState`] for types that can restore
+/// their state without requiring mutable access. While user structs
+/// will generally use `ToMigrationState`, the device will have multiple
+/// references and therefore the device struct has to employ an interior
+/// mutability wrapper like [`Mutex`] or [`BqlRefCell`].
+///
+/// Anything that implements this trait can in turn be used within
+/// [`Migratable<T>`], which makes no assumptions on how to achieve mutable
+/// access to the runtime state.
+///
+/// # Examples
+///
+/// ```
+/// use std::sync::Mutex;
+///
+/// use migration::ToMigrationStateShared;
+///
+/// let device_state = Mutex::new(42);
+/// // Can restore without &mut access
+/// device_state.restore_migrated_state(100, 1).unwrap();
+/// assert_eq!(*device_state.lock().unwrap(), 100);
+/// ```
+pub trait ToMigrationStateShared: ToMigrationState {
+ /// Restores state from a migrated representation to an interior-mutable
+ /// object. Similar to `restore_migrated_state_mut`, but requires a
+ /// shared reference; therefore it can be used to restore a device's
+ /// state even though devices have multiple references to them.
+ fn restore_migrated_state(
+ &self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError>;
+}
+
+impl<T: ToMigrationStateShared, const N: usize> ToMigrationStateShared for [T; N]
+where
+ [T::Migrated; N]: Default,
+{
+ fn restore_migrated_state(
+ &self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ for (item, source_item) in self.iter().zip(source) {
+ item.restore_migrated_state(source_item, version_id)?;
+ }
+ Ok(())
+ }
+}
+
+impl<T: ToMigrationStateShared> ToMigrationState for Arc<T> {
+ type Migrated = T::Migrated;
+
+ fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), InvalidError> {
+ (**self).snapshot_migration_state(target)
+ }
+
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ (**self).restore_migrated_state(source, version_id)
+ }
+}
+
+impl<T: ToMigrationStateShared> ToMigrationStateShared for Arc<T> {
+ fn restore_migrated_state(
+ &self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ (**self).restore_migrated_state(source, version_id)
+ }
+}
+
+// Interior-mutable types only require ToMigrationState for the inner type!
+
+impl<T: ToMigrationState> ToMigrationState for Mutex<T> {
+ type Migrated = T::Migrated;
+
+ fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), InvalidError> {
+ self.lock().unwrap().snapshot_migration_state(target)
+ }
+
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ self.restore_migrated_state(source, version_id)
+ }
+}
+
+impl<T: ToMigrationState> ToMigrationStateShared for Mutex<T> {
+ fn restore_migrated_state(
+ &self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ self.lock()
+ .unwrap()
+ .restore_migrated_state_mut(source, version_id)
+ }
+}
+
+impl<T: ToMigrationState> ToMigrationState for BqlRefCell<T> {
+ type Migrated = T::Migrated;
+
+ fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), InvalidError> {
+ self.borrow().snapshot_migration_state(target)
+ }
+
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ self.get_mut()
+ .restore_migrated_state_mut(source, version_id)
+ }
+}
+
+impl<T: ToMigrationState> ToMigrationStateShared for BqlRefCell<T> {
+ fn restore_migrated_state(
+ &self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ self.borrow_mut()
+ .restore_migrated_state_mut(source, version_id)
+ }
+}
+
+/// A wrapper that enables QEMU migration for types with shared state.
+///
+/// `Migratable<T>` provides a bridge between Rust types that use interior
+/// mutability (like `Mutex<T>`) and QEMU's C-based migration infrastructure.
+/// It manages the lifecycle of migration state and provides automatic
+/// conversion between runtime and migration representations.
+///
+/// ```ignore
+/// # use std::sync::Mutex;
+/// # use migration::Migratable;
+///
+/// pub struct DeviceRegs {
+/// status: u32,
+/// }
+///
+/// pub struct SomeDevice {
+/// // ...
+/// registers: Migratable<Mutex<DeviceRegs>>,
+/// }
+/// ```
+#[repr(C)]
+pub struct Migratable<T: ToMigrationStateShared> {
+ /// Pointer to migration state, valid only during migration operations.
+ /// C vmstate does not support NULL pointers, so no `Option<Box<>>`.
+ migration_state: BqlCell<*mut T::Migrated>,
+
+ /// The runtime state that can be accessed during normal operation
+ runtime_state: T,
+}
+
+impl<T: ToMigrationStateShared> std::ops::Deref for Migratable<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ &self.runtime_state
+ }
+}
+
+impl<T: ToMigrationStateShared> std::ops::DerefMut for Migratable<T> {
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ &mut self.runtime_state
+ }
+}
+
+impl<T: ToMigrationStateShared> Migratable<T> {
+ /// Creates a new `Migratable` wrapper around the given runtime state.
+ ///
+ /// # Returns
+ /// A new `Migratable` instance ready for use and migration
+ pub fn new(runtime_state: T) -> Self {
+ Self {
+ migration_state: BqlCell::new(ptr::null_mut()),
+ runtime_state,
+ }
+ }
+
+ fn pre_save(&self) -> Result<(), InvalidError> {
+ let state = self.runtime_state.to_migration_state()?;
+ self.migration_state.set(Box::into_raw(state));
+ Ok(())
+ }
+
+ fn post_save(&self) -> Result<(), InvalidError> {
+ let state = unsafe { Box::from_raw(self.migration_state.take()) };
+ drop(state);
+ Ok(())
+ }
+
+ fn pre_load(&self) -> Result<(), InvalidError> {
+ self.migration_state
+ .set(Box::into_raw(Box::<T::Migrated>::default()));
+ Ok(())
+ }
+
+ fn post_load(&self, version_id: u8) -> Result<(), InvalidError> {
+ let state = unsafe { Box::from_raw(self.migration_state.take()) };
+ self.runtime_state
+ .restore_migrated_state(*state, version_id)
+ }
+}
+
+impl<T: ToMigrationStateShared + fmt::Debug> fmt::Debug for Migratable<T>
+where
+ T::Migrated: fmt::Debug,
+{
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ let mut struct_f = f.debug_struct("Migratable");
+ struct_f.field("runtime_state", &self.runtime_state);
+
+ let state = NonNull::new(self.migration_state.get()).map(|x| unsafe { x.as_ref() });
+ struct_f.field("migration_state", &state);
+ struct_f.finish()
+ }
+}
+
+impl<T: ToMigrationStateShared + Default> Default for Migratable<T> {
+ fn default() -> Self {
+ Self::new(T::default())
+ }
+}
+
+impl<T: 'static + ToMigrationStateShared> Migratable<T> {
+ const FIELD: bindings::VMStateField = vmstate_of!(Self, migration_state);
+
+ const FIELDS: &[bindings::VMStateField] = vmstate_fields_ref! {
+ Migratable::<T>::FIELD
+ };
+
+ const VMSD: &'static bindings::VMStateDescription = VMStateDescriptionBuilder::<Self>::new()
+ .version_id(1)
+ .minimum_version_id(1)
+ .pre_save(&Self::pre_save)
+ .pre_load(&Self::pre_load)
+ .post_save(&Self::post_save)
+ .post_load(&Self::post_load)
+ .fields(Self::FIELDS.as_ptr())
+ .build()
+ .as_ref();
+}
+
+unsafe impl<T: 'static + ToMigrationStateShared> VMState for Migratable<T> {
+ const BASE: bindings::VMStateField = {
+ bindings::VMStateField {
+ vmsd: addr_of!(*Self::VMSD),
+ size: size_of::<Self>(),
+ flags: bindings::VMStateFlags::VMS_STRUCT,
+ ..Zeroable::ZERO
+ }
+ };
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 4/7] rust: migration: add high-level migration wrappers
2025-09-20 14:29 ` [PATCH 4/7] rust: migration: add high-level migration wrappers Paolo Bonzini
@ 2025-09-25 9:05 ` Zhao Liu
2025-09-25 9:26 ` Paolo Bonzini
0 siblings, 1 reply; 21+ messages in thread
From: Zhao Liu @ 2025-09-25 9:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On Sat, Sep 20, 2025 at 04:29:55PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:55 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 4/7] rust: migration: add high-level migration wrappers
> X-Mailer: git-send-email 2.51.0
>
> Instead of dealing with pre/post callbacks, allow devices to
> implement a snapshot/restore mechanism; this has two main
> advantages:
>
> - it can be easily implemented via procedural macros
>
> - there can be generic implementations to deal with various
> kinds of interior-mutable containers, from BqlRefCell to Mutex,
> so that C code does not see Rust concepts such as Mutex<>.
>
> Using it is easy; you can implement the snapshot/restore trait
> ToMigrationState and declare your state like:
>
> regs: Migratable<Mutex<MyDeviceRegisters>>
>
> Migratable<> allows dereferencing to the underlying object with
> no run-time cost.
>
> Note that Migratable<> actually does not accept ToMigrationState,
> only the similar ToMigrationStateShared trait that the user will mostly
> not care about. This is required by the fact that pre/post callbacks
> take a &self, and ensures that the argument is a Mutex or BqlRefCell
> (including an array or Arc<> thereof).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/Cargo.lock | 1 +
> rust/migration/Cargo.toml | 1 +
> rust/migration/meson.build | 1 +
> rust/migration/src/lib.rs | 3 +
> rust/migration/src/migratable.rs | 430 +++++++++++++++++++++++++++++++
> 5 files changed, 436 insertions(+)
> create mode 100644 rust/migration/src/migratable.rs
The entire wrapper is quite nice. So,
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Only a few comments for comments inline:
> +// Interior-mutable types only require ToMigrationState for the inner type!
> +
extra blank line.
> +impl<T: ToMigrationState> ToMigrationState for Mutex<T> {
> + type Migrated = T::Migrated;
> +
> + fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), InvalidError> {
> + self.lock().unwrap().snapshot_migration_state(target)
> + }
Or maybe your previous sentence is worth commenting on here:
// For non-BQL-protected device we cannot know that another
// thread isn't taking the lock. So, always acquire the lock.
> + fn restore_migrated_state_mut(
> + &mut self,
> + source: Self::Migrated,
> + version_id: u8,
> + ) -> Result<(), InvalidError> {
> + self.restore_migrated_state(source, version_id)
> + }
> +}
> +
> +impl<T: ToMigrationState> ToMigrationStateShared for Mutex<T> {
> + fn restore_migrated_state(
> + &self,
> + source: Self::Migrated,
> + version_id: u8,
> + ) -> Result<(), InvalidError> {
> + self.lock()
> + .unwrap()
> + .restore_migrated_state_mut(source, version_id)
> + }
> +}
> +
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/7] rust: migration: add high-level migration wrappers
2025-09-25 9:05 ` Zhao Liu
@ 2025-09-25 9:26 ` Paolo Bonzini
0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-25 9:26 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, Emmanouil Pitsidianakis
[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]
On Thu, Sep 25, 2025, 10:43 Zhao Liu <zhao1.liu@intel.com> wrote:
> > + fn snapshot_migration_state(&self, target: &mut Self::Migrated) ->
> Result<(), InvalidError> {
> > + self.lock().unwrap().snapshot_migration_state(target)
> > + }
>
> Or maybe your previous sentence is worth commenting on here:
>
> // For non-BQL-protected device we cannot know that another
> // thread isn't taking the lock. So, always acquire the lock.
>
I don't think here there's any alternative, that is a way to write code
without taking the lock. However...
>
> > + fn restore_migrated_state_mut(
> > + &mut self,
> > + source: Self::Migrated,
> > + version_id: u8,
> > + ) -> Result<(), InvalidError> {
> > + self.restore_migrated_state(source, version_id)
>
... this could use get_mut().
Paolo
> + }
> > +}
> > +
> > +impl<T: ToMigrationState> ToMigrationStateShared for Mutex<T> {
> > + fn restore_migrated_state(
> > + &self,
> > + source: Self::Migrated,
> > + version_id: u8,
> > + ) -> Result<(), InvalidError> {
> > + self.lock()
> > + .unwrap()
> > + .restore_migrated_state_mut(source, version_id)
> > + }
> > +}
> > +
>
>
[-- Attachment #2: Type: text/html, Size: 2422 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/7] rust: qemu-macros: add ToMigrationState derive macro
2025-09-20 14:29 [RFC PATCH 0/7] rust: migration: add high-level migration wrappers Paolo Bonzini
` (3 preceding siblings ...)
2025-09-20 14:29 ` [PATCH 4/7] rust: migration: add high-level migration wrappers Paolo Bonzini
@ 2025-09-20 14:29 ` Paolo Bonzini
2025-09-25 12:35 ` Zhao Liu
2025-09-20 14:29 ` [PATCH 6/7] rust: migration: implement ToMigrationState for Timer Paolo Bonzini
2025-09-20 14:29 ` [PATCH 7/7] rust: migration: implement ToMigrationState as part of impl_vmstate_bitsized Paolo Bonzini
6 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-20 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, manos.pitsidianakis
Add a macro that recursively builds the "migrated" version
of a struct.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/migration/meson.build | 2 +-
rust/migration/src/lib.rs | 2 +
rust/migration/src/migratable.rs | 12 +-
rust/qemu-macros/src/lib.rs | 88 +++++++
rust/qemu-macros/src/migration_state.rs | 296 ++++++++++++++++++++++++
rust/qemu-macros/src/tests.rs | 112 ++++++++-
6 files changed, 507 insertions(+), 5 deletions(-)
create mode 100644 rust/qemu-macros/src/migration_state.rs
diff --git a/rust/migration/meson.build b/rust/migration/meson.build
index c258881790d..3843b364c69 100644
--- a/rust/migration/meson.build
+++ b/rust/migration/meson.build
@@ -38,7 +38,7 @@ _migration_rs = static_library(
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
link_with: [_util_rs, _bql_rs],
- dependencies: [common_rs],
+ dependencies: [common_rs, qemu_macros],
)
migration_rs = declare_dependency(link_with: [_migration_rs],
diff --git a/rust/migration/src/lib.rs b/rust/migration/src/lib.rs
index efe9896b619..c9bdf0d4133 100644
--- a/rust/migration/src/lib.rs
+++ b/rust/migration/src/lib.rs
@@ -2,6 +2,8 @@
pub mod bindings;
+pub use qemu_macros::ToMigrationState;
+
pub mod migratable;
pub use migratable::*;
diff --git a/rust/migration/src/migratable.rs b/rust/migration/src/migratable.rs
index d09eeb35f11..fa25317eea8 100644
--- a/rust/migration/src/migratable.rs
+++ b/rust/migration/src/migratable.rs
@@ -79,6 +79,10 @@
/// # dev2.restore_migrated_state_mut(*mig, 1).unwrap();
/// # assert_eq!(dev2, dev);
/// ```
+///
+/// More commonly, the trait is derived through the
+/// [`derive(ToMigrationState)`](qemu_macros::ToMigrationState) procedural
+/// macro.
pub trait ToMigrationState {
/// The type used to represent the migrated state.
type Migrated: Default + VMState;
@@ -305,13 +309,17 @@ fn restore_migrated_state(
/// It manages the lifecycle of migration state and provides automatic
/// conversion between runtime and migration representations.
///
-/// ```ignore
+/// ```
/// # use std::sync::Mutex;
-/// # use migration::Migratable;
+/// # use migration::{Migratable, ToMigrationState, VMState, VMStateField};
///
+/// #[derive(ToMigrationState)]
/// pub struct DeviceRegs {
/// status: u32,
/// }
+/// # unsafe impl VMState for DeviceRegsMigration {
+/// # const BASE: VMStateField = ::common::Zeroable::ZERO;
+/// # }
///
/// pub struct SomeDevice {
/// // ...
diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
index 7ab18061776..8151fb6d4e1 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -13,9 +13,13 @@
Attribute, Data, DeriveInput, Error, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token,
Variant,
};
+
mod bits;
use bits::BitsConstInternal;
+mod migration_state;
+use migration_state::MigrationStateDerive;
+
#[cfg(test)]
mod tests;
@@ -393,3 +397,87 @@ pub fn bits_const_internal(ts: TokenStream) -> TokenStream {
.unwrap_or_else(syn::Error::into_compile_error)
.into()
}
+
+/// Derive macro for generating migration state structures and trait
+/// implementations.
+///
+/// This macro generates a migration state struct and implements the
+/// `ToMigrationState` trait for the annotated struct, enabling state
+/// serialization and restoration. Note that defining a `VMStateDescription`
+/// for the migration state struct is left to the user.
+///
+/// # Container attributes
+///
+/// The following attributes can be applied to the struct:
+///
+/// - `#[migration_state(rename = CustomName)]` - Customizes the name of the
+/// generated migration struct. By default, the generated struct is named
+/// `{OriginalName}Migration`.
+///
+/// # Field attributes
+///
+/// The following attributes can be applied to individual fields:
+///
+/// - `#[migration_state(omit)]` - Excludes the field from the migration state
+/// entirely.
+///
+/// - `#[migration_state(into(Type))]` - Converts the field using `.into()`
+/// during both serialization and restoration.
+///
+/// - `#[migration_state(try_into(Type))]` - Converts the field using
+/// `.try_into()` during both serialization and restoration. Returns
+/// `InvalidError` on conversion failure.
+///
+/// - `#[migration_state(clone)]` - Clones the field value.
+///
+/// Fields without any attributes use `ToMigrationState` recursively; note that
+/// this is a simple copy for types that implement `Copy`.
+///
+/// # Attribute compatibility
+///
+/// - `omit` cannot be used with any other attributes
+/// - only one of `into(Type)`, `try_into(Type)` can be used, but they can be
+/// coupled with `clone`.
+///
+/// # Examples
+///
+/// Basic usage:
+/// ```ignore
+/// #[derive(ToMigrationState)]
+/// struct MyStruct {
+/// field1: u32,
+/// field2: Timer,
+/// }
+/// ```
+///
+/// With attributes:
+/// ```ignore
+/// #[derive(ToMigrationState)]
+/// #[migration_state(rename = CustomMigration)]
+/// struct MyStruct {
+/// #[migration_state(omit)]
+/// runtime_field: u32,
+///
+/// #[migration_state(clone)]
+/// shared_data: String,
+///
+/// #[migration_state(into(Cow<'static, str>), clone)]
+/// converted_field: String,
+///
+/// #[migration_state(try_into(i8))]
+/// fallible_field: u32,
+///
+/// // Default: use ToMigrationState trait recursively
+/// nested_field: NestedStruct,
+///
+/// // Primitive types have a default implementation of ToMigrationState
+/// simple_field: u32,
+/// }
+/// ```
+#[proc_macro_derive(ToMigrationState, attributes(migration_state))]
+pub fn derive_to_migration_state(input: TokenStream) -> TokenStream {
+ let input = parse_macro_input!(input as DeriveInput);
+ MigrationStateDerive::expand(input)
+ .unwrap_or_else(syn::Error::into_compile_error)
+ .into()
+}
diff --git a/rust/qemu-macros/src/migration_state.rs b/rust/qemu-macros/src/migration_state.rs
new file mode 100644
index 00000000000..89feae1e767
--- /dev/null
+++ b/rust/qemu-macros/src/migration_state.rs
@@ -0,0 +1,296 @@
+use std::borrow::Cow;
+
+use proc_macro2::TokenStream;
+use quote::{format_ident, quote, ToTokens};
+use syn::{spanned::Spanned, DeriveInput, Error, Field, Ident, Result, Type};
+
+use crate::get_fields;
+
+#[derive(Debug, Default)]
+enum ConversionMode {
+ #[default]
+ None,
+ Omit,
+ Into(Type),
+ TryInto(Type),
+ ToMigrationState,
+}
+
+impl ConversionMode {
+ fn target_type(&self, original_type: &Type) -> TokenStream {
+ match self {
+ ConversionMode::Into(ty) | ConversionMode::TryInto(ty) => ty.to_token_stream(),
+ ConversionMode::ToMigrationState => {
+ quote! { <#original_type as ToMigrationState>::Migrated }
+ }
+ _ => original_type.to_token_stream(),
+ }
+ }
+}
+
+#[derive(Debug, Default)]
+struct ContainerAttrs {
+ rename: Option<Ident>,
+}
+
+impl ContainerAttrs {
+ fn parse_from(&mut self, attrs: &[syn::Attribute]) -> Result<()> {
+ use attrs::{set, with, Attrs};
+ Attrs::new()
+ .once("rename", with::eq(set::parse(&mut self.rename)))
+ .parse_attrs("migration_state", attrs)?;
+ Ok(())
+ }
+
+ fn parse(attrs: &[syn::Attribute]) -> Result<Self> {
+ let mut container_attrs = Self::default();
+ container_attrs.parse_from(attrs)?;
+ Ok(container_attrs)
+ }
+}
+
+#[derive(Debug, Default)]
+struct FieldAttrs {
+ conversion: ConversionMode,
+ clone: bool,
+}
+
+impl FieldAttrs {
+ fn parse_from(&mut self, attrs: &[syn::Attribute]) -> Result<()> {
+ let mut omit_flag = false;
+ let mut into_type: Option<Type> = None;
+ let mut try_into_type: Option<Type> = None;
+
+ use attrs::{set, with, Attrs};
+ Attrs::new()
+ .once("omit", set::flag(&mut omit_flag))
+ .once("into", with::paren(set::parse(&mut into_type)))
+ .once("try_into", with::paren(set::parse(&mut try_into_type)))
+ .once("clone", set::flag(&mut self.clone))
+ .parse_attrs("migration_state", attrs)?;
+
+ self.conversion = match (omit_flag, into_type, try_into_type, self.clone) {
+ // Valid combinations of attributes first...
+ (true, None, None, false) => ConversionMode::Omit,
+ (false, Some(ty), None, _) => ConversionMode::Into(ty),
+ (false, None, Some(ty), _) => ConversionMode::TryInto(ty),
+ (false, None, None, true) => ConversionMode::None, // clone without conversion
+ (false, None, None, false) => ConversionMode::ToMigrationState, // default behavior
+
+ // ... then the error cases
+ (true, _, _, _) => {
+ return Err(Error::new(
+ attrs[0].span(),
+ "ToMigrationState: omit cannot be used with other attributes",
+ ));
+ }
+ (_, Some(_), Some(_), _) => {
+ return Err(Error::new(
+ attrs[0].span(),
+ "ToMigrationState: into and try_into attributes cannot be used together",
+ ));
+ }
+ };
+
+ Ok(())
+ }
+
+ fn parse(attrs: &[syn::Attribute]) -> Result<Self> {
+ let mut field_attrs = Self::default();
+ field_attrs.parse_from(attrs)?;
+ Ok(field_attrs)
+ }
+}
+
+#[derive(Debug)]
+struct MigrationStateField {
+ name: Ident,
+ original_type: Type,
+ attrs: FieldAttrs,
+}
+
+impl MigrationStateField {
+ fn maybe_clone(&self, mut value: TokenStream) -> TokenStream {
+ if self.attrs.clone {
+ value = quote! { #value.clone() };
+ }
+ value
+ }
+
+ fn generate_migration_state_field(&self) -> TokenStream {
+ let name = &self.name;
+ let field_type = self.attrs.conversion.target_type(&self.original_type);
+
+ quote! {
+ pub #name: #field_type,
+ }
+ }
+
+ fn generate_snapshot_field(&self) -> TokenStream {
+ let name = &self.name;
+ let value = self.maybe_clone(quote! { self.#name });
+
+ match &self.attrs.conversion {
+ ConversionMode::Omit => {
+ unreachable!("Omitted fields are filtered out during processing")
+ }
+ ConversionMode::None => quote! {
+ target.#name = #value;
+ },
+ ConversionMode::Into(_) => quote! {
+ target.#name = #value.into();
+ },
+ ConversionMode::TryInto(_) => quote! {
+ target.#name = #value.try_into().map_err(|_| migration::InvalidError)?;
+ },
+ ConversionMode::ToMigrationState => quote! {
+ self.#name.snapshot_migration_state(&mut target.#name)?;
+ },
+ }
+ }
+
+ fn generate_restore_field(&self) -> TokenStream {
+ let name = &self.name;
+
+ match &self.attrs.conversion {
+ ConversionMode::Omit => {
+ unreachable!("Omitted fields are filtered out during processing")
+ }
+ ConversionMode::None => quote! {
+ self.#name = #name;
+ },
+ ConversionMode::Into(_) => quote! {
+ self.#name = #name.into();
+ },
+ ConversionMode::TryInto(_) => quote! {
+ self.#name = #name.try_into().map_err(|_| migration::InvalidError)?;
+ },
+ ConversionMode::ToMigrationState => quote! {
+ self.#name.restore_migrated_state_mut(#name, version_id)?;
+ },
+ }
+ }
+}
+
+#[derive(Debug)]
+pub struct MigrationStateDerive {
+ input: DeriveInput,
+ fields: Vec<MigrationStateField>,
+ container_attrs: ContainerAttrs,
+}
+
+impl MigrationStateDerive {
+ fn parse(input: DeriveInput) -> Result<Self> {
+ let container_attrs = ContainerAttrs::parse(&input.attrs)?;
+ let fields = get_fields(&input, "ToMigrationState")?;
+ let fields = Self::process_fields(fields)?;
+
+ Ok(Self {
+ input,
+ fields,
+ container_attrs,
+ })
+ }
+
+ fn process_fields(
+ fields: &syn::punctuated::Punctuated<Field, syn::token::Comma>,
+ ) -> Result<Vec<MigrationStateField>> {
+ let processed = fields
+ .iter()
+ .map(|field| {
+ let attrs = FieldAttrs::parse(&field.attrs)?;
+ Ok((field, attrs))
+ })
+ .collect::<Result<Vec<_>>>()?
+ .into_iter()
+ .filter(|(_, attrs)| !matches!(attrs.conversion, ConversionMode::Omit))
+ .map(|(field, attrs)| MigrationStateField {
+ name: field.ident.as_ref().unwrap().clone(),
+ original_type: field.ty.clone(),
+ attrs,
+ })
+ .collect();
+
+ Ok(processed)
+ }
+
+ fn migration_state_name(&self) -> Cow<'_, Ident> {
+ match &self.container_attrs.rename {
+ Some(rename) => Cow::Borrowed(rename),
+ None => Cow::Owned(format_ident!("{}Migration", &self.input.ident)),
+ }
+ }
+
+ fn generate_migration_state_struct(&self) -> TokenStream {
+ let name = self.migration_state_name();
+ let fields = self
+ .fields
+ .iter()
+ .map(MigrationStateField::generate_migration_state_field);
+
+ quote! {
+ #[derive(Default)]
+ pub struct #name {
+ #(#fields)*
+ }
+ }
+ }
+
+ fn generate_snapshot_migration_state(&self) -> TokenStream {
+ let fields = self
+ .fields
+ .iter()
+ .map(MigrationStateField::generate_snapshot_field);
+
+ quote! {
+ fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), migration::InvalidError> {
+ #(#fields)*
+ Ok(())
+ }
+ }
+ }
+
+ fn generate_restore_migrated_state(&self) -> TokenStream {
+ let names: Vec<_> = self.fields.iter().map(|f| &f.name).collect();
+ let fields = self
+ .fields
+ .iter()
+ .map(MigrationStateField::generate_restore_field);
+
+ quote! {
+ fn restore_migrated_state_mut(&mut self, source: Self::Migrated, version_id: u8) -> Result<(), migration::InvalidError> {
+ let Self::Migrated { #(#names),* } = source;
+ #(#fields)*
+ Ok(())
+ }
+ }
+ }
+
+ fn generate(&self) -> TokenStream {
+ let struct_name = &self.input.ident;
+ let generics = &self.input.generics;
+
+ let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
+ let name = self.migration_state_name();
+ let migration_state_struct = self.generate_migration_state_struct();
+ let snapshot_impl = self.generate_snapshot_migration_state();
+ let restore_impl = self.generate_restore_migrated_state();
+
+ quote! {
+ #migration_state_struct
+
+ impl #impl_generics ToMigrationState for #struct_name #ty_generics #where_clause {
+ type Migrated = #name;
+
+ #snapshot_impl
+
+ #restore_impl
+ }
+ }
+ }
+
+ pub fn expand(input: DeriveInput) -> Result<TokenStream> {
+ let tokens = Self::parse(input)?.generate();
+ Ok(tokens)
+ }
+}
diff --git a/rust/qemu-macros/src/tests.rs b/rust/qemu-macros/src/tests.rs
index 00a106612fc..1ce43aa568e 100644
--- a/rust/qemu-macros/src/tests.rs
+++ b/rust/qemu-macros/src/tests.rs
@@ -7,7 +7,7 @@
use super::*;
macro_rules! derive_compile_fail {
- ($derive_fn:ident, $input:expr, $($error_msg:expr),+ $(,)?) => {{
+ ($derive_fn:path, $input:expr, $($error_msg:expr),+ $(,)?) => {{
let input: proc_macro2::TokenStream = $input;
let error_msg = &[$( quote! { ::core::compile_error! { $error_msg } } ),*];
let derive_fn: fn(input: syn::DeriveInput) -> Result<proc_macro2::TokenStream, syn::Error> =
@@ -24,7 +24,7 @@ macro_rules! derive_compile_fail {
}
macro_rules! derive_compile {
- ($derive_fn:ident, $input:expr, $($expected:tt)*) => {{
+ ($derive_fn:path, $input:expr, $($expected:tt)*) => {{
let input: proc_macro2::TokenStream = $input;
let expected: proc_macro2::TokenStream = $($expected)*;
let derive_fn: fn(input: syn::DeriveInput) -> Result<proc_macro2::TokenStream, syn::Error> =
@@ -244,3 +244,111 @@ fn try_from(value: u8) -> Result<Self, u8> {
}
);
}
+
+#[test]
+fn test_derive_to_migration_state() {
+ derive_compile_fail!(
+ MigrationStateDerive::expand,
+ quote! {
+ struct MyStruct {
+ #[migration_state(omit, clone)]
+ bad: u32,
+ }
+ },
+ "ToMigrationState: omit cannot be used with other attributes"
+ );
+ derive_compile_fail!(
+ MigrationStateDerive::expand,
+ quote! {
+ struct MyStruct {
+ #[migration_state(into)]
+ bad: u32,
+ }
+ },
+ "unexpected end of input, expected parentheses"
+ );
+ derive_compile_fail!(
+ MigrationStateDerive::expand,
+ quote! {
+ struct MyStruct {
+ #[migration_state(into(String), try_into(String))]
+ bad: &'static str,
+ }
+ },
+ "ToMigrationState: into and try_into attributes cannot be used together"
+ );
+ derive_compile!(
+ MigrationStateDerive::expand,
+ quote! {
+ #[migration_state(rename = CustomMigration)]
+ struct MyStruct {
+ #[migration_state(omit)]
+ runtime_field: u32,
+
+ #[migration_state(clone)]
+ shared_data: String,
+
+ #[migration_state(into(Cow<'static, str>), clone)]
+ converted_field: String,
+
+ #[migration_state(try_into(i8))]
+ fallible_field: u32,
+
+ nested_field: NestedStruct,
+ simple_field: u32,
+ }
+ },
+ quote! {
+ #[derive(Default)]
+ pub struct CustomMigration {
+ pub shared_data: String,
+ pub converted_field: Cow<'static, str>,
+ pub fallible_field: i8,
+ pub nested_field: <NestedStruct as ToMigrationState>::Migrated,
+ pub simple_field: <u32 as ToMigrationState>::Migrated,
+ }
+ impl ToMigrationState for MyStruct {
+ type Migrated = CustomMigration;
+ fn snapshot_migration_state(
+ &self,
+ target: &mut Self::Migrated
+ ) -> Result<(), migration::InvalidError> {
+ target.shared_data = self.shared_data.clone();
+ target.converted_field = self.converted_field.clone().into();
+ target.fallible_field = self
+ .fallible_field
+ .try_into()
+ .map_err(|_| migration::InvalidError)?;
+ self.nested_field
+ .snapshot_migration_state(&mut target.nested_field)?;
+ self.simple_field
+ .snapshot_migration_state(&mut target.simple_field)?;
+ Ok(())
+ }
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8
+ ) -> Result<(), migration::InvalidError> {
+ let Self::Migrated {
+ shared_data,
+ converted_field,
+ fallible_field,
+ nested_field,
+ simple_field
+ } = source;
+ self.shared_data = shared_data;
+ self.converted_field = converted_field.into();
+ self.fallible_field = fallible_field
+ .try_into()
+ .map_err(|_| migration::InvalidError)?;
+ self.nested_field
+ .restore_migrated_state_mut(nested_field, version_id)?;
+ self.simple_field
+ .restore_migrated_state_mut(simple_field, version_id)?;
+ Ok(())
+ }
+ }
+ }
+ );
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 5/7] rust: qemu-macros: add ToMigrationState derive macro
2025-09-20 14:29 ` [PATCH 5/7] rust: qemu-macros: add ToMigrationState derive macro Paolo Bonzini
@ 2025-09-25 12:35 ` Zhao Liu
2025-09-25 16:56 ` Paolo Bonzini
0 siblings, 1 reply; 21+ messages in thread
From: Zhao Liu @ 2025-09-25 12:35 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On Sat, Sep 20, 2025 at 04:29:56PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:56 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 5/7] rust: qemu-macros: add ToMigrationState derive macro
> X-Mailer: git-send-email 2.51.0
>
> Add a macro that recursively builds the "migrated" version
> of a struct.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/migration/meson.build | 2 +-
> rust/migration/src/lib.rs | 2 +
> rust/migration/src/migratable.rs | 12 +-
> rust/qemu-macros/src/lib.rs | 88 +++++++
> rust/qemu-macros/src/migration_state.rs | 296 ++++++++++++++++++++++++
> rust/qemu-macros/src/tests.rs | 112 ++++++++-
> 6 files changed, 507 insertions(+), 5 deletions(-)
> create mode 100644 rust/qemu-macros/src/migration_state.rs
...
> diff --git a/rust/migration/src/migratable.rs b/rust/migration/src/migratable.rs
> index d09eeb35f11..fa25317eea8 100644
> --- a/rust/migration/src/migratable.rs
> +++ b/rust/migration/src/migratable.rs
> @@ -79,6 +79,10 @@
> /// # dev2.restore_migrated_state_mut(*mig, 1).unwrap();
> /// # assert_eq!(dev2, dev);
> /// ```
> +///
> +/// More commonly, the trait is derived through the
> +/// [`derive(ToMigrationState)`](qemu_macros::ToMigrationState) procedural
> +/// macro.
> pub trait ToMigrationState {
> /// The type used to represent the migrated state.
> type Migrated: Default + VMState;
> @@ -305,13 +309,17 @@ fn restore_migrated_state(
> /// It manages the lifecycle of migration state and provides automatic
> /// conversion between runtime and migration representations.
> ///
> -/// ```ignore
> +/// ```
> /// # use std::sync::Mutex;
> -/// # use migration::Migratable;
> +/// # use migration::{Migratable, ToMigrationState, VMState, VMStateField};
> ///
> +/// #[derive(ToMigrationState)]
> /// pub struct DeviceRegs {
> /// status: u32,
> /// }
> +/// # unsafe impl VMState for DeviceRegsMigration {
> +/// # const BASE: VMStateField = ::common::Zeroable::ZERO;
> +/// # }
Outdated comment? Looks like the DeviceRegsMigration definition is
missing.
> /// pub struct SomeDevice {
> /// // ...
...
> +/// Derive macro for generating migration state structures and trait
> +/// implementations.
> +///
> +/// This macro generates a migration state struct and implements the
> +/// `ToMigrationState` trait for the annotated struct, enabling state
> +/// serialization and restoration. Note that defining a `VMStateDescription`
> +/// for the migration state struct is left to the user.
> +///
> +/// # Container attributes
> +///
> +/// The following attributes can be applied to the struct:
> +///
> +/// - `#[migration_state(rename = CustomName)]` - Customizes the name of the
> +/// generated migration struct. By default, the generated struct is named
> +/// `{OriginalName}Migration`.
> +///
> +/// # Field attributes
> +///
> +/// The following attributes can be applied to individual fields:
> +///
> +/// - `#[migration_state(omit)]` - Excludes the field from the migration state
> +/// entirely.
> +///
> +/// - `#[migration_state(into(Type))]` - Converts the field using `.into()`
> +/// during both serialization and restoration.
> +///
> +/// - `#[migration_state(try_into(Type))]` - Converts the field using
> +/// `.try_into()` during both serialization and restoration. Returns
> +/// `InvalidError` on conversion failure.
Good idea. These conversion modes are very useful, and inspiring.
It may be not necessary for #[property] to integrate into()/try_into()
mode, but the below conversion is ugly:
#[property(rename = "msi", bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8, default = false)]
conversion should happen within the macro parsing process. But unfortunately,
try_into() is not const, maybe I could do this for bit property:
diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
index c459f9bcb42f..e67df57c3712 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -275,7 +275,10 @@ macro_rules! str_to_c_str {
name: ::std::ffi::CStr::as_ptr(#prop_name),
info: #qdev_prop,
offset: ::core::mem::offset_of!(#name, #field_name) as isize,
- bitnr: #bitnr,
+ bitnr: {
+ const _: () = assert!(#bitnr <= u8::MAX as _, "bit exceeds u8 range");
+ #bitnr as u8
+ },
set_default: #set_default,
defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: #defval as u64 },
..::common::Zeroable::ZERO
> +/// - `#[migration_state(clone)]` - Clones the field value.
How about emphasizing the use case?
"Clones the field value, especially for the types don't implement `Copy`."
> +/// Fields without any attributes use `ToMigrationState` recursively; note that
> +/// this is a simple copy for types that implement `Copy`.
> +///
> +/// # Attribute compatibility
> +///
> +/// - `omit` cannot be used with any other attributes
> +/// - only one of `into(Type)`, `try_into(Type)` can be used, but they can be
> +/// coupled with `clone`.
> +///
...
The implementation of the entire macro is great.
> +#[test]
> +fn test_derive_to_migration_state() {
...
> + quote! {
> + #[derive(Default)]
> + pub struct CustomMigration {
> + pub shared_data: String,
> + pub converted_field: Cow<'static, str>,
> + pub fallible_field: i8,
> + pub nested_field: <NestedStruct as ToMigrationState>::Migrated,
> + pub simple_field: <u32 as ToMigrationState>::Migrated,
> + }
In the production code, CustomMigration still needs to implement VMState
trait, so that String & Cow<'static, str> also need to implement VMState
trait. This seems like the thing that we are currently missing.
For test, it's enough to show how the macro works.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 5/7] rust: qemu-macros: add ToMigrationState derive macro
2025-09-25 12:35 ` Zhao Liu
@ 2025-09-25 16:56 ` Paolo Bonzini
0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-25 16:56 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On 9/25/25 14:35, Zhao Liu wrote:
>> +/// #[derive(ToMigrationState)]
>> /// pub struct DeviceRegs {
>> /// status: u32,
>> /// }
>> +/// # unsafe impl VMState for DeviceRegsMigration {
>> +/// # const BASE: VMStateField = ::common::Zeroable::ZERO;
>> +/// # }
>
> Outdated comment? Looks like the DeviceRegsMigration definition is
> missing.
It's defined by the #[derive(ToMigrationState)].
> the below conversion is ugly:>
> #[property(rename = "msi", bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8, default = false)]
>
> conversion should happen within the macro parsing process. But unfortunately,
> try_into() is not const, maybe I could do this for bit property:
>
> diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
> index c459f9bcb42f..e67df57c3712 100644
> --- a/rust/qemu-macros/src/lib.rs
> +++ b/rust/qemu-macros/src/lib.rs
> @@ -275,7 +275,10 @@ macro_rules! str_to_c_str {
> name: ::std::ffi::CStr::as_ptr(#prop_name),
> info: #qdev_prop,
> offset: ::core::mem::offset_of!(#name, #field_name) as isize,
> - bitnr: #bitnr,
> + bitnr: {
> + const _: () = assert!(#bitnr <= u8::MAX as _, "bit exceeds u8 range");
> + #bitnr as u8
> + },
> set_default: #set_default,
> defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: #defval as u64 },
> ..::common::Zeroable::ZERO
Good idea (also testing >= 0 is needed). "const { assert!(...); }" is
even simpler.
>> +/// - `#[migration_state(clone)]` - Clones the field value.
>
> How about emphasizing the use case?
>
> "Clones the field value, especially for the types don't implement `Copy`."
I don't have a use case yet to be honest, but at the same time I want to
help potential device authors and remove the need to muck with the macro.
>> +/// Fields without any attributes use `ToMigrationState` recursively; note that
>> +/// this is a simple copy for types that implement `Copy`.
>> +///
>> +/// # Attribute compatibility
>> +///
>> +/// - `omit` cannot be used with any other attributes
>> +/// - only one of `into(Type)`, `try_into(Type)` can be used, but they can be
>> +/// coupled with `clone`.
>> +///
>
> ...
>
> The implementation of the entire macro is great.
Thanks. :) It's indeed pretty easy to follow, and I like procedural
macro code that is simple but powerful.
The attrs crate also helps a lot!
>> +#[test]
>> +fn test_derive_to_migration_state() {
>
> ...
>
>> + quote! {
>> + #[derive(Default)]
>> + pub struct CustomMigration {
>> + pub shared_data: String,
>> + pub converted_field: Cow<'static, str>,
>> + pub fallible_field: i8,
>> + pub nested_field: <NestedStruct as ToMigrationState>::Migrated,
>> + pub simple_field: <u32 as ToMigrationState>::Migrated,
>> + }
>
> In the production code, CustomMigration still needs to implement VMState
> trait, so that String & Cow<'static, str> also need to implement VMState
> trait. This seems like the thing that we are currently missing.
Or more simply they're not chosen well. :) For the documentation I will
think of better types.
> For test, it's enough to show how the macro works.
Yes, for testing it's a lesser deal.
Paolo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/7] rust: migration: implement ToMigrationState for Timer
2025-09-20 14:29 [RFC PATCH 0/7] rust: migration: add high-level migration wrappers Paolo Bonzini
` (4 preceding siblings ...)
2025-09-20 14:29 ` [PATCH 5/7] rust: qemu-macros: add ToMigrationState derive macro Paolo Bonzini
@ 2025-09-20 14:29 ` Paolo Bonzini
2025-09-29 16:12 ` Zhao Liu
2025-09-20 14:29 ` [PATCH 7/7] rust: migration: implement ToMigrationState as part of impl_vmstate_bitsized Paolo Bonzini
6 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-20 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, manos.pitsidianakis
Timer is a complex struct, allow adding it to a struct that
uses #[derive(ToMigrationState)]; similar to vmstate_timer, only
the expiration time has to be preserved.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/migration/src/migratable.rs | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/rust/migration/src/migratable.rs b/rust/migration/src/migratable.rs
index fa25317eea8..c4ad4f73d5c 100644
--- a/rust/migration/src/migratable.rs
+++ b/rust/migration/src/migratable.rs
@@ -202,6 +202,37 @@ fn restore_migrated_state(
) -> Result<(), InvalidError>;
}
+impl ToMigrationState for util::timer::Timer {
+ type Migrated = i64;
+
+ fn snapshot_migration_state(&self, target: &mut i64) -> Result<(), InvalidError> {
+ // SAFETY: as_ptr() is unsafe to ensure that the caller reasons about
+ // the pinning of the data inside the Opaque<>. Here all we do is
+ // access a field.
+ *target = unsafe { &*self.as_ptr() }.expire_time;
+ Ok(())
+ }
+
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), InvalidError> {
+ self.restore_migrated_state(source, version_id)
+ }
+}
+
+impl ToMigrationStateShared for util::timer::Timer {
+ fn restore_migrated_state(&self, source: i64, _version_id: u8) -> Result<(), InvalidError> {
+ if source >= 0 {
+ self.modify(source as u64);
+ } else {
+ self.delete();
+ }
+ Ok(())
+ }
+}
+
impl<T: ToMigrationStateShared, const N: usize> ToMigrationStateShared for [T; N]
where
[T::Migrated; N]: Default,
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 6/7] rust: migration: implement ToMigrationState for Timer
2025-09-20 14:29 ` [PATCH 6/7] rust: migration: implement ToMigrationState for Timer Paolo Bonzini
@ 2025-09-29 16:12 ` Zhao Liu
2025-09-29 16:11 ` Paolo Bonzini
0 siblings, 1 reply; 21+ messages in thread
From: Zhao Liu @ 2025-09-29 16:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On Sat, Sep 20, 2025 at 04:29:57PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:57 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 6/7] rust: migration: implement ToMigrationState for Timer
> X-Mailer: git-send-email 2.51.0
>
> Timer is a complex struct, allow adding it to a struct that
> uses #[derive(ToMigrationState)]; similar to vmstate_timer, only
> the expiration time has to be preserved.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/migration/src/migratable.rs | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/rust/migration/src/migratable.rs b/rust/migration/src/migratable.rs
> index fa25317eea8..c4ad4f73d5c 100644
> --- a/rust/migration/src/migratable.rs
> +++ b/rust/migration/src/migratable.rs
> @@ -202,6 +202,37 @@ fn restore_migrated_state(
> ) -> Result<(), InvalidError>;
> }
>
> +impl ToMigrationState for util::timer::Timer {
> + type Migrated = i64;
This converts a Timer to i64, then we don't need vmstate_info_timer
anymore. Good idea.
> + fn snapshot_migration_state(&self, target: &mut i64) -> Result<(), InvalidError> {
> + // SAFETY: as_ptr() is unsafe to ensure that the caller reasons about
> + // the pinning of the data inside the Opaque<>. Here all we do is
> + // access a field.
> + *target = unsafe { &*self.as_ptr() }.expire_time;
C side checks timer_pending(), which ensures when the timer is
inactive, it always saves u64::MAX.
But now we save the expire_time directly in Rust. I think this would be
possible to break the migration from Rust timer to C timer, because at
C side, timer_get() checks whether expire_time is -1 and we can't ensure
expire_time won't store -2 (or other unusual negative values).
So maybe it's better to follow C side behavior to return u64::MAX for
non-pending case?
> + Ok(())
> + }
> +
> + fn restore_migrated_state_mut(
> + &mut self,
> + source: Self::Migrated,
> + version_id: u8,
> + ) -> Result<(), InvalidError> {
> + self.restore_migrated_state(source, version_id)
> + }
> +}
> +
> +impl ToMigrationStateShared for util::timer::Timer {
> + fn restore_migrated_state(&self, source: i64, _version_id: u8) -> Result<(), InvalidError> {
> + if source >= 0 {
This can work for the migration cases of C->Rust and Rust->Rust.
Thanks,
Zhao
> + self.modify(source as u64);
> + } else {
> + self.delete();
> + }
> + Ok(())
> + }
> +}
> +
> impl<T: ToMigrationStateShared, const N: usize> ToMigrationStateShared for [T; N]
> where
> [T::Migrated; N]: Default,
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 6/7] rust: migration: implement ToMigrationState for Timer
2025-09-29 16:12 ` Zhao Liu
@ 2025-09-29 16:11 ` Paolo Bonzini
0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-29 16:11 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On 9/29/25 18:12, Zhao Liu wrote:
>> + fn snapshot_migration_state(&self, target: &mut i64) -> Result<(), InvalidError> {
>> + // SAFETY: as_ptr() is unsafe to ensure that the caller reasons about
>> + // the pinning of the data inside the Opaque<>. Here all we do is
>> + // access a field.
>> + *target = unsafe { &*self.as_ptr() }.expire_time;
>
> C side checks timer_pending(), which ensures when the timer is
> inactive, it always saves u64::MAX.
>
> But now we save the expire_time directly in Rust. I think this would be
> possible to break the migration from Rust timer to C timer, because at
> C side, timer_get() checks whether expire_time is -1 and we can't ensure
> expire_time won't store -2 (or other unusual negative values).
I think this should work in both cases because timer_pending() checks >=
0 and negative values are extremely far in the future. But I'll change
it to timer_expire_time_ns() for safety and clarity.
Paolo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 7/7] rust: migration: implement ToMigrationState as part of impl_vmstate_bitsized
2025-09-20 14:29 [RFC PATCH 0/7] rust: migration: add high-level migration wrappers Paolo Bonzini
` (5 preceding siblings ...)
2025-09-20 14:29 ` [PATCH 6/7] rust: migration: implement ToMigrationState for Timer Paolo Bonzini
@ 2025-09-20 14:29 ` Paolo Bonzini
2025-09-30 6:53 ` Zhao Liu
6 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2025-09-20 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, manos.pitsidianakis
This is most likely desirable, and is the easiest way to migrate
a bit-sized value without peeking at the innards of the bilge crate.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/migration/src/vmstate.rs | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/rust/migration/src/vmstate.rs b/rust/migration/src/vmstate.rs
index 421a236194d..882dab746fc 100644
--- a/rust/migration/src/vmstate.rs
+++ b/rust/migration/src/vmstate.rs
@@ -295,6 +295,25 @@ unsafe impl $crate::vmstate::VMState for $type {
as ::bilge::prelude::Number>::UnderlyingType
as $crate::vmstate::VMState>::VARRAY_FLAG;
}
+
+ impl $crate::migratable::ToMigrationState for $type {
+ type Migrated = <<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
+ as ::bilge::prelude::Number>::UnderlyingType;
+
+ fn snapshot_migration_state(&self, target: &mut Self::Migrated) -> Result<(), $crate::InvalidError> {
+ *target = Self::Migrated::from(*self);
+ Ok(())
+ }
+
+ fn restore_migrated_state_mut(
+ &mut self,
+ source: Self::Migrated,
+ version_id: u8,
+ ) -> Result<(), $crate::InvalidError> {
+ *self = Self::from(source);
+ Ok(())
+ }
+ }
};
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 7/7] rust: migration: implement ToMigrationState as part of impl_vmstate_bitsized
2025-09-20 14:29 ` [PATCH 7/7] rust: migration: implement ToMigrationState as part of impl_vmstate_bitsized Paolo Bonzini
@ 2025-09-30 6:53 ` Zhao Liu
0 siblings, 0 replies; 21+ messages in thread
From: Zhao Liu @ 2025-09-30 6:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, manos.pitsidianakis
On Sat, Sep 20, 2025 at 04:29:58PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:58 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 7/7] rust: migration: implement ToMigrationState as part of
> impl_vmstate_bitsized
> X-Mailer: git-send-email 2.51.0
>
> This is most likely desirable, and is the easiest way to migrate
> a bit-sized value without peeking at the innards of the bilge crate.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/migration/src/vmstate.rs | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 21+ messages in thread