[v1] rust/vmstate: Clean up, fix, enhance & test

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

Posted by Zhao Liu 2 weeks, 2 days ago

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

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

Posted by Paolo Bonzini 2 weeks, 2 days ago

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 03/17] rust/vmstate: Add a prefix separator ", " for the array field in vmstate macros

Posted by Zhao Liu 2 weeks, 2 days ago

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