[PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test

[PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test

[Zhao Liu]

Hi,

This series is in preparation for HPET migration support (in particular,
to support varray and vmstate_validate), and it also cleans up and fixes
the current vmstate! However, there is still the gap from being a ‘stable’
vmstate.


Patch Summary
=============

Patch 1-11: Clean up & fix for vmstate_of & vmstate_struct, where the
            issues are catched by unit tests.

Patch 12,13: Add versioned vmstate and vmstate_validate support, and
             vmstate_validate can accept safe "test" callback.

Patch 13-17: Add unit test to cover as much as possible cases to be
             compatible with C version macros.

             * Note while in principle Rust's vmstate pattern doesn't
               have to match the C version, the C vmstate macros are
               rich enough to cover as much logic as possible. So
               checking against the C version is the most effective way
               to detect the error.

With unit tests, the 2 vmstate gaps that come to mind right now are:

 * `test` parameter in vmstate_of/vmstate_struct. I think there's not
   too much difficulty, since referring to vmstate_validate makes it
   straightforward...

 * pointer to `struct`. assert_field_type() can't aware a inner type of
   the pointer. We may need another trait (different from
   impl_vmstate_pointer). Or, we may need to use the new
   vmstate_struct_ptr macro. But I haven't tried it in details yet.


Thoughts about 'stable' vmstate
===============================

To make vmstate 'stable', I think one key point is to make vmstate
related things accept "safe" callbacks.

vmstate_validate (and future `test` parameters) has achieved this. But
VMStateDescription hasn't and it has the following callbacks:
    int (*pre_load)(void *opaque);
    int (*post_load)(void *opaque, int version_id);
    int (*pre_save)(void *opaque);
    int (*post_save)(void *opaque);
    bool (*needed)(void *opaque);
    bool (*dev_unplug_pending)(void *opaque);

I find there would be 2 options to address this:

1. introduce macros (like vmstate_validate) to accept "safe" callback.
   For example,

static VMSTATE_HPET: VMStateDescription = VMStateDescription {
    name: c_str!("hpet").as_ptr(),
    version_id: 2,
    minimum_version_id: 1,
    pre_save: pre_save!(hpet_pre_save), // the pre_save macro will convert "safe" hpet_pre_save() to C style callback.
    post_load: post_load!(hpet_post_load), // ditto
    fields: vmstate_fields! {
        vmstate_of!(HPETState, config),
        vmstate_of!(HPETState, int_status),
        vmstate_of!(HPETState, counter),
        vmstate_of!(HPETState, num_timers, version = 2),
        vmstate_validate!(HPETState, VALIDATE_TIMERS_NAME, HPETState::validate_num_timers),
        vmstate_struct!(HPETState, timers, [0 .. num_timers], VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, version = 0),
    },
    subsections: vmstate_subsections! {
        VMSTATE_HPET_RTC_IRQ_LEVEL,
        VMSTATE_HPET_OFFSET,
    },
    ..Zeroable::ZERO
};

2. introduce VMStateDescriptionBuilder (like MemoryRegionOpsBuilder) and
   use the call chain to accept callbacks and initialize VMStateField.


About these 2 options, which one do you like?


Best Regards,
Zhao
---
Zhao Liu (17):
  rust/vmstate: Remove unnecessary unsafe
  rust/vmstate: Fix num_offset in vmstate macros
  rust/vmstate: Add a prefix separator ", " for the array field in
    vmstate macros
  rust/vmstate: Use ident instead of expr to parse vmsd in
    vmstate_struct macro
  rust/vmstate: Fix num field when varray flags are set
  rust/vmstate: Fix size field of VMStateField with VMS_ARRAY_OF_POINTER
    flag
  rust/vmstate: Fix type check for varray in vmstate_struct
  rust/vmstate: Fix "cannot infer type" error in vmstate_struct
  rust/vmstate: Fix unnecessary VMState bound of with_varray_flag()
  rust/vmstate: Relax array check when build varray in vmstate_struct
  rust/vmstate: Re-implement VMState trait for timer binding
  rust/vmstate: Support version field in vmstate macros
  rust/vmstate: Support vmstate_validate
  rust/vmstate: Add unit test for vmstate_of macro
  rust/vmstate: Add unit test for vmstate_{of|struct} macro
  rust/vmstate: Add unit test for pointer case
  rust/vmstate: Add unit test for vmstate_validate

 rust/hw/char/pl011/src/device_class.rs |   2 +-
 rust/qemu-api/meson.build              |   5 +-
 rust/qemu-api/src/assertions.rs        |  15 +
 rust/qemu-api/src/vmstate.rs           | 110 ++++--
 rust/qemu-api/tests/tests.rs           |   2 +
 rust/qemu-api/tests/vmstate_tests.rs   | 467 +++++++++++++++++++++++++
 6 files changed, 575 insertions(+), 26 deletions(-)
 create mode 100644 rust/qemu-api/tests/vmstate_tests.rs

-- 
2.34.1

[PATCH 01/17] rust/vmstate: Remove unnecessary unsafe

[Zhao Liu]

Remove the `unsafe` block of vmsd, because vmsd (passed to
vmstate_struct) is defined in Rust side now, and it doesn't need
`unsafe`.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index f0510ae769d1..6698dfe7aeb8 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -447,7 +447,7 @@ macro_rules! vmstate_struct {
             },
             size: ::core::mem::size_of::<$type>(),
             flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
-            vmsd: unsafe { $vmsd },
+            vmsd: $vmsd,
             ..$crate::zeroable::Zeroable::ZERO $(
                 .with_varray_flag($crate::call_func_with_field!(
                     $crate::vmstate::vmstate_varray_flag,
-- 
2.34.1

[PATCH 02/17] rust/vmstate: Fix num_offset in vmstate macros

[Zhao Liu]

`num_offset` is a member of `VMStateField`, and there's no need to use
"." to access this field in a `VMStateField` instance.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 6698dfe7aeb8..9533b1250fa5 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -208,7 +208,7 @@ macro_rules! vmstate_of {
                 .as_bytes()
                 .as_ptr() as *const ::std::os::raw::c_char,
             offset: $crate::offset_of!($struct_name, $field_name),
-            $(.num_offset: $crate::offset_of!($struct_name, $num),)?
+            $(num_offset: $crate::offset_of!($struct_name, $num),)?
             // 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!(
@@ -440,7 +440,7 @@ macro_rules! vmstate_struct {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
                 .as_ptr() as *const ::std::os::raw::c_char,
-            $(.num_offset: $crate::offset_of!($struct_name, $num),)?
+            $(num_offset: $crate::offset_of!($struct_name, $num),)?
             offset: {
                 $crate::assert_field_type!($struct_name, $field_name, $type);
                 $crate::offset_of!($struct_name, $field_name)
-- 
2.34.1

[PATCH 03/17] rust/vmstate: Add a prefix separator ", " for the array field in vmstate macros

[Zhao Liu]

The fields are separated by ",", so it's necessary to add ", " in array
field to avoid matching failure.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 9533b1250fa5..94efbd8bb735 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -202,7 +202,7 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
 /// and [`impl_vmstate_forward!`](crate::impl_vmstate_forward) help with this.
 #[macro_export]
 macro_rules! vmstate_of {
-    ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
+    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
         $crate::bindings::VMStateField {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
@@ -435,7 +435,7 @@ macro_rules! vmstate_unused {
 #[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 $(,)?) => {
+    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])?, $vmsd:expr, $type:ty $(,)?) => {
         $crate::bindings::VMStateField {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
-- 
2.34.1

[PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro

[Zhao Liu]

When specify an array field in vmstate_struct macro, there will be an
error:

> local ambiguity when calling macro `vmstate_struct`: multiple parsing
> options: built-in NTs expr ('vmsd') or 1 other option.

This is because "expr" can't recognize the "vmsd" field correctly, so
that it gets confused with the previous array field.

To fix the above issue, use "ident" for "vmsd" field, and explicitly
refer to it in the macro.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/char/pl011/src/device_class.rs | 2 +-
 rust/qemu-api/src/vmstate.rs           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index 0b2076ddaa0f..e43a5d6cd063 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -72,7 +72,7 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
     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>),
+        vmstate_struct!(PL011State, regs, VMSTATE_PL011_REGS, BqlRefCell<PL011Registers>),
     },
     subsections: vmstate_subsections! {
         VMSTATE_PL011_CLOCK
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 94efbd8bb735..3f95d4825149 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -435,7 +435,7 @@ macro_rules! vmstate_unused {
 #[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 $(,)?) => {
+    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])?, $vmsd:ident, $type:ty $(,)?) => {
         $crate::bindings::VMStateField {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
@@ -447,7 +447,7 @@ macro_rules! vmstate_struct {
             },
             size: ::core::mem::size_of::<$type>(),
             flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
-            vmsd: $vmsd,
+            vmsd: &$vmsd,
             ..$crate::zeroable::Zeroable::ZERO $(
                 .with_varray_flag($crate::call_func_with_field!(
                     $crate::vmstate::vmstate_varray_flag,
-- 
2.34.1

[PATCH 05/17] rust/vmstate: Fix num field when varray flags are set

[Zhao Liu]

Array type vmstate has the VMStateField with `num` equals its length.

When the varray vmstate is built based a array type, the `num` field
should be cleaned to 0, because varray uses `num_offset` instead of
`num` to store elements number information.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 3f95d4825149..e877ee5e391d 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -275,6 +275,7 @@ pub const fn with_varray_flag<T: VMState>(mut self, flag: VMStateFlags) -> VMSta
         assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
         self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
         self.flags = VMStateFlags(self.flags.0 | flag.0);
+        self.num = 0; // varray uses num_offset instead of num.
         self
     }
 
-- 
2.34.1

[PATCH 06/17] rust/vmstate: Fix size field of VMStateField with VMS_ARRAY_OF_POINTER flag

[Zhao Liu]

The `size` field of the VMStateField with VMS_ARRAY_OF_POINTER flag
should stores the size of pointer, which depends on platform.

Currently, `*const`, `*mut`, `NonNull`, `Box<>` and their wrapper are
supported, and they have the same size as `usize`.

Store the size (of `usize`) when VMS_ARRAY_OF_POINTER flag is set.

The size may be changed when more smart pointers are supported, but now
the size of "usize" is enough.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index e877ee5e391d..5af75b4777e9 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -256,6 +256,10 @@ pub const fn with_array_flag(mut self, num: usize) -> Self {
         if (self.flags.0 & VMStateFlags::VMS_POINTER.0) != 0 {
             self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_POINTER.0);
             self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0);
+            // VMS_ARRAY_OF_POINTER flag stores the size of pointer.
+            // FIXME: *const, *mut, NonNull and Box<> have the same size as usize.
+            //        Resize if more smart pointers are supported.
+            self.size = std::mem::size_of::<usize>();
         }
         self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_SINGLE.0);
         self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_ARRAY.0);
-- 
2.34.1

[PATCH 07/17] rust/vmstate: Fix type check for varray in vmstate_struct

[Zhao Liu]

When pass a varray to vmstate_struct, the `type` parameter should be the
type of the element in the varray, for example:

vmstate_struct!(HPETState, timers, [0 .. num_timers], VMSTATE_HPET_TIMER,
		BqlRefCell<HPETTimer>, version = 0)

But this breaks current type check, because it checks the type of
`field`, which is an array type (for the above example, type of timers
is [BqlRefCell<HPETTimer>; 32], not BqlRefCell<HPETTimer>).

But the current assert_field_type() can no longer be extended to include
new arguments, so a variant of it (a second macro containing the
`num = $num:ident` parameter) had to be added to handle array cases.

In this new macro, it not only checks the type of element, but also
checks whether the `num` (number of elements in varray) is out of range.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/assertions.rs | 15 +++++++++++++++
 rust/qemu-api/src/vmstate.rs    |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs
index 104dec39774e..176060e32acd 100644
--- a/rust/qemu-api/src/assertions.rs
+++ b/rust/qemu-api/src/assertions.rs
@@ -91,6 +91,21 @@ fn types_must_be_equal<T, U>(_: T)
             }
         };
     };
+
+    ($t:ty, $i:tt, $ti:ty, num = $num:ident) => {
+        const _: () = {
+            #[allow(unused)]
+            fn assert_field_type(v: $t) {
+                fn types_must_be_equal<T, U>(_: T)
+                where
+                    T: $crate::assertions::EqType<Itself = U>,
+                {
+                }
+                let index: usize = v.$num.try_into().unwrap();
+                types_must_be_equal::<_, &$ti>(&v.$i[index]);
+            }
+        };
+    };
 }
 
 /// 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 5af75b4777e9..8adef175634a 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -447,7 +447,7 @@ macro_rules! vmstate_struct {
                 .as_ptr() as *const ::std::os::raw::c_char,
             $(num_offset: $crate::offset_of!($struct_name, $num),)?
             offset: {
-                $crate::assert_field_type!($struct_name, $field_name, $type);
+                $crate::assert_field_type!($struct_name, $field_name, $type $(, num = $num)?);
                 $crate::offset_of!($struct_name, $field_name)
             },
             size: ::core::mem::size_of::<$type>(),
-- 
2.34.1

[PATCH 08/17] rust/vmstate: Fix "cannot infer type" error in vmstate_struct

[Zhao Liu]

Rust cannot infer the type (it should be VMStateField) after
Zeroable::ZERO, which cause the compiling error.

To fix this error, call with_varray_flag() after VMStateField's
initialization.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 8adef175634a..a9d97d9a856e 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -453,13 +453,15 @@ macro_rules! vmstate_struct {
             size: ::core::mem::size_of::<$type>(),
             flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
             vmsd: &$vmsd,
-            ..$crate::zeroable::Zeroable::ZERO $(
-                .with_varray_flag($crate::call_func_with_field!(
-                    $crate::vmstate::vmstate_varray_flag,
-                    $struct_name,
-                    $num))
-               $(.with_varray_multiply($factor))?)?
-        }
+            ..$crate::zeroable::Zeroable::ZERO
+         } $(.with_varray_flag(
+                  $crate::call_func_with_field!(
+                      $crate::vmstate::vmstate_varray_flag,
+                      $struct_name,
+                      $num
+                  )
+              )
+           $(.with_varray_multiply($factor))?)?
     };
 }
 
-- 
2.34.1

[PATCH 09/17] rust/vmstate: Fix unnecessary VMState bound of with_varray_flag()

[Zhao Liu]

The VMState type bound is not used in with_varray_flag().

And for vmstate_struct, Rust cannot infer the type of `num` from the
call_func_with_field(), so this causes the compiling error because it
complains "cannot satisfy `_: VMState`" in with_varray_flag().

Note Rust can infer the type in vmstate_of macro so that
with_varray_flag() can work at there. It is possible that the different
initialization ways in the two macros cause differences in Rust's
type inference.

But in fact, the VMState type bound is not used in with_varray_flag()
and vmstate_varray_flag() has already checked the VMState type, it's
safe to drop VMState bound of with_varray_flag(), which can fix the
above compiling error.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index a9d97d9a856e..2749e8af2b5a 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -275,7 +275,7 @@ pub const fn with_pointer_flag(mut self) -> Self {
     }
 
     #[must_use]
-    pub const fn with_varray_flag<T: VMState>(mut self, flag: VMStateFlags) -> VMStateField {
+    pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> VMStateField {
         assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
         self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
         self.flags = VMStateFlags(self.flags.0 | flag.0);
-- 
2.34.1

[PATCH 10/17] rust/vmstate: Relax array check when build varray in vmstate_struct

[Zhao Liu]

The varry of structure created by vmstate_struct is different with
vmstate_of. This is because vmstate_struct uses the `vmsd` to traverse
the vmstates of structure's fields, rather than treating the structure
directly as a well-defined vmstate.

Therefore, there's no need to check array flag when building varray by
vmstate_struct.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 2749e8af2b5a..806d531b0c37 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -275,14 +275,20 @@ pub const fn with_pointer_flag(mut self) -> Self {
     }
 
     #[must_use]
-    pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> VMStateField {
-        assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
+    pub const fn with_varray_flag_unchecked(mut self, flag: VMStateFlags) -> VMStateField {
         self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
         self.flags = VMStateFlags(self.flags.0 | flag.0);
         self.num = 0; // varray uses num_offset instead of num.
         self
     }
 
+    #[must_use]
+    #[allow(unused_mut)]
+    pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> VMStateField {
+        assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
+        self.with_varray_flag_unchecked(flag)
+    }
+
     #[must_use]
     pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
         assert!(num <= 0x7FFF_FFFFu32);
@@ -454,7 +460,7 @@ macro_rules! vmstate_struct {
             flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
             vmsd: &$vmsd,
             ..$crate::zeroable::Zeroable::ZERO
-         } $(.with_varray_flag(
+         } $(.with_varray_flag_unchecked(
                   $crate::call_func_with_field!(
                       $crate::vmstate::vmstate_varray_flag,
                       $struct_name,
-- 
2.34.1

[PATCH 11/17] rust/vmstate: Re-implement VMState trait for timer binding

[Zhao Liu]

At present, Rust side has a timer binding "timer::Timer", so the vmstate
for timer should base on that binding instead of the raw
"binding::QEMUTimer".

It's possible to apply impl_vmstate_transparent for cell::Opaque and
then impl_vmstate_forward for timer::Timer. But binding::QEMUTimer
shouldn't be used directly, so that vmstate for such raw timer type is
useless.

Thus, apply impl_vmstate_scalar for timer::Timer. And since Opaque<> is
useful, apply impl_vmstate_transparent for cell::Opaque as well.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 806d531b0c37..3d4c50ca86f9 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -27,12 +27,7 @@
 use core::{marker::PhantomData, mem, ptr::NonNull};
 
 pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{
-    bindings::{self, VMStateFlags},
-    prelude::*,
-    qom::Owned,
-    zeroable::Zeroable,
-};
+use crate::{bindings::VMStateFlags, prelude::*, qom::Owned, zeroable::Zeroable};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
@@ -344,6 +339,7 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
 impl_vmstate_transparent!(std::pin::Pin<T> where T: VMState);
 impl_vmstate_transparent!(crate::cell::BqlCell<T> where T: VMState);
 impl_vmstate_transparent!(crate::cell::BqlRefCell<T> where T: VMState);
+impl_vmstate_transparent!(crate::cell::Opaque<T> where T: VMState);
 
 #[macro_export]
 macro_rules! impl_vmstate_bitsized {
@@ -390,7 +386,7 @@ unsafe impl VMState for $type {
 impl_vmstate_scalar!(vmstate_info_uint16, u16, VMS_VARRAY_UINT16);
 impl_vmstate_scalar!(vmstate_info_uint32, u32, VMS_VARRAY_UINT32);
 impl_vmstate_scalar!(vmstate_info_uint64, u64);
-impl_vmstate_scalar!(vmstate_info_timer, bindings::QEMUTimer);
+impl_vmstate_scalar!(vmstate_info_timer, crate::timer::Timer);
 
 // Pointer types using the underlying type's VMState plus VMS_POINTER
 // Note that references are not supported, though references to cells
-- 
2.34.1

[PATCH 12/17] rust/vmstate: Support version field in vmstate macros

[Zhao Liu]

Add "version = *" in vmstate macros to help set version_id in
VMStateField.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 3d4c50ca86f9..bb41bfd291c0 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -197,7 +197,7 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
 /// and [`impl_vmstate_forward!`](crate::impl_vmstate_forward) help with this.
 #[macro_export]
 macro_rules! vmstate_of {
-    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
+    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])? $(, version = $version:expr)? $(,)?) => {
         $crate::bindings::VMStateField {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
@@ -211,6 +211,7 @@ macro_rules! vmstate_of {
                 $struct_name,
                 $field_name
             )),
+            $(version_id: $version,)?
             ..$crate::call_func_with_field!(
                 $crate::vmstate::vmstate_base,
                 $struct_name,
@@ -442,7 +443,7 @@ macro_rules! vmstate_unused {
 #[doc(alias = "VMSTATE_STRUCT")]
 #[macro_export]
 macro_rules! vmstate_struct {
-    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])?, $vmsd:ident, $type:ty $(,)?) => {
+    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $( * $factor:expr)? ])?, $vmsd:ident, $type:ty $(, version = $version:expr)? $(,)?) => {
         $crate::bindings::VMStateField {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
@@ -455,6 +456,7 @@ macro_rules! vmstate_struct {
             size: ::core::mem::size_of::<$type>(),
             flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
             vmsd: &$vmsd,
+            $(version_id: $version,)?
             ..$crate::zeroable::Zeroable::ZERO
          } $(.with_varray_flag_unchecked(
                   $crate::call_func_with_field!(
-- 
2.34.1

[PATCH 13/17] rust/vmstate: Support vmstate_validate

[Zhao Liu]

In C version, VMSTATE_VALIDATE accepts the function pointer, which is
used to check if some conditions of structure could meet, although the
C version macro doesn't accept any structure as the opaque type.

But it's hard to integrate VMSTATE_VALIDAE into vmstate_struct, a new
macro has to be introduced to specifically handle the case corresponding
to VMSTATE_VALIDATE.

One of the difficulties is inferring the type of a callback by its name
`test_fn`. We can't directly use `test_fn` as a parameter of
test_cb_builder__() to get its type "F", because in this way, Rust
compiler will be too conservative on drop check and complain "the
destructor for this type cannot be evaluated in constant functions".

Fortunately, PhantomData<T> could help in this case, because it is
considered to never have a destructor, no matter its field type [*].

The `phantom__()` in the `call_func_with_field` macro provides a good
example of using PhantomData to infer type. So copy this idea and apply
it to the `vmstate_validate` macro.

[*]: https://doc.rust-lang.org/std/ops/trait.Drop.html#drop-check

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/vmstate.rs | 57 +++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index bb41bfd291c0..4eefd54ca120 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -25,9 +25,12 @@
 //!   functionality that is missing from `vmstate_of!`.
 
 use core::{marker::PhantomData, mem, ptr::NonNull};
+use std::os::raw::{c_int, c_void};
 
 pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{bindings::VMStateFlags, prelude::*, qom::Owned, zeroable::Zeroable};
+use crate::{
+    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
+};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
@@ -292,6 +295,14 @@ pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
         self.num = num as i32;
         self
     }
+
+    #[must_use]
+    pub const fn with_exist_check(mut self) -> Self {
+        assert!(self.flags.0 == 0);
+        self.flags = VMStateFlags(VMStateFlags::VMS_MUST_EXIST.0 | VMStateFlags::VMS_ARRAY.0);
+        self.num = 0; // 0 elements: no data, only run test_fn callback
+        self
+    }
 }
 
 /// This macro can be used (by just passing it a type) to forward the `VMState`
@@ -510,6 +521,50 @@ macro_rules! vmstate_fields {
     }}
 }
 
+pub extern "C" fn rust_vms_test_field_exists<T, F: for<'a> FnCall<(&'a T, u8), bool>>(
+    opaque: *mut c_void,
+    version_id: c_int,
+) -> bool {
+    let owner: &T = unsafe { &*(opaque.cast::<T>()) };
+    let version: u8 = version_id.try_into().unwrap();
+    // SAFETY: the opaque was passed as a reference to `T`.
+    F::call((owner, version))
+}
+
+pub type VMSFieldExistCb = unsafe extern "C" fn(
+    opaque: *mut std::os::raw::c_void,
+    version_id: std::os::raw::c_int,
+) -> bool;
+
+#[doc(alias = "VMSTATE_VALIDATE")]
+#[macro_export]
+macro_rules! vmstate_validate {
+    ($struct_name:ty, $test_name:expr, $test_fn:expr $(,)?) => {
+        $crate::bindings::VMStateField {
+            name: ::std::ffi::CStr::as_ptr($test_name),
+            // TODO: Use safe callback.
+            field_exists: {
+                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;
+                    $crate::vmstate::rust_vms_test_field_exists::<T, F>
+                }
+
+                const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
+                    ::core::marker::PhantomData
+                }
+                Some(test_cb_builder__::<$struct_name, _>(phantom__(&$test_fn)))
+            },
+            ..$crate::zeroable::Zeroable::ZERO
+        }
+        .with_exist_check()
+    };
+}
+
 /// A transparent wrapper type for the `subsections` field of
 /// [`VMStateDescription`].
 ///
-- 
2.34.1

[PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro

[Zhao Liu]

The vmstate has too many combinations of VMStateFlags and VMStateField.
Currently, the best way to test is to ensure that the Rust vmstate
definition is consistent with the (possibly corresponding) C version.

Add a unit test to cover some patterns accepted by vmstate_of macro,
which correspond to the following C version macros:
 * VMSTATE_U16
 * VMSTATE_UNUSED
 * VMSTATE_VARRAY_UINT16_UNSAFE
 * VMSTATE_VARRAY_MULTIPLY

Note: Because vmstate_info_* are defined in vmstate-types.c, it's
necessary to link libmigration to rust unit tests. In the future,
maybe it's possible to spilt libmigration from rust_qemu_api_objs.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/meson.build            |   5 +-
 rust/qemu-api/src/vmstate.rs         |   6 +-
 rust/qemu-api/tests/tests.rs         |   2 +
 rust/qemu-api/tests/vmstate_tests.rs | 127 +++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 6 deletions(-)
 create mode 100644 rust/qemu-api/tests/vmstate_tests.rs

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index a3f226ccc2a5..858685ddd4a4 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -58,7 +58,8 @@ rust_qemu_api_objs = static_library(
               libchardev.extract_all_objects(recursive: false),
               libcrypto.extract_all_objects(recursive: false),
               libauthz.extract_all_objects(recursive: false),
-              libio.extract_all_objects(recursive: false)])
+              libio.extract_all_objects(recursive: false),
+              libmigration.extract_all_objects(recursive: false)])
 rust_qemu_api_deps = declare_dependency(
     dependencies: [
       qom_ss.dependencies(),
@@ -71,7 +72,7 @@ rust_qemu_api_deps = declare_dependency(
 test('rust-qemu-api-integration',
     executable(
         'rust-qemu-api-integration',
-        'tests/tests.rs',
+        files('tests/tests.rs', 'tests/vmstate_tests.rs'),
         override_options: ['rust_std=2021', 'build.rust_std=2021'],
         rust_args: ['--test'],
         install: false,
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 4eefd54ca120..3fdd5948f6d7 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -27,10 +27,8 @@
 use core::{marker::PhantomData, mem, ptr::NonNull};
 use std::os::raw::{c_int, c_void};
 
-pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{
-    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
-};
+pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};
+use crate::{callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 269122e7ec19..99a7aab6fed9 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -17,6 +17,8 @@
     zeroable::Zeroable,
 };
 
+mod vmstate_tests;
+
 // Test that macros can compile.
 pub static VMSTATE: VMStateDescription = VMStateDescription {
     name: c_str!("name").as_ptr(),
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
new file mode 100644
index 000000000000..25b28b298066
--- /dev/null
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -0,0 +1,127 @@
+// Copyright (C) 2025 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::{ffi::CStr, mem::size_of, slice};
+
+use qemu_api::{
+    bindings::{vmstate_info_int8, vmstate_info_uint8, vmstate_info_unused_buffer},
+    c_str,
+    vmstate::{VMStateDescription, VMStateField, VMStateFlags},
+    vmstate_fields, vmstate_of, vmstate_unused,
+    zeroable::Zeroable,
+};
+
+const FOO_ARRAY_MAX: usize = 3;
+
+// =========================== Test VMSTATE_FOOA ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOA:
+//     - VMSTATE_U16
+//     - VMSTATE_UNUSED
+//     - VMSTATE_VARRAY_UINT16_UNSAFE
+//     - VMSTATE_VARRAY_MULTIPLY
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooA {
+    arr: [u8; FOO_ARRAY_MAX],
+    num: u16,
+    arr_mul: [i8; FOO_ARRAY_MAX],
+    num_mul: u32,
+    elem: i8,
+}
+
+static VMSTATE_FOOA: VMStateDescription = VMStateDescription {
+    name: c_str!("foo_a").as_ptr(),
+    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], version = 0),
+        vmstate_of!(FooA, arr_mul, [0 .. num_mul * 16]),
+    },
+    ..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_fooa() {
+    let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+
+    // 1st VMStateField ("elem") in VMSTATE_FOOA (corresponding to VMSTATE_U16)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+        b"elem\0"
+    );
+    assert_eq!(foo_fields[0].offset, 16);
+    assert_eq!(foo_fields[0].num_offset, 0);
+    assert_eq!(foo_fields[0].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_int8)
+    });
+    assert_eq!(foo_fields[0].version_id, 0);
+    assert_eq!(foo_fields[0].size, 1);
+    assert_eq!(foo_fields[0].num, 0);
+    assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
+    assert_eq!(foo_fields[0].vmsd.is_null(), true);
+    assert_eq!(foo_fields[0].field_exists.is_none(), true);
+
+    // 2nd VMStateField ("unused") in VMSTATE_FOOA (corresponding to VMSTATE_UNUSED)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+        b"unused\0"
+    );
+    assert_eq!(foo_fields[1].offset, 0);
+    assert_eq!(foo_fields[1].num_offset, 0);
+    assert_eq!(foo_fields[1].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_unused_buffer)
+    });
+    assert_eq!(foo_fields[1].version_id, 0);
+    assert_eq!(foo_fields[1].size, 8);
+    assert_eq!(foo_fields[1].num, 0);
+    assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_BUFFER);
+    assert_eq!(foo_fields[1].vmsd.is_null(), true);
+    assert_eq!(foo_fields[1].field_exists.is_none(), true);
+
+    // 3rd VMStateField ("arr") in VMSTATE_FOOA (corresponding to
+    // VMSTATE_VARRAY_UINT16_UNSAFE)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+        b"arr\0"
+    );
+    assert_eq!(foo_fields[2].offset, 0);
+    assert_eq!(foo_fields[2].num_offset, 4);
+    assert_eq!(foo_fields[2].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_uint8)
+    });
+    assert_eq!(foo_fields[2].version_id, 0);
+    assert_eq!(foo_fields[2].size, 1);
+    assert_eq!(foo_fields[2].num, 0);
+    assert_eq!(foo_fields[2].flags, VMStateFlags::VMS_VARRAY_UINT16);
+    assert_eq!(foo_fields[2].vmsd.is_null(), true);
+    assert_eq!(foo_fields[2].field_exists.is_none(), true);
+
+    // 4th VMStateField ("arr_mul") in VMSTATE_FOOA (corresponding to
+    // VMSTATE_VARRAY_MULTIPLY)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
+        b"arr_mul\0"
+    );
+    assert_eq!(foo_fields[3].offset, 6);
+    assert_eq!(foo_fields[3].num_offset, 12);
+    assert_eq!(foo_fields[3].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_int8)
+    });
+    assert_eq!(foo_fields[3].version_id, 0);
+    assert_eq!(foo_fields[3].size, 1);
+    assert_eq!(foo_fields[3].num, 16);
+    assert_eq!(
+        foo_fields[3].flags.0,
+        VMStateFlags::VMS_VARRAY_UINT32.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
+    );
+    assert_eq!(foo_fields[3].vmsd.is_null(), true);
+    assert_eq!(foo_fields[3].field_exists.is_none(), true);
+
+    // The last VMStateField in VMSTATE_FOOA.
+    assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
+}
-- 
2.34.1

