Re: [PATCH 08/10] rust: qdev: switch from legacy reset to Resettable

[Zhao Liu <zhao1.liu@intel.com>]

> +/// Trait providing the contents of [`ResettablePhases`].
> +pub trait ResettablePhasesImpl {
> +    /// If not None, this is called when the object enters reset. It
> +    /// can reset local state of the object, but it must not do anything that
> +    /// has a side-effect on other objects, such as raising or lowering a
> +    /// [`qemu_irq`] line or reading or writing guest memory. It takes the
> +    /// reset's type as argument.
> +    const ENTER: Option<fn(&Self, ResetType)> = None;
> +
> +    /// If not None, this is called when the object for entry into reset, once
> +    /// every object in the system which is being reset has had its
> +    /// @phases.enter method called. At this point devices can do actions

Maybe s/@phases.enter/ResettablePhasesImpl::ENTER/?

> +    /// that affect other objects.
> +    ///
> +    /// If in doubt, implement this method.
> +    const HOLD: Option<fn(&Self, ResetType)> = None;
> +
> +    /// If not None, this phase is called when the object leaves the reset
> +    /// state. Actions affecting other objects are permitted.
> +    const EXIT: Option<fn(&Self, ResetType)> = None;
> +}
> +

...
 
>  impl<T> ClassInitImpl<DeviceClass> for T
>  where
> -    T: ClassInitImpl<ObjectClass> + DeviceImpl,
> +    T: ClassInitImpl<ObjectClass> + ClassInitImpl<ResettableClass> + DeviceImpl,

This constraint requires each device to explicitly implement ResettablePhasesImpl,
even the device doesn't want to do anything for reset.

So what about the following changes:
 * Define 3-Phases methods in DeviceImpl.
 * Implement ResettablePhasesImpl for all devices by passing their 3-Phases
   methods to ResettablePhasesImpl's.

Then device doesn't need to implement this reset trait if unnecessary.

For example:

--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -84,7 +84,7 @@ pub trait ResettablePhasesImpl {
 }

 /// Trait providing the contents of [`DeviceClass`].
-pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl {
+pub trait DeviceImpl: ObjectImpl {
     /// _Realization_ is the second stage of device creation. It contains
     /// all operations that depend on device properties and can fail (note:
     /// this is not yet supported for Rust devices).
@@ -106,6 +106,18 @@ fn properties() -> &'static [Property] {
     fn vmsd() -> Option<&'static VMStateDescription> {
         None
     }
+
+    const RESET_ENTER: Option<fn(&Self, ResetType)> = None;
+    const RESET_HOLD: Option<fn(&Self, ResetType)> = None;
+    const RESET_EXIT: Option<fn(&Self, ResetType)> = None;
+}
+
+impl<T> ResettablePhasesImpl for T
+where T: DeviceImpl
+{
+    const ENTER: Option<fn(&Self, ResetType)> = T::RESET_ENTER;
+    const HOLD: Option<fn(&Self, ResetType)> = T::RESET_HOLD;
+    const EXIT: Option<fn(&Self, ResetType)> = T::RESET_EXIT;
 }

 /// # Safety

---

Though this way add duplicate methods, it reduces the burden on the
device developer.

Regards,
Zhao

>  {
>      fn class_init(dc: &mut DeviceClass) {
>          if <T as DeviceImpl>::REALIZE.is_some() {
>              dc.realize = Some(rust_realize_fn::<T>);
>          }
> -        if <T as DeviceImpl>::RESET.is_some() {
> -            unsafe {
> -                bindings::device_class_set_legacy_reset(dc, Some(rust_reset_fn::<T>));
> -            }
> -        }
>          if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
>              dc.vmsd = vmsd;
>          }
> @@ -99,6 +162,7 @@ fn class_init(dc: &mut DeviceClass) {
>              }
>          }
>  
> +        ResettableClass::interface_init::<T, DeviceState>(dc);
>          <T as ClassInitImpl<ObjectClass>>::class_init(&mut dc.parent_class);
>      }
>  }