The proposed suggestion is not correct.  First it is not necessary for
*all* classes to be Zeroable, only for Rust-defined ones; classes
defined in C never implement ObjectImpl.

Second, the parent class field need not be Zeroable.  For example,
ChardevClass's chr_write and chr_be_event fields cannot be NULL,
therefore ChardevClass cannot be Zeroable.  However, char_class_init()
initializes them, therefore ChardevClass could be subclassed by Rust code.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/qom.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 6929e4d33ae..52e3a1ec981 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -534,9 +534,10 @@ pub trait ObjectImpl: ObjectType + IsA<Object> {
     /// While `klass`'s parent class is initialized on entry, the other fields
     /// are all zero; it is therefore assumed that all fields in `T` can be
     /// zeroed, otherwise it would not be possible to provide the class as a
-    /// `&mut T`.  TODO: add a bound of [`Zeroable`](crate::zeroable::Zeroable)
-    /// to T; this is more easily done once Zeroable does not require a manual
-    /// implementation (Rust 1.75.0).
+    /// `&mut T`.  TODO: it may be possible to add an unsafe trait that checks
+    /// that all fields *after the parent class* (but not the parent class
+    /// itself) are Zeroable.  This unsafe trait can be added via a derive
+    /// macro.
     const CLASS_INIT: fn(&mut Self::Class);
 }
 
-- 
2.49.0

On Mon, May 05, 2025 at 11:04:32AM +0200, Paolo Bonzini wrote:
> Date: Mon,  5 May 2025 11:04:32 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 07/11] rust: qom: fix TODO about zeroability of classes
> X-Mailer: git-send-email 2.49.0
> 
> The proposed suggestion is not correct.  First it is not necessary for
> *all* classes to be Zeroable, only for Rust-defined ones; classes
> defined in C never implement ObjectImpl.
> 
> Second, the parent class field need not be Zeroable.  For example,
> ChardevClass's chr_write and chr_be_event fields cannot be NULL,
> therefore ChardevClass cannot be Zeroable.  However, char_class_init()
> initializes them, therefore ChardevClass could be subclassed by Rust code.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/qom.rs | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Yes, it's clearly explained.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

On Mon, May 5, 2025 at 12:04 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The proposed suggestion is not correct.  First it is not necessary for
> *all* classes to be Zeroable, only for Rust-defined ones; classes
> defined in C never implement ObjectImpl.
>
> Second, the parent class field need not be Zeroable.  For example,
> ChardevClass's chr_write and chr_be_event fields cannot be NULL,
> therefore ChardevClass cannot be Zeroable.  However, char_class_init()
> initializes them, therefore ChardevClass could be subclassed by Rust code.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/qom.rs | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
> index 6929e4d33ae..52e3a1ec981 100644
> --- a/rust/qemu-api/src/qom.rs
> +++ b/rust/qemu-api/src/qom.rs
> @@ -534,9 +534,10 @@ pub trait ObjectImpl: ObjectType + IsA<Object> {
>      /// While `klass`'s parent class is initialized on entry, the other fields
>      /// are all zero; it is therefore assumed that all fields in `T` can be
>      /// zeroed, otherwise it would not be possible to provide the class as a
> -    /// `&mut T`.  TODO: add a bound of [`Zeroable`](crate::zeroable::Zeroable)
> -    /// to T; this is more easily done once Zeroable does not require a manual
> -    /// implementation (Rust 1.75.0).
> +    /// `&mut T`.  TODO: it may be possible to add an unsafe trait that checks
> +    /// that all fields *after the parent class* (but not the parent class
> +    /// itself) are Zeroable.  This unsafe trait can be added via a derive
> +    /// macro.
>      const CLASS_INIT: fn(&mut Self::Class);
>  }
>
> --
> 2.49.0
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>