[PATCH 15/17] rust/vmstate: Add unit test for vmstate_{of|struct} macro

[Zhao Liu]

Add a unit test to cover some patterns accepted by vmstate_of and
vmstate_struct macros, which correspond to the following C version
macros:

 * VMSTATE_BOOL_V
 * VMSTATE_U64
 * VMSTATE_STRUCT_VARRAY_UINT8
 * (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32
 * VMSTATE_ARRAY

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/tests/vmstate_tests.rs | 144 ++++++++++++++++++++++++++-
 1 file changed, 142 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index 25b28b298066..27df66b5766e 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -5,10 +5,14 @@
 use std::{ffi::CStr, mem::size_of, slice};
 
 use qemu_api::{
-    bindings::{vmstate_info_int8, vmstate_info_uint8, vmstate_info_unused_buffer},
+    bindings::{
+        vmstate_info_bool, vmstate_info_int64, vmstate_info_int8, vmstate_info_uint64,
+        vmstate_info_uint8, vmstate_info_unused_buffer,
+    },
     c_str,
+    cell::BqlCell,
     vmstate::{VMStateDescription, VMStateField, VMStateFlags},
-    vmstate_fields, vmstate_of, vmstate_unused,
+    vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
     zeroable::Zeroable,
 };
 
@@ -125,3 +129,139 @@ fn test_vmstate_macro_fooa() {
     // The last VMStateField in VMSTATE_FOOA.
     assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
 }
+
+// =========================== Test VMSTATE_FOOB ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOB:
+//     - VMSTATE_BOOL_V
+//     - VMSTATE_U64
+//     - VMSTATE_STRUCT_VARRAY_UINT8
+//     - (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32
+//     - VMSTATE_ARRAY
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooB {
+    arr_a: [FooA; FOO_ARRAY_MAX],
+    num_a: u8,
+    arr_a_mul: [FooA; FOO_ARRAY_MAX],
+    num_a_mul: u32,
+    wrap: BqlCell<u64>,
+    val: bool,
+    // FIXME: Use Timer array. Now we can't since it's hard to link savevm.c to test.
+    arr_i64: [i64; FOO_ARRAY_MAX],
+}
+
+static VMSTATE_FOOB: VMStateDescription = VMStateDescription {
+    name: c_str!("foo_b").as_ptr(),
+    version_id: 2,
+    minimum_version_id: 1,
+    fields: vmstate_fields! {
+        vmstate_of!(FooB, val, version = 2),
+        vmstate_of!(FooB, wrap),
+        vmstate_struct!(FooB, arr_a, [0 .. num_a], VMSTATE_FOOA, FooA, version = 1),
+        vmstate_struct!(FooB, arr_a_mul, [0 .. num_a_mul * 32], VMSTATE_FOOA, FooA, version = 2),
+        vmstate_of!(FooB, arr_i64),
+    },
+    ..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_foob() {
+    let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 6) };
+
+    // 1st VMStateField ("val") in VMSTATE_FOOB (corresponding to VMSTATE_BOOL_V)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+        b"val\0"
+    );
+    assert_eq!(foo_fields[0].offset, 136);
+    assert_eq!(foo_fields[0].num_offset, 0);
+    assert_eq!(foo_fields[0].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_bool)
+    });
+    assert_eq!(foo_fields[0].version_id, 2);
+    assert_eq!(foo_fields[0].size, 1);
+    assert_eq!(foo_fields[0].num, 0);
+    assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
+    assert_eq!(foo_fields[0].vmsd.is_null(), true);
+    assert_eq!(foo_fields[0].field_exists.is_none(), true);
+
+    // 2nd VMStateField ("wrap") in VMSTATE_FOOB (corresponding to VMSTATE_U64)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+        b"wrap\0"
+    );
+    assert_eq!(foo_fields[1].offset, 128);
+    assert_eq!(foo_fields[1].num_offset, 0);
+    assert_eq!(foo_fields[1].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_uint64)
+    });
+    assert_eq!(foo_fields[1].version_id, 0);
+    assert_eq!(foo_fields[1].size, 8);
+    assert_eq!(foo_fields[1].num, 0);
+    assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_SINGLE);
+    assert_eq!(foo_fields[1].vmsd.is_null(), true);
+    assert_eq!(foo_fields[1].field_exists.is_none(), true);
+
+    // 3rd VMStateField ("arr_a") in VMSTATE_FOOB (corresponding to
+    // VMSTATE_STRUCT_VARRAY_UINT8)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+        b"arr_a\0"
+    );
+    assert_eq!(foo_fields[2].offset, 0);
+    assert_eq!(foo_fields[2].num_offset, 60);
+    assert_eq!(foo_fields[2].info.is_null(), true); // VMSTATE_STRUCT_VARRAY_UINT8 doesn't set info field.
+    assert_eq!(foo_fields[2].version_id, 1);
+    assert_eq!(foo_fields[2].size, 20);
+    assert_eq!(foo_fields[2].num, 0);
+    assert_eq!(
+        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].field_exists.is_none(), true);
+
+    // 4th VMStateField ("arr_a_mul") in VMSTATE_FOOB (corresponding to
+    // (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
+        b"arr_a_mul\0"
+    );
+    assert_eq!(foo_fields[3].offset, 64);
+    assert_eq!(foo_fields[3].num_offset, 124);
+    assert_eq!(foo_fields[3].info.is_null(), true); // VMSTATE_STRUCT_VARRAY_UINT8 doesn't set info field.
+    assert_eq!(foo_fields[3].version_id, 2);
+    assert_eq!(foo_fields[3].size, 20);
+    assert_eq!(foo_fields[3].num, 32);
+    assert_eq!(
+        foo_fields[3].flags.0,
+        VMStateFlags::VMS_STRUCT.0
+            | VMStateFlags::VMS_VARRAY_UINT32.0
+            | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
+    );
+    assert_eq!(foo_fields[3].vmsd, &VMSTATE_FOOA);
+    assert_eq!(foo_fields[3].field_exists.is_none(), true);
+
+    // 5th VMStateField ("arr_i64") in VMSTATE_FOOB (corresponding to
+    // VMSTATE_ARRAY)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[4].name) }.to_bytes_with_nul(),
+        b"arr_i64\0"
+    );
+    assert_eq!(foo_fields[4].offset, 144);
+    assert_eq!(foo_fields[4].num_offset, 0);
+    assert_eq!(foo_fields[4].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_int64)
+    });
+    assert_eq!(foo_fields[4].version_id, 0);
+    assert_eq!(foo_fields[4].size, 8);
+    assert_eq!(foo_fields[4].num, FOO_ARRAY_MAX as i32);
+    assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_ARRAY);
+    assert_eq!(foo_fields[4].vmsd.is_null(), true);
+    assert_eq!(foo_fields[4].field_exists.is_none(), true);
+
+    // The last VMStateField in VMSTATE_FOOB.
+    assert_eq!(foo_fields[5].flags, VMStateFlags::VMS_END);
+}
-- 
2.34.1

