Re: [PATCH 2/9] rust/vmstate: Support varray's num field wrapped in BqlCell

[Paolo Bonzini]

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