1. Patchew
  2. QEMU
  3. [PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test
  4. View patch

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

Zhao Liu posted 17 patches 2 weeks, 2 days ago
There is a newer version of this series
[PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro
Posted by Zhao Liu 2 weeks, 2 days ago 
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
Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro
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:
>
> 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 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro
Posted by Zhao Liu 2 weeks, 2 days ago 
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 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro
Posted by Zhao Liu 2 weeks, 2 days ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.