[PATCH 16/17] rust/vmstate: Add unit test for pointer case

[Zhao Liu]

Add a unit test to cover some patterns accepted by vmstate_of macro,
which correspond to the following C version macros:
 * VMSTATE_POINTER
 * VMSTATE_ARRAY_OF_POINTER

Note: Currently, vmstate_struct can't handle the pointer to structure
case. Leave this case as a FIXME and use vmstate_unused as a place
holder.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/tests/vmstate_tests.rs | 121 ++++++++++++++++++++++++++-
 1 file changed, 117 insertions(+), 4 deletions(-)

diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index 27df66b5766e..a8bc00a56494 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -2,15 +2,16 @@
 // Author(s): Zhao Liu <zhai1.liu@intel.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::{ffi::CStr, mem::size_of, slice};
+use std::{ffi::CStr, mem::size_of, ptr::NonNull, slice};
 
 use qemu_api::{
     bindings::{
-        vmstate_info_bool, vmstate_info_int64, vmstate_info_int8, vmstate_info_uint64,
-        vmstate_info_uint8, vmstate_info_unused_buffer,
+        vmstate_info_bool, vmstate_info_int32, vmstate_info_int64, vmstate_info_int8,
+        vmstate_info_uint64, vmstate_info_uint8, vmstate_info_unused_buffer,
     },
     c_str,
-    cell::BqlCell,
+    cell::{BqlCell, Opaque},
+    impl_vmstate_forward,
     vmstate::{VMStateDescription, VMStateField, VMStateFlags},
     vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
     zeroable::Zeroable,
@@ -265,3 +266,115 @@ fn test_vmstate_macro_foob() {
     // The last VMStateField in VMSTATE_FOOB.
     assert_eq!(foo_fields[5].flags, VMStateFlags::VMS_END);
 }
+
+// =========================== Test VMSTATE_FOOC ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOC:
+//     - VMSTATE_POINTER
+//     - VMSTATE_ARRAY_OF_POINTER
+struct FooCWrapper([Opaque<*mut u8>; FOO_ARRAY_MAX]); // Though Opaque<> array is almost impossible.
+
+impl_vmstate_forward!(FooCWrapper);
+
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooC {
+    ptr: *const i32,
+    ptr_a: NonNull<FooA>,
+    arr_ptr: [Box<u8>; FOO_ARRAY_MAX],
+    arr_ptr_wrap: FooCWrapper,
+}
+
+static VMSTATE_FOOC: VMStateDescription = VMStateDescription {
+    name: c_str!("foo_c").as_ptr(),
+    version_id: 3,
+    minimum_version_id: 1,
+    fields: vmstate_fields! {
+        vmstate_of!(FooC, ptr, version = 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
+};
+
+#[test]
+fn test_vmstate_macro_fooc() {
+    let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+
+    // 1st VMStateField ("ptr") in VMSTATE_FOOC (corresponding to VMSTATE_POINTER)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+        b"ptr\0"
+    );
+    assert_eq!(foo_fields[0].offset, 0);
+    assert_eq!(foo_fields[0].num_offset, 0);
+    assert_eq!(foo_fields[0].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_int32)
+    });
+    assert_eq!(foo_fields[0].version_id, 2);
+    assert_eq!(foo_fields[0].size, 4);
+    assert_eq!(foo_fields[0].num, 0);
+    assert_eq!(
+        foo_fields[0].flags.0,
+        VMStateFlags::VMS_SINGLE.0 | VMStateFlags::VMS_POINTER.0
+    );
+    assert_eq!(foo_fields[0].vmsd.is_null(), true);
+    assert_eq!(foo_fields[0].field_exists.is_none(), true);
+
+    // 2nd VMStateField ("unused") in VMSTATE_FOOC
+    let mut offset = size_of::<*const i32>();
+    let size_ptr_a = size_of::<NonNull<FooA>>();
+
+    // 3rd VMStateField ("arr_ptr") in VMSTATE_FOOC (corresponding to
+    // VMSTATE_ARRAY_OF_POINTER)
+    offset = offset + size_ptr_a;
+    let size_arr_ptr = size_of::<Box<[FooB; FOO_ARRAY_MAX]>>();
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+        b"arr_ptr\0"
+    );
+    assert_eq!(foo_fields[2].offset, offset);
+    assert_eq!(foo_fields[2].num_offset, 0);
+    assert_eq!(foo_fields[2].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_uint8)
+    });
+    assert_eq!(foo_fields[2].version_id, 0);
+    assert_eq!(foo_fields[2].size, size_arr_ptr);
+    assert_eq!(foo_fields[2].num, FOO_ARRAY_MAX as i32);
+    assert_eq!(
+        foo_fields[2].flags.0,
+        VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0
+    );
+    assert_eq!(foo_fields[2].vmsd.is_null(), true);
+    assert_eq!(foo_fields[2].field_exists.is_none(), true);
+
+    // 4th VMStateField ("arr_ptr_wrap") in VMSTATE_FOOC (corresponding to
+    // VMSTATE_ARRAY_OF_POINTER)
+    offset = offset + size_of::<Box<u8>>() * FOO_ARRAY_MAX;
+    let size_arr_ptr_wrap = size_of::<Opaque<*mut u8>>();
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
+        b"arr_ptr_wrap\0"
+    );
+    assert_eq!(foo_fields[3].offset, offset);
+    assert_eq!(foo_fields[3].num_offset, 0);
+    assert_eq!(foo_fields[2].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_uint8)
+    });
+    assert_eq!(foo_fields[3].version_id, 0);
+    assert_eq!(foo_fields[3].size, size_arr_ptr_wrap);
+    assert_eq!(foo_fields[3].num, FOO_ARRAY_MAX as i32);
+    assert_eq!(
+        foo_fields[2].flags.0,
+        VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0
+    );
+    assert_eq!(foo_fields[3].vmsd.is_null(), true);
+    assert_eq!(foo_fields[3].field_exists.is_none(), true);
+
+    // The last VMStateField in VMSTATE_FOOC.
+    assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
+}
-- 
2.34.1

