| Message ID | 20250414144943.1112885-3-zhao1.liu@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust/hpet: Initial support for migration | expand |
On 4/14/25 16:49, Zhao Liu wrote: > Currently, if the `num` field of a varray is not a numeric type, such as > being placed in a wrapper, the array variant of assert_field_type will > fail the check. > > HPET currently wraps num_timers in BqlCell<>. Although BqlCell<> is not > necessary from strictly speaking, it makes sense for vmstate to respect > BqlCell. Dropping BqlCell<> from num_timers is indeed possible. But I agree that getting BqlCell<> varrays to work is a good thing anyway; then you can separately decide whether to drop BqlCell<> from num_timers. > The failure of assert_field_type is because it cannot convert BqlCell<T> > into usize for use as the index. > > Therefore, first, implement `From` trait for common numeric types on > BqlCell<>. Then, abstract the wrapper and non-wrapper cases uniformly > into a `IntoUsize` trait and make assert_field_type to get usize type > index via `IntoUsize` trait. > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > rust/qemu-api/src/assertions.rs | 30 +++++++++++++++++++++++++++++- > rust/qemu-api/src/cell.rs | 23 +++++++++++++++++++++++ > 2 files changed, 52 insertions(+), 1 deletion(-) I think you can drop the "num=" case of assert_field_type!, and use something like this macro: /// Drop everything up to the colon, with the intention that /// `if_present!` is called inside an optional macro substitution /// (such as `$(... $arg ...)?` or `$(... $arg ...)*`). This allows /// expanding `$result` depending on the presence of an argument, /// even if the argument itself is not included in `$result`. /// /// # Examples /// /// ``` /// # use qemu_api::if_present; /// macro_rules! is_present { /// ($($cond:expr)?) => { /// loop { /// $(if_present!([$cond]: break true;);)? /// #[allow(unreachable_code)] /// break false; /// } /// } /// } /// /// assert!(!is_present!()); /// assert!(is_present!("abc")); /// ``` #[macro_export] macro_rules! if_present { ([$($cond:tt)*]: $($result:tt)*) => { $($result)* }; } to expand the array part of the access: assert_field_type!(... $($crate::if_present!([$num]: [0]))?; ); With this change, assert_field_type! is nicer and at least the trait you're introducing in assertions.rs goes away... > +// Orphan rules don't like something like `impl<T> From<BqlCell<T>> for T`. > +// It's enough to just implement Into for common types. > +macro_rules! impl_into_inner { > + ($type:ty) => { > + impl From<BqlCell<$type>> for $type { > + fn from(c: BqlCell<$type>) -> $type { > + c.get() > + } > + } > + }; > +} ... and it's not clear to me whether this is needed with the change above? Would impl_vmstate_transparent!'s definition of VARRAY_FLAG be enough? If not, I *think* you can do a blanket implementation of Into<T> for BqlCell<T>. Maybe that's nicer, you can decide. Paolo
diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs index eb12e9499a72..232cac5b8dba 100644 --- a/rust/qemu-api/src/assertions.rs +++ b/rust/qemu-api/src/assertions.rs @@ -22,6 +22,34 @@ impl<T> EqType for T { type Itself = T; } +pub trait IntoUsize { + fn into_usize(v: Self) -> usize; +} + +macro_rules! impl_into_usize { + ($type:ty) => { + impl IntoUsize for $type { + fn into_usize(v: Self) -> usize { + v.try_into().unwrap() + } + } + + impl IntoUsize for crate::cell::BqlCell<$type> { + fn into_usize(v: Self) -> usize { + let tmp: $type = v.try_into().unwrap(); + tmp.try_into().unwrap() + } + } + }; +} + +// vmstate_n_elems() in C side supports such types. +impl_into_usize!(u8); +impl_into_usize!(u16); +impl_into_usize!(i32); +impl_into_usize!(u32); +impl_into_usize!(u64); + /// Assert that two types are the same. /// /// # Examples @@ -101,7 +129,7 @@ fn types_must_be_equal<T, U>(_: T) T: $crate::assertions::EqType<Itself = U>, { } - let index: usize = v.$num.try_into().unwrap(); + let index: usize = $crate::assertions::IntoUsize::into_usize(v.$num); types_must_be_equal::<_, &$ti>(&v.$i[index]); } }; diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs index ab0785a26928..d31bff093707 100644 --- a/rust/qemu-api/src/cell.rs +++ b/rust/qemu-api/src/cell.rs @@ -309,6 +309,29 @@ fn from(t: T) -> BqlCell<T> { } } +// Orphan rules don't like something like `impl<T> From<BqlCell<T>> for T`. +// It's enough to just implement Into for common types. +macro_rules! impl_into_inner { + ($type:ty) => { + impl From<BqlCell<$type>> for $type { + fn from(c: BqlCell<$type>) -> $type { + c.get() + } + } + }; +} + +impl_into_inner!(bool); +impl_into_inner!(i8); +impl_into_inner!(i16); +impl_into_inner!(i32); +impl_into_inner!(i64); +impl_into_inner!(u8); +impl_into_inner!(u16); +impl_into_inner!(u32); +impl_into_inner!(u64); +impl_into_inner!(usize); + impl<T: fmt::Debug + Copy> fmt::Debug for BqlCell<T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.get().fmt(f)
Currently, if the `num` field of a varray is not a numeric type, such as being placed in a wrapper, the array variant of assert_field_type will fail the check. HPET currently wraps num_timers in BqlCell<>. Although BqlCell<> is not necessary from strictly speaking, it makes sense for vmstate to respect BqlCell. The failure of assert_field_type is because it cannot convert BqlCell<T> into usize for use as the index. Therefore, first, implement `From` trait for common numeric types on BqlCell<>. Then, abstract the wrapper and non-wrapper cases uniformly into a `IntoUsize` trait and make assert_field_type to get usize type index via `IntoUsize` trait. Signed-off-by: Zhao Liu <zhao1.liu@intel.com> --- rust/qemu-api/src/assertions.rs | 30 +++++++++++++++++++++++++++++- rust/qemu-api/src/cell.rs | 23 +++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-)