Re: [RFC 07/18] rust: move Cell vmstate impl

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

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

Hi

On Tue, Aug 26, 2025 at 10:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 8/26/25 16:04, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > This will allow to split vmstate to a standalone crate next.
>
> Can you explain why this is needed?  Could "migration" depend on "bql"
> (or even, "bql" could stay in util), and keep the implementation of
> VMState for cells, just like you do for Opaque?
>

vmstate doesn't require bql. Why should we enforce it at rust level?

If we merge bql in util, then sure we can make vmstate keep the VMState
impl for BqlCells. But why should we do that? To save one crate? I think it
will more future proof if we have a lower-level util crate without bql, and
higher-level crates that rely on it. Perhaps "bql" should be renamed though
(qemu-loop/iothread or something?)


>
> Thanks,
>
> Paolo
>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   rust/qemu-api/src/cell.rs    |  5 ++++-
> >   rust/qemu-api/src/vmstate.rs | 16 +++++++---------
> >   2 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs
> > index 98d720caf9..4bce526e45 100644
> > --- a/rust/qemu-api/src/cell.rs
> > +++ b/rust/qemu-api/src/cell.rs
> > @@ -152,7 +152,7 @@
> >       ptr::NonNull,
> >   };
> >
> > -use crate::bindings;
> > +use crate::{bindings, impl_vmstate_transparent};
> >
> >   /// An internal function that is used by doctests.
> >   pub fn bql_start_test() {
> > @@ -866,3 +866,6 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) ->
> fmt::Result {
> >           (**self).fmt(f)
> >       }
> >   }
> > +
> > +impl_vmstate_transparent!(BqlCell<T> where T: VMState);
> > +impl_vmstate_transparent!(BqlRefCell<T> where T: VMState);
> > diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> > index c1e2b06390..9d33997c57 100644
> > --- a/rust/qemu-api/src/vmstate.rs
> > +++ b/rust/qemu-api/src/vmstate.rs
> > @@ -29,8 +29,7 @@
> >
> >   use common::{callbacks::FnCall, Zeroable};
> >
> > -use crate::bindings::VMStateFlags;
> > -pub use crate::bindings::{VMStateDescription, VMStateField};
> > +pub use crate::bindings::{VMStateDescription, VMStateField,
> VMStateFlags};
> >
> >   /// This macro is used to call a function with a generic argument bound
> >   /// to the type of a field.  The function must take a
> > @@ -325,15 +324,16 @@ unsafe impl $crate::vmstate::VMState for $tuple {
> >
> >   // Transparent wrappers: just use the internal type
> >
> > +#[macro_export]
> >   macro_rules! impl_vmstate_transparent {
> >       ($type:ty where $base:tt: VMState $($where:tt)*) => {
> > -        unsafe impl<$base> VMState for $type where $base: VMState
> $($where)* {
> > -            const SCALAR_TYPE: VMStateFieldType = <$base as
> VMState>::SCALAR_TYPE;
> > -            const BASE: VMStateField = VMStateField {
> > +        unsafe impl<$base> $crate::vmstate::VMState for $type where
> $base: $crate::vmstate::VMState $($where)* {
> > +            const SCALAR_TYPE: $crate::vmstate::VMStateFieldType =
> <$base as $crate::vmstate::VMState>::SCALAR_TYPE;
> > +            const BASE: $crate::vmstate::VMStateField =
> $crate::vmstate::VMStateField {
> >                   size: mem::size_of::<$type>(),
> > -                ..<$base as VMState>::BASE
> > +                ..<$base as $crate::vmstate::VMState>::BASE
> >               };
> > -            const VARRAY_FLAG: VMStateFlags = <$base as
> VMState>::VARRAY_FLAG;
> > +            const VARRAY_FLAG: $crate::vmstate::VMStateFlags = <$base
> as $crate::vmstate::VMState>::VARRAY_FLAG;
> >           }
> >       };
> >   }
> > @@ -341,8 +341,6 @@ unsafe impl<$base> VMState for $type where $base:
> VMState $($where)* {
> >   impl_vmstate_transparent!(std::cell::Cell<T> where T: VMState);
> >   impl_vmstate_transparent!(std::cell::UnsafeCell<T> where T: VMState);
> >   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!(::common::Opaque<T> where T: VMState);
> >
> >   #[macro_export]
>
>

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