[PATCH 17/17] rust/vmstate: Add unit test for vmstate_validate

[Zhao Liu]

Add a unit test for vmstate_validate, which corresponds to the C version
macro: VMSTATE_VALIDATE.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/tests/vmstate_tests.rs | 91 +++++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index a8bc00a56494..bb615fd7bdee 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -2,7 +2,7 @@
 // Author(s): Zhao Liu <zhai1.liu@intel.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::{ffi::CStr, mem::size_of, ptr::NonNull, slice};
+use std::{ffi::CStr, mem::size_of, os::raw::c_void, ptr::NonNull, slice};
 
 use qemu_api::{
     bindings::{
@@ -13,7 +13,7 @@
     cell::{BqlCell, Opaque},
     impl_vmstate_forward,
     vmstate::{VMStateDescription, VMStateField, VMStateFlags},
-    vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
+    vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused, vmstate_validate,
     zeroable::Zeroable,
 };
 
@@ -378,3 +378,90 @@ fn test_vmstate_macro_fooc() {
     // The last VMStateField in VMSTATE_FOOC.
     assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
 }
+
+// =========================== Test VMSTATE_FOOD ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOD:
+//     - VMSTATE_VALIDATE
+
+// Add more member fields when vmstate_of/vmstate_struct support "test"
+// parameter.
+struct FooD;
+
+impl FooD {
+    fn validate_food_0(&self, _version_id: u8) -> bool {
+        true
+    }
+
+    fn validate_food_1(_state: &FooD, _version_id: u8) -> bool {
+        false
+    }
+}
+
+fn validate_food_2(_state: &FooD, _version_id: u8) -> bool {
+    true
+}
+
+static VMSTATE_FOOD: VMStateDescription = VMStateDescription {
+    name: c_str!("foo_d").as_ptr(),
+    version_id: 3,
+    minimum_version_id: 1,
+    fields: vmstate_fields! {
+        vmstate_validate!(FooD, c_str!("foo_d_0"), FooD::validate_food_0),
+        vmstate_validate!(FooD, c_str!("foo_d_1"), FooD::validate_food_1),
+        vmstate_validate!(FooD, c_str!("foo_d_2"), validate_food_2),
+    },
+    ..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_food() {
+    let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOD.fields, 4) };
+    let mut foo_d = FooD;
+    let foo_d_p = &mut foo_d as *mut _ as *mut c_void;
+
+    // 1st VMStateField in VMSTATE_FOOD
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+        b"foo_d_0\0"
+    );
+    assert_eq!(foo_fields[0].offset, 0);
+    assert_eq!(foo_fields[0].num_offset, 0);
+    assert_eq!(foo_fields[0].info.is_null(), true);
+    assert_eq!(foo_fields[0].version_id, 0);
+    assert_eq!(foo_fields[0].size, 0);
+    assert_eq!(foo_fields[0].num, 0);
+    assert_eq!(
+        foo_fields[0].flags.0,
+        VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_MUST_EXIST.0
+    );
+    assert_eq!(foo_fields[0].vmsd.is_null(), true);
+    assert_eq!(
+        unsafe { foo_fields[0].field_exists.unwrap()(foo_d_p, 0) },
+        true
+    );
+
+    // 2nd VMStateField in VMSTATE_FOOD
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+        b"foo_d_1\0"
+    );
+    assert_eq!(
+        unsafe { foo_fields[1].field_exists.unwrap()(foo_d_p, 1) },
+        false
+    );
+
+    // 3rd VMStateField in VMSTATE_FOOD
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+        b"foo_d_2\0"
+    );
+    assert_eq!(
+        unsafe { foo_fields[2].field_exists.unwrap()(foo_d_p, 2) },
+        true
+    );
+
+    // The last VMStateField in VMSTATE_FOOD.
+    assert_eq!(foo_fields[3].flags, VMStateFlags::VMS_END);
+}
-- 
2.34.1

Re: [PATCH 03/17] rust/vmstate: Add a prefix separator ", " for the array field in vmstate macros

[Paolo Bonzini]

On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> The fields are separated by ",", so it's necessary to add ", " in array
> field to avoid matching failure.

This is not a field though, the only (intended) fields are name and
field. It's meant to mimic the slice since &a[0..n].

Paolo

> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  rust/qemu-api/src/vmstate.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index 9533b1250fa5..94efbd8bb735 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -202,7 +202,7 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
>  /// and [`impl_vmstate_forward!`](crate::impl_vmstate_forward) help with this.
>  #[macro_export]
>  macro_rules! vmstate_of {
> -    ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
> +    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
>          $crate::bindings::VMStateField {
>              name: ::core::concat!(::core::stringify!($field_name), "\0")
>                  .as_bytes()
> @@ -435,7 +435,7 @@ macro_rules! vmstate_unused {
>  #[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 $(,)?) => {
> +    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])?, $vmsd:expr, $type:ty $(,)?) => {
>          $crate::bindings::VMStateField {
>              name: ::core::concat!(::core::stringify!($field_name), "\0")
>                  .as_bytes()
> --
> 2.34.1
>

Re: [PATCH 12/17] rust/vmstate: Support version field in vmstate macros

[Paolo Bonzini]

On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> Add "version = *" in vmstate macros to help set version_id in
> VMStateField.

Could it use a ".with_min_version(2)" annotation (or something similar) instead?

Paolo

> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  rust/qemu-api/src/vmstate.rs | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index 3d4c50ca86f9..bb41bfd291c0 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -197,7 +197,7 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
>  /// and [`impl_vmstate_forward!`](crate::impl_vmstate_forward) help with this.
>  #[macro_export]
>  macro_rules! vmstate_of {
> -    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
> +    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])? $(, version = $version:expr)? $(,)?) => {
>          $crate::bindings::VMStateField {
>              name: ::core::concat!(::core::stringify!($field_name), "\0")
>                  .as_bytes()
> @@ -211,6 +211,7 @@ macro_rules! vmstate_of {
>                  $struct_name,
>                  $field_name
>              )),
> +            $(version_id: $version,)?
>              ..$crate::call_func_with_field!(
>                  $crate::vmstate::vmstate_base,
>                  $struct_name,
> @@ -442,7 +443,7 @@ macro_rules! vmstate_unused {
>  #[doc(alias = "VMSTATE_STRUCT")]
>  #[macro_export]
>  macro_rules! vmstate_struct {
> -    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])?, $vmsd:ident, $type:ty $(,)?) => {
> +    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $( * $factor:expr)? ])?, $vmsd:ident, $type:ty $(, version = $version:expr)? $(,)?) => {
>          $crate::bindings::VMStateField {
>              name: ::core::concat!(::core::stringify!($field_name), "\0")
>                  .as_bytes()
> @@ -455,6 +456,7 @@ macro_rules! vmstate_struct {
>              size: ::core::mem::size_of::<$type>(),
>              flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
>              vmsd: &$vmsd,
> +            $(version_id: $version,)?
>              ..$crate::zeroable::Zeroable::ZERO
>           } $(.with_varray_flag_unchecked(
>                    $crate::call_func_with_field!(
> --
> 2.34.1
>

Re: [PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro

[Paolo Bonzini]

Thanks very much for the tests!

On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> -pub use crate::bindings::{VMStateDescription, VMStateField};
> -use crate::{
> -    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
> -};
> +pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};

Does VMStateFlags have to be part of the public API?

> +    assert_eq!(foo_fields[0].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_int8)
> +    });

You can use & instead of addr_of here.

> +    assert_eq!(foo_fields[0].version_id, 0);
> +    assert_eq!(foo_fields[0].size, 1);
> +    assert_eq!(foo_fields[0].num, 0);
> +    assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
> +    assert_eq!(foo_fields[0].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[0].field_exists.is_none(), true);
> +
> +    // 2nd VMStateField ("unused") in VMSTATE_FOOA (corresponding to VMSTATE_UNUSED)
> +    assert_eq!(
> +        unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
> +        b"unused\0"
> +    );
> +    assert_eq!(foo_fields[1].offset, 0);
> +    assert_eq!(foo_fields[1].num_offset, 0);
> +    assert_eq!(foo_fields[1].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_unused_buffer)
> +    });
> +    assert_eq!(foo_fields[1].version_id, 0);
> +    assert_eq!(foo_fields[1].size, 8);
> +    assert_eq!(foo_fields[1].num, 0);
> +    assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_BUFFER);
> +    assert_eq!(foo_fields[1].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[1].field_exists.is_none(), true);
> +
> +    // 3rd VMStateField ("arr") in VMSTATE_FOOA (corresponding to
> +    // VMSTATE_VARRAY_UINT16_UNSAFE)
> +    assert_eq!(
> +        unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
> +        b"arr\0"
> +    );
> +    assert_eq!(foo_fields[2].offset, 0);
> +    assert_eq!(foo_fields[2].num_offset, 4);
> +    assert_eq!(foo_fields[2].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_uint8)
> +    });
> +    assert_eq!(foo_fields[2].version_id, 0);
> +    assert_eq!(foo_fields[2].size, 1);
> +    assert_eq!(foo_fields[2].num, 0);
> +    assert_eq!(foo_fields[2].flags, VMStateFlags::VMS_VARRAY_UINT16);
> +    assert_eq!(foo_fields[2].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[2].field_exists.is_none(), true);
> +
> +    // 4th VMStateField ("arr_mul") in VMSTATE_FOOA (corresponding to
> +    // VMSTATE_VARRAY_MULTIPLY)
> +    assert_eq!(
> +        unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
> +        b"arr_mul\0"
> +    );
> +    assert_eq!(foo_fields[3].offset, 6);
> +    assert_eq!(foo_fields[3].num_offset, 12);
> +    assert_eq!(foo_fields[3].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_int8)
> +    });
> +    assert_eq!(foo_fields[3].version_id, 0);
> +    assert_eq!(foo_fields[3].size, 1);
> +    assert_eq!(foo_fields[3].num, 16);
> +    assert_eq!(
> +        foo_fields[3].flags.0,
> +        VMStateFlags::VMS_VARRAY_UINT32.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
> +    );
> +    assert_eq!(foo_fields[3].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[3].field_exists.is_none(), true);
> +
> +    // The last VMStateField in VMSTATE_FOOA.
> +    assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
> +}
> --
> 2.34.1
>

Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro

[Paolo Bonzini]

On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> When specify an array field in vmstate_struct macro, there will be an
> error:
>
> > local ambiguity when calling macro `vmstate_struct`: multiple parsing
> > options: built-in NTs expr ('vmsd') or 1 other option.
>
> This is because "expr" can't recognize the "vmsd" field correctly, so
> that it gets confused with the previous array field.
>
> To fix the above issue, use "ident" for "vmsd" field, and explicitly
> refer to it in the macro.

I think this is not needed if the varray case is left as is, and other
cases use .with_...() instead of arguments?

Paolo

> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  rust/hw/char/pl011/src/device_class.rs | 2 +-
>  rust/qemu-api/src/vmstate.rs           | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
> index 0b2076ddaa0f..e43a5d6cd063 100644
> --- a/rust/hw/char/pl011/src/device_class.rs
> +++ b/rust/hw/char/pl011/src/device_class.rs
> @@ -72,7 +72,7 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
>      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>),
> +        vmstate_struct!(PL011State, regs, VMSTATE_PL011_REGS, BqlRefCell<PL011Registers>),
>      },
>      subsections: vmstate_subsections! {
>          VMSTATE_PL011_CLOCK
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index 94efbd8bb735..3f95d4825149 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -435,7 +435,7 @@ macro_rules! vmstate_unused {
>  #[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 $(,)?) => {
> +    ($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* $factor:expr)?])?, $vmsd:ident, $type:ty $(,)?) => {
>          $crate::bindings::VMStateField {
>              name: ::core::concat!(::core::stringify!($field_name), "\0")
>                  .as_bytes()
> @@ -447,7 +447,7 @@ macro_rules! vmstate_struct {
>              },
>              size: ::core::mem::size_of::<$type>(),
>              flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
> -            vmsd: $vmsd,
> +            vmsd: &$vmsd,
>              ..$crate::zeroable::Zeroable::ZERO $(
>                  .with_varray_flag($crate::call_func_with_field!(
>                      $crate::vmstate::vmstate_varray_flag,
> --
> 2.34.1
>

Re: [PATCH 13/17] rust/vmstate: Support vmstate_validate

[Paolo Bonzini]

On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> In C version, VMSTATE_VALIDATE accepts the function pointer, which is
> used to check if some conditions of structure could meet, although the
> C version macro doesn't accept any structure as the opaque type.
>
> But it's hard to integrate VMSTATE_VALIDAE into vmstate_struct, a new
> macro has to be introduced to specifically handle the case corresponding
> to VMSTATE_VALIDATE.
>
> One of the difficulties is inferring the type of a callback by its name
> `test_fn`. We can't directly use `test_fn` as a parameter of
> test_cb_builder__() to get its type "F", because in this way, Rust
> compiler will be too conservative on drop check and complain "the
> destructor for this type cannot be evaluated in constant functions".
>
> Fortunately, PhantomData<T> could help in this case, because it is
> considered to never have a destructor, no matter its field type [*].
>
> The `phantom__()` in the `call_func_with_field` macro provides a good
> example of using PhantomData to infer type. So copy this idea and apply
> it to the `vmstate_validate` macro.
>
> [*]: https://doc.rust-lang.org/std/ops/trait.Drop.html#drop-check
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  rust/qemu-api/src/vmstate.rs | 57 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index bb41bfd291c0..4eefd54ca120 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -25,9 +25,12 @@
>  //!   functionality that is missing from `vmstate_of!`.
>
>  use core::{marker::PhantomData, mem, ptr::NonNull};
> +use std::os::raw::{c_int, c_void};
>
>  pub use crate::bindings::{VMStateDescription, VMStateField};
> -use crate::{bindings::VMStateFlags, prelude::*, qom::Owned, zeroable::Zeroable};
> +use crate::{
> +    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
> +};
>
>  /// This macro is used to call a function with a generic argument bound
>  /// to the type of a field.  The function must take a
> @@ -292,6 +295,14 @@ pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
>          self.num = num as i32;
>          self
>      }
> +
> +    #[must_use]
> +    pub const fn with_exist_check(mut self) -> Self {
> +        assert!(self.flags.0 == 0);
> +        self.flags = VMStateFlags(VMStateFlags::VMS_MUST_EXIST.0 | VMStateFlags::VMS_ARRAY.0);
> +        self.num = 0; // 0 elements: no data, only run test_fn callback
> +        self
> +    }
>  }
>
>  /// This macro can be used (by just passing it a type) to forward the `VMState`
> @@ -510,6 +521,50 @@ macro_rules! vmstate_fields {
>      }}
>  }
>
> +pub extern "C" fn rust_vms_test_field_exists<T, F: for<'a> FnCall<(&'a T, u8), bool>>(
> +    opaque: *mut c_void,
> +    version_id: c_int,
> +) -> bool {
> +    let owner: &T = unsafe { &*(opaque.cast::<T>()) };
> +    let version: u8 = version_id.try_into().unwrap();
> +    // SAFETY: the opaque was passed as a reference to `T`.
> +    F::call((owner, version))
> +}
> +
> +pub type VMSFieldExistCb = unsafe extern "C" fn(
> +    opaque: *mut std::os::raw::c_void,
> +    version_id: std::os::raw::c_int,
> +) -> bool;
> +
> +#[doc(alias = "VMSTATE_VALIDATE")]
> +#[macro_export]
> +macro_rules! vmstate_validate {
> +    ($struct_name:ty, $test_name:expr, $test_fn:expr $(,)?) => {
> +        $crate::bindings::VMStateField {
> +            name: ::std::ffi::CStr::as_ptr($test_name),
> +            // TODO: Use safe callback.

Why is the TODO still there?

> +            field_exists: {
> +                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;
> +                    $crate::vmstate::rust_vms_test_field_exists::<T, F>
> +                }
> +
> +                const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
> +                    ::core::marker::PhantomData
> +                }
> +                Some(test_cb_builder__::<$struct_name, _>(phantom__(&$test_fn)))
> +            },
> +            ..$crate::zeroable::Zeroable::ZERO
> +        }
> +        .with_exist_check()
> +    };

Would it be possible, or make sense, to move most of the code for
field_exists inside .with_exist_check()?

Paolo

Re: [PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test

[Paolo Bonzini]

On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> Hi,
>
> This series is in preparation for HPET migration support (in particular,
> to support varray and vmstate_validate), and it also cleans up and fixes
> the current vmstate! However, there is still the gap from being a ‘stable’
> vmstate.

Already a great improvement!

I quickly went through things that I was planning to do in a slightly
different way, but overall this is great work.  Give me a day or two
to finish reviewing and testing, most of it can already be applied to
qemu-rust or even to 10.0.

Thanks,

Paolo

Re: [PATCH 03/17] rust/vmstate: Add a prefix separator ", " for the array field in vmstate macros

[Zhao Liu]

On Mon, Mar 17, 2025 at 05:37:06PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 17:37:06 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 03/17] rust/vmstate: Add a prefix separator ", " for
>  the array field in vmstate macros
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > The fields are separated by ",", so it's necessary to add ", " in array
> > field to avoid matching failure.
> 
> This is not a field though, the only (intended) fields are name and
> field. It's meant to mimic the slice since &a[0..n].
> 

Oops, I misunderstood the format...yes, something like

vmstate_struct!(HPETState, timers[0 .. num_timers], VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>)

can work!

I'll drop this patch and refresh the unit test in v2.

Thanks,
Zhao

Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro

[Zhao Liu]

On Mon, Mar 17, 2025 at 06:17:07PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 18:17:07 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
>  vmsd in vmstate_struct macro
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > When specify an array field in vmstate_struct macro, there will be an
> > error:
> >
> > > local ambiguity when calling macro `vmstate_struct`: multiple parsing
> > > options: built-in NTs expr ('vmsd') or 1 other option.
> >
> > This is because "expr" can't recognize the "vmsd" field correctly, so
> > that it gets confused with the previous array field.
> >
> > To fix the above issue, use "ident" for "vmsd" field, and explicitly
> > refer to it in the macro.
> 
> I think this is not needed if the varray case is left as is, and other
> cases use .with_...() instead of arguments?
> 

Yes! With a[0..num], the `vmsd` could be parsed correctly. I'll drop this
patch as well and refresh unit tests.

Thanks,
Zhao

Re: [PATCH 12/17] rust/vmstate: Support version field in vmstate macros

[Zhao Liu]

On Mon, Mar 17, 2025 at 05:38:10PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 17:38:10 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 12/17] rust/vmstate: Support version field in vmstate
>  macros
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > Add "version = *" in vmstate macros to help set version_id in
> > VMStateField.
> 
> Could it use a ".with_min_version(2)" annotation (or something similar) instead?
> 

Ah, thanks! I didn't realize there was already a with_version_id() and
didn't understand your design intent, so

vmstate_of!(FooC, ptr).with_version_id(2),

would have been good enough! There is no need to add this `version` field.

Regards,
Zhao

Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro

[Zhao Liu]

On Tue, Mar 18, 2025 at 10:46:10AM +0800, Zhao Liu wrote:
> Date: Tue, 18 Mar 2025 10:46:10 +0800
> From: Zhao Liu <zhao1.liu@intel.com>
> Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
>  vmsd in vmstate_struct macro
> 
> On Mon, Mar 17, 2025 at 06:17:07PM +0100, Paolo Bonzini wrote:
> > Date: Mon, 17 Mar 2025 18:17:07 +0100
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
> >  vmsd in vmstate_struct macro
> > 
> > On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > >
> > > When specify an array field in vmstate_struct macro, there will be an
> > > error:
> > >
> > > > local ambiguity when calling macro `vmstate_struct`: multiple parsing
> > > > options: built-in NTs expr ('vmsd') or 1 other option.
> > >
> > > This is because "expr" can't recognize the "vmsd" field correctly, so
> > > that it gets confused with the previous array field.
> > >
> > > To fix the above issue, use "ident" for "vmsd" field, and explicitly
> > > refer to it in the macro.
> > 
> > I think this is not needed if the varray case is left as is, and other
> > cases use .with_...() instead of arguments?
> > 
> 
> Yes! With a[0..num], the `vmsd` could be parsed correctly. I'll drop this
> patch as well and refresh unit tests.
> 

Additionally, at present, IMO it is not suitable to replace the vmsd argument
with .with_vmsd() method because VMS_STRUCT requires a vmsd field, and
.with_vmsd() is optional.

There is no way to ensure that vmsd is not omitted... unless VMS_STRUCT is
also set within .with_vmsd(). This would be akin to merging vmstate_struct
and vmstate_of, but I suppose that would be a long way as for now.

So I prefer vmsd argument at the moment :-)

Thanks,
Zhao

Re: [PATCH 13/17] rust/vmstate: Support vmstate_validate

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]

Il mar 18 mar 2025, 07:16 Zhao Liu <zhao1.liu@intel.com> ha scritto:

> > Would it be possible, or make sense, to move most of the code for
> > field_exists inside .with_exist_check()?
> >
>
> If so, the method would be like:
>
>     pub fn with_exist_check<T, F>(
>          mut self,
>          _cb: F
>      ) -> Self
>      where
>          F: for<'a> FnCall<(&'a T, u8), bool>,
>
> Then the use case could be like:
>
>     vmstate_struct!(HPETState, timers[0 .. num_timers],
> &VMSTATE_HPET_TIMER,
> BqlRefCell<HPETTimer>).with_exist_check<HPETState, _>(foo_field_check),
>
> Here we need to specify the structure type in with_exist_check, though it's
> already specified in vmstate_struct as the first field.
>
> In this way, I understand with_exist_check() doesn't need phantom__()
> trick.
>
> Instead, (after I dropped the few patches you mentioned,) now vmstate_of
> & vmstate_struct could accept the optional "test_fn" field (luckily, at
> least test_fn can still be parsed!), then the example would be:
>
>     vmstate_struct!(HPETState, timers[0 .. num_timers],
> &VMSTATE_HPET_TIMER,
> BqlRefCell<HPETTimer>, foo_field_check)
>
> And in this way, phantom__() is necessary.
>
> So I think the main issue is the format, which do you prefer?
>

For now I would leave out a generic field-exists check, and keep the
implementation of vmstate_validate as you did it in v1.

Once we look more in the builder concept we can think of adding also a
VMStateField<T> (with a PhantomData<fn(&T) -> bool> inside) and add
with_field_exists().

Paolo


> Thanks,
> Zhao
>
>

[-- Attachment #2: Type: text/html, Size: 2517 bytes --]

Re: [PATCH 13/17] rust/vmstate: Support vmstate_validate

[Zhao Liu]

> > +#[doc(alias = "VMSTATE_VALIDATE")]
> > +#[macro_export]
> > +macro_rules! vmstate_validate {
> > +    ($struct_name:ty, $test_name:expr, $test_fn:expr $(,)?) => {
> > +        $crate::bindings::VMStateField {
> > +            name: ::std::ffi::CStr::as_ptr($test_name),
> > +            // TODO: Use safe callback.
> 
> Why is the TODO still there?

I forgot to delete this comment...

> > +            field_exists: {
> > +                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;
> > +                    $crate::vmstate::rust_vms_test_field_exists::<T, F>
> > +                }
> > +
> > +                const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
> > +                    ::core::marker::PhantomData
> > +                }
> > +                Some(test_cb_builder__::<$struct_name, _>(phantom__(&$test_fn)))
> > +            },
> > +            ..$crate::zeroable::Zeroable::ZERO
> > +        }
> > +        .with_exist_check()
> > +    };
> 
> Would it be possible, or make sense, to move most of the code for
> field_exists inside .with_exist_check()?
> 

If so, the method would be like:

    pub fn with_exist_check<T, F>(
         mut self,
         _cb: F
     ) -> Self
     where
         F: for<'a> FnCall<(&'a T, u8), bool>,

Then the use case could be like:

    vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER,
BqlRefCell<HPETTimer>).with_exist_check<HPETState, _>(foo_field_check),

Here we need to specify the structure type in with_exist_check, though it's
already specified in vmstate_struct as the first field.

In this way, I understand with_exist_check() doesn't need phantom__()
trick.

Instead, (after I dropped the few patches you mentioned,) now vmstate_of
& vmstate_struct could accept the optional "test_fn" field (luckily, at
least test_fn can still be parsed!), then the example would be:

    vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER,
BqlRefCell<HPETTimer>, foo_field_check)

And in this way, phantom__() is necessary.

So I think the main issue is the format, which do you prefer?

Thanks,
Zhao

Re: [PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro

[Zhao Liu]

On Mon, Mar 17, 2025 at 06:11:35PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 18:11:35 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro
> 
> Thanks very much for the tests!
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > -pub use crate::bindings::{VMStateDescription, VMStateField};
> > -use crate::{
> > -    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
> > -};
> > +pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};
> 
> Does VMStateFlags have to be part of the public API?

I can do `use qemu_api::bindings::VMStateFlags` in vmstate_test.rs
directly, which is better since it's the raw value of VMStateField that
I need to check!

> > +    assert_eq!(foo_fields[0].info, unsafe {
> > +        ::core::ptr::addr_of!(vmstate_info_int8)
> > +    });
> 
> You can use & instead of addr_of here.

Thanks! Will fix.

Regards,
Zhao

Re: [PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test

[Zhao Liu]

On Mon, Mar 17, 2025 at 06:20:15PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 18:20:15 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > Hi,
> >
> > This series is in preparation for HPET migration support (in particular,
> > to support varray and vmstate_validate), and it also cleans up and fixes
> > the current vmstate! However, there is still the gap from being a ‘stable’
> > vmstate.
> 
> Already a great improvement!
> 
> I quickly went through things that I was planning to do in a slightly
> different way, but overall this is great work.  Give me a day or two
> to finish reviewing and testing, most of it can already be applied to
> qemu-rust or even to 10.0.

Thanks for your quick feedback! I think the main open now is about the
best way to implement `field_exists`... pls see my patch 13 reply.

I can quickly refresh v2 to help you apply!

Regards,
Zhao

Re: [PATCH 13/17] rust/vmstate: Support vmstate_validate

[Zhao Liu]

> For now I would leave out a generic field-exists check, and keep the
> implementation of vmstate_validate as you did it in v1.
> 
> Once we look more in the builder concept we can think of adding also a
> VMStateField<T> (with a PhantomData<fn(&T) -> bool> inside) and add
> with_field_exists().

This makes sense!

Thanks,
Zhao