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

[Paolo Bonzini]

On Mon, Jan 27, 2025 at 11:12 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> > +    /// 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/?

Yes.

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

Yes, that's true but it's just a line of code. Almost all devices
*will* want to do
something with 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.
>
> +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
>
> ---
>
> Then device doesn't need to implement this reset trait if unnecessary.
> Though this way add duplicate methods, it reduces the burden on the
> device developer.

For now I prefer to make things homogeneous... this way if someone has
to copy the code into a wrapper for a new interface they don't have to
wonder about small differences.

(This by the way will also be a reason to use function pointers for
character devices as well, instead of the trait approach that I used
in https://lists.nongnu.org/archive/html/qemu-rust/2024-12/msg00006.html).

Paolo