[PATCH 0/9] qom: Consolidate OBJECT_{DEFINE|DECLARE}_SIMPLE_TYPE*

[PATCH 0/9] qom: Consolidate OBJECT_{DEFINE|DECLARE}_SIMPLE_TYPE*

[Zhao Liu]

Hi,

When I read the qom doc (qom.rst) about OBJECT_DECLARE_SIMPLE_TYPE, I
found there's the wrong example. So I tried to fix the doc and cleaned
up the relevant code case to correctly show that for simple type, the
class definition is not needed.

This series tries to:

 * fix & improve the documentation of OBJECT_{DEFINE|DECLARE}_SIMPLE_TYPE*,

 * and clean up several cases that could have used simple type macros
   (mainly by dropping unneeded class definitions).

Thanks and Best Regards,
Zhao
---
Zhao Liu (9):
  qom/object: Improve the doc of macros related with simple type
  docs/devel/qom: Fix the doc about OBJECT_DECLARE_SIMPLE_TYPE
  hw/acpi/pci: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
  hw/char/sh_serial: Consolidate
    OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
  hw/hyperv/hv-balloon: Consolidate
    OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
  hw/ppc/pef: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
  hw/core/resetcontainer: Consolidate OBJECT_DECLARE_SIMPLE_TYPE
  target/s390x/kvm/pv: Consolidate
    OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
  ui/console-vc: Consolidate OBJECT_DEFINE_SIMPLE_TYPE

 docs/devel/qom.rst               | 11 +----------
 hw/acpi/pci.c                    | 24 ++++++++----------------
 hw/char/sh_serial.c              |  4 +---
 hw/hyperv/hv-balloon.c           |  8 ++------
 hw/ppc/pef.c                     | 17 ++++++-----------
 include/hw/core/resetcontainer.h |  2 +-
 include/qom/object.h             |  5 +++--
 target/s390x/kvm/pv.c            | 18 ++++++------------
 ui/console-vc.c                  | 14 ++++++++------
 9 files changed, 36 insertions(+), 67 deletions(-)

-- 
2.34.1

[PATCH 1/9] qom/object: Improve the doc of macros related with simple type

[Zhao Liu]

There're 2 changes:
 - For OBJECT_DECLARE_SIMPLE_TYPE:

   Since a clase may not only has virtual method, but also field, thus
   mention when class is not needed for either, then there is no need
   to have the class.

 - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:

   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <eduardo@habkost.net>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/qom/object.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 1d5b03372429..14f84ae454d3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -249,7 +249,7 @@ struct Object
  * declared.
  *
  * This macro should be used unless the class struct needs to have
- * virtual methods declared.
+ * virtual methods or fields declared.
  */
 #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
     typedef struct InstanceType InstanceType; \
@@ -402,7 +402,8 @@ struct Object
  *
  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
  * the case of a non-abstract type, with interfaces, and with no requirement
- * for a class struct.
+ * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
+ * then this is probably the right choice for defining it.
  */
 #define OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(ModuleObjName, \
                                                   module_obj_name, \
-- 
2.34.1

[PATCH 2/9] docs/devel/qom: Fix the doc about OBJECT_DECLARE_SIMPLE_TYPE

[Zhao Liu]

Currently, the expansion example of OBJECT_DECLARE_SIMPLE_TYPE "roughly"
reflects what OBJECT_DECLARE_TYPE is doing.

Why "roughly"? Because this line -

>    G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)

- is also wrong for OBJECT_DECLARE_TYPE.

Fix the expansion example of OBJECT_DECLARE_SIMPLE_TYPE, especially
drop that definition of MyDeviceClass.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <eduardo@habkost.net>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 docs/devel/qom.rst | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index 5870745ba27b..185f4c2f5921 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -326,21 +326,12 @@ This is equivalent to the following:
    :caption: Expansion from declaring a simple type
 
    typedef struct MyDevice MyDevice;
-   typedef struct MyDeviceClass MyDeviceClass;
 
-   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
+   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
 
-   #define MY_DEVICE_GET_CLASS(void *obj) \
-           OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
-   #define MY_DEVICE_CLASS(void *klass) \
-           OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
    #define MY_DEVICE(void *obj)
            OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
 
-   struct MyDeviceClass {
-       DeviceClass parent_class;
-   };
-
 The 'struct MyDevice' needs to be declared separately.
 If the type requires virtual functions to be declared in the class
 struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
-- 
2.34.1

[PATCH 3/9] hw/acpi/pci: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

The QOM types of AcpiGenericInitiator and AcpiGenericPort are declared
by OBJECT_DECLARE_SIMPLE_TYPE, which means they don't need the class!

Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
the type, then there's no need for class definition.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/acpi/pci.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index d511a8502954..acac6744525e 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -75,16 +75,12 @@ typedef struct AcpiGenericInitiator {
     uint32_t node;
 } AcpiGenericInitiator;
 
-typedef struct AcpiGenericInitiatorClass {
-    ObjectClass parent_class;
-} AcpiGenericInitiatorClass;
-
 #define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
 
-OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
-                   ACPI_GENERIC_INITIATOR, OBJECT,
-                   { TYPE_USER_CREATABLE },
-                   { NULL })
+OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
+                                          ACPI_GENERIC_INITIATOR, OBJECT,
+                                          { TYPE_USER_CREATABLE },
+                                          { NULL })
 
 OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericInitiator, ACPI_GENERIC_INITIATOR)
 
@@ -191,16 +187,12 @@ typedef struct AcpiGenericPort {
     uint32_t node;
 } AcpiGenericPort;
 
-typedef struct AcpiGenericPortClass {
-    ObjectClass parent_class;
-} AcpiGenericPortClass;
-
 #define TYPE_ACPI_GENERIC_PORT "acpi-generic-port"
 
-OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
-                   ACPI_GENERIC_PORT, OBJECT,
-                   { TYPE_USER_CREATABLE },
-                   { NULL })
+OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
+                                          ACPI_GENERIC_PORT, OBJECT,
+                                          { TYPE_USER_CREATABLE },
+                                          { NULL })
 
 OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericPort, ACPI_GENERIC_PORT)
 
-- 
2.34.1

[PATCH 4/9] hw/char/sh_serial: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

The QOM type of SHSerialState is declared by OBJECT_DECLARE_SIMPLE_TYPE,
which means it doesn't need the class!

Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
the type, then there's no need for class definition.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/char/sh_serial.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
index 6abd80386fb4..8ccc2234ba4d 100644
--- a/hw/char/sh_serial.c
+++ b/hw/char/sh_serial.c
@@ -78,9 +78,7 @@ struct SHSerialState {
     qemu_irq bri;
 };
 
-typedef struct {} SHSerialStateClass;
-
-OBJECT_DEFINE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
+OBJECT_DEFINE_SIMPLE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
 
 static void sh_serial_clear_fifo(SHSerialState *s)
 {
-- 
2.34.1

[PATCH 5/9] hw/hyperv/hv-balloon: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

The QOM type of HvBalloon is declared by OBJECT_DECLARE_SIMPLE_TYPE,
which means it doesn't need the class!

Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
the type, then there's no need for class definition.

Cc: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/hyperv/hv-balloon.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/hyperv/hv-balloon.c b/hw/hyperv/hv-balloon.c
index 94b0abbd6834..db3be4246eae 100644
--- a/hw/hyperv/hv-balloon.c
+++ b/hw/hyperv/hv-balloon.c
@@ -67,10 +67,6 @@
  * these requests
  */
 
-struct HvBalloonClass {
-    VMBusDeviceClass parent_class;
-} HvBalloonClass;
-
 typedef enum State {
     /* not a real state */
     S_NO_CHANGE = 0,
@@ -162,8 +158,8 @@ typedef struct HvBalloon {
     MemoryRegion *mr;
 } HvBalloon;
 
-OBJECT_DEFINE_TYPE_WITH_INTERFACES(HvBalloon, hv_balloon, HV_BALLOON, VMBUS_DEVICE, \
-                                   { TYPE_MEMORY_DEVICE }, { })
+OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(HvBalloon, hv_balloon, HV_BALLOON, VMBUS_DEVICE, \
+                                          { TYPE_MEMORY_DEVICE }, { })
 
 #define HV_BALLOON_SET_STATE(hvb, news)             \
     do {                                            \
-- 
2.34.1

[PATCH 6/9] hw/ppc/pef: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

The QOM type of PefGuest is declared by OBJECT_DECLARE_SIMPLE_TYPE,
which means it doesn't need the class!

Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
the type, then there's no need for class definition.

Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/ppc/pef.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
index 254f5707876f..1832791ee533 100644
--- a/hw/ppc/pef.c
+++ b/hw/ppc/pef.c
@@ -20,11 +20,6 @@
 OBJECT_DECLARE_SIMPLE_TYPE(PefGuest, PEF_GUEST)
 
 typedef struct PefGuest PefGuest;
-typedef struct PefGuestClass PefGuestClass;
-
-struct PefGuestClass {
-    ConfidentialGuestSupportClass parent_class;
-};
 
 /**
  * PefGuest:
@@ -121,12 +116,12 @@ static int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp)
     return kvmppc_svm_off(errp);
 }
 
-OBJECT_DEFINE_TYPE_WITH_INTERFACES(PefGuest,
-                                   pef_guest,
-                                   PEF_GUEST,
-                                   CONFIDENTIAL_GUEST_SUPPORT,
-                                   { TYPE_USER_CREATABLE },
-                                   { NULL })
+OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(PefGuest,
+                                          pef_guest,
+                                          PEF_GUEST,
+                                          CONFIDENTIAL_GUEST_SUPPORT,
+                                          { TYPE_USER_CREATABLE },
+                                          { NULL })
 
 static void pef_guest_class_init(ObjectClass *oc, const void *data)
 {
-- 
2.34.1

[PATCH 7/9] hw/core/resetcontainer: Consolidate OBJECT_DECLARE_SIMPLE_TYPE

[Zhao Liu]

The QOM type of ResettableContainer is defined by
OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES, which means it doesn't need
the class!

Therefore, use OBJECT_DECLARE_SIMPLE_TYPE to declare the type, then
there's no need for class definition.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/core/resetcontainer.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/core/resetcontainer.h b/include/hw/core/resetcontainer.h
index 23db0c7a8806..daeb18c1ea81 100644
--- a/include/hw/core/resetcontainer.h
+++ b/include/hw/core/resetcontainer.h
@@ -20,7 +20,7 @@
 #include "qom/object.h"
 
 #define TYPE_RESETTABLE_CONTAINER "resettable-container"
-OBJECT_DECLARE_TYPE(ResettableContainer, ResettableContainerClass, RESETTABLE_CONTAINER)
+OBJECT_DECLARE_SIMPLE_TYPE(ResettableContainer, RESETTABLE_CONTAINER)
 
 /**
  * resettable_container_add: Add a resettable object to the container
-- 
2.34.1

[PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

The QOM type of S390PVGuest is declared by OBJECT_DECLARE_SIMPLE_TYPE,
which means it doesn't need the class!

Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
the type, then there's no need for class definition.

Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Ilya Leoshkevich <iii@linux.ibm.com>
Cc: qemu-s390x@nongnu.org
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/s390x/kvm/pv.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
index 2bc916a5455f..a4dbbcef7e08 100644
--- a/target/s390x/kvm/pv.c
+++ b/target/s390x/kvm/pv.c
@@ -313,12 +313,6 @@ struct S390PVGuest {
     ConfidentialGuestSupport parent_obj;
 };
 
-typedef struct S390PVGuestClass S390PVGuestClass;
-
-struct S390PVGuestClass {
-    ConfidentialGuestSupportClass parent_class;
-};
-
 /*
  * If protected virtualization is enabled, the amount of data that the
  * Read SCP Info Service Call can use is limited to one page. The
@@ -380,12 +374,12 @@ static int s390_pv_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
     return 0;
 }
 
-OBJECT_DEFINE_TYPE_WITH_INTERFACES(S390PVGuest,
-                                   s390_pv_guest,
-                                   S390_PV_GUEST,
-                                   CONFIDENTIAL_GUEST_SUPPORT,
-                                   { TYPE_USER_CREATABLE },
-                                   { NULL })
+OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
+                                          s390_pv_guest,
+                                          S390_PV_GUEST,
+                                          CONFIDENTIAL_GUEST_SUPPORT,
+                                          { TYPE_USER_CREATABLE },
+                                          { NULL })
 
 static void s390_pv_guest_class_init(ObjectClass *oc, const void *data)
 {
-- 
2.34.1

[PATCH 9/9] ui/console-vc: Consolidate OBJECT_DEFINE_SIMPLE_TYPE

[Zhao Liu]

The QOM types of QemuTextConsole and QemuFixedTextConsole are declared
by OBJECT_DECLARE_SIMPLE_TYPE, which means they don't need the class!

Therefore, use OBJECT_DEFINE_SIMPLE_TYPE to implement the type, then
there's no need for class definition.

Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 ui/console-vc.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/ui/console-vc.c b/ui/console-vc.c
index 830842064d69..1b2c6951c275 100644
--- a/ui/console-vc.c
+++ b/ui/console-vc.c
@@ -70,17 +70,19 @@ typedef struct QemuTextConsole {
     Fifo8 out_fifo;
 } QemuTextConsole;
 
-typedef QemuConsoleClass QemuTextConsoleClass;
-
-OBJECT_DEFINE_TYPE(QemuTextConsole, qemu_text_console, QEMU_TEXT_CONSOLE, QEMU_CONSOLE)
+OBJECT_DEFINE_SIMPLE_TYPE(QemuTextConsole,
+                          qemu_text_console,
+                          QEMU_TEXT_CONSOLE,
+                          QEMU_CONSOLE)
 
 typedef struct QemuFixedTextConsole {
     QemuTextConsole parent;
 } QemuFixedTextConsole;
 
-typedef QemuTextConsoleClass QemuFixedTextConsoleClass;
-
-OBJECT_DEFINE_TYPE(QemuFixedTextConsole, qemu_fixed_text_console, QEMU_FIXED_TEXT_CONSOLE, QEMU_TEXT_CONSOLE)
+OBJECT_DEFINE_SIMPLE_TYPE(QemuFixedTextConsole,
+                          qemu_fixed_text_console,
+                          QEMU_FIXED_TEXT_CONSOLE,
+                          QEMU_TEXT_CONSOLE)
 
 struct VCChardev {
     Chardev parent;
-- 
2.34.1

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[David Hildenbrand]

On 14.05.25 10:49, Zhao Liu wrote:
> The QOM type of S390PVGuest is declared by OBJECT_DECLARE_SIMPLE_TYPE,
> which means it doesn't need the class!
> 
> Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
> the type, then there's no need for class definition.
> 
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/s390x/kvm/pv.c | 18 ++++++------------
>   1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
> index 2bc916a5455f..a4dbbcef7e08 100644
> --- a/target/s390x/kvm/pv.c
> +++ b/target/s390x/kvm/pv.c
> @@ -313,12 +313,6 @@ struct S390PVGuest {
>       ConfidentialGuestSupport parent_obj;
>   };
>   
> -typedef struct S390PVGuestClass S390PVGuestClass;
> -
> -struct S390PVGuestClass {
> -    ConfidentialGuestSupportClass parent_class;
> -};
> -
>   /*
>    * If protected virtualization is enabled, the amount of data that the
>    * Read SCP Info Service Call can use is limited to one page. The
> @@ -380,12 +374,12 @@ static int s390_pv_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>       return 0;
>   }
>   
> -OBJECT_DEFINE_TYPE_WITH_INTERFACES(S390PVGuest,
> -                                   s390_pv_guest,
> -                                   S390_PV_GUEST,
> -                                   CONFIDENTIAL_GUEST_SUPPORT,
> -                                   { TYPE_USER_CREATABLE },
> -                                   { NULL })
> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
> +                                          s390_pv_guest,
> +                                          S390_PV_GUEST,
> +                                          CONFIDENTIAL_GUEST_SUPPORT,
> +                                          { TYPE_USER_CREATABLE },
> +                                          { NULL })

I'll note that existing callers of 
OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES happily ignore the line limit 
and put it into a single line.

(which ends up looking better IMHO)

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 6/9] hw/ppc/pef: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[BALATON Zoltan]

On Wed, 14 May 2025, Zhao Liu wrote:
> The QOM type of PefGuest is declared by OBJECT_DECLARE_SIMPLE_TYPE,
> which means it doesn't need the class!
>
> Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
> the type, then there's no need for class definition.
>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> hw/ppc/pef.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
> index 254f5707876f..1832791ee533 100644
> --- a/hw/ppc/pef.c
> +++ b/hw/ppc/pef.c
> @@ -20,11 +20,6 @@
> OBJECT_DECLARE_SIMPLE_TYPE(PefGuest, PEF_GUEST)
>
> typedef struct PefGuest PefGuest;
> -typedef struct PefGuestClass PefGuestClass;
> -
> -struct PefGuestClass {
> -    ConfidentialGuestSupportClass parent_class;
> -};
>
> /**
>  * PefGuest:
> @@ -121,12 +116,12 @@ static int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp)
>     return kvmppc_svm_off(errp);
> }
>
> -OBJECT_DEFINE_TYPE_WITH_INTERFACES(PefGuest,
> -                                   pef_guest,
> -                                   PEF_GUEST,
> -                                   CONFIDENTIAL_GUEST_SUPPORT,
> -                                   { TYPE_USER_CREATABLE },
> -                                   { NULL })
> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(PefGuest,
> +                                          pef_guest,
> +                                          PEF_GUEST,
> +                                          CONFIDENTIAL_GUEST_SUPPORT,
> +                                          { TYPE_USER_CREATABLE },
> +                                          { NULL })

Maybe also change the name while at it because the WITH_INTERFACES macros 
usually allow to pass an interfaces list instead of the single hardcoded 
USER_CREATABLE interface here so if such variant is added it would clash. 
Better call this something that shows this is PEF specific or just use the 
appropriate macro from qemu/object.h to define it.

Regards,
BALATON Zoltan

>
> static void pef_guest_class_init(ObjectClass *oc, const void *data)
> {
>

Re: [PATCH 6/9] hw/ppc/pef: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[BALATON Zoltan]

On Wed, 14 May 2025, BALATON Zoltan wrote:
> On Wed, 14 May 2025, Zhao Liu wrote:
>> The QOM type of PefGuest is declared by OBJECT_DECLARE_SIMPLE_TYPE,
>> which means it doesn't need the class!
>> 
>> Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
>> the type, then there's no need for class definition.
>> 
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> Cc: qemu-ppc@nongnu.org
>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
>> ---
>> hw/ppc/pef.c | 17 ++++++-----------
>> 1 file changed, 6 insertions(+), 11 deletions(-)
>> 
>> diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
>> index 254f5707876f..1832791ee533 100644
>> --- a/hw/ppc/pef.c
>> +++ b/hw/ppc/pef.c
>> @@ -20,11 +20,6 @@
>> OBJECT_DECLARE_SIMPLE_TYPE(PefGuest, PEF_GUEST)
>> 
>> typedef struct PefGuest PefGuest;
>> -typedef struct PefGuestClass PefGuestClass;
>> -
>> -struct PefGuestClass {
>> -    ConfidentialGuestSupportClass parent_class;
>> -};
>> 
>> /**
>>  * PefGuest:
>> @@ -121,12 +116,12 @@ static int pef_kvm_reset(ConfidentialGuestSupport 
>> *cgs, Error **errp)
>>     return kvmppc_svm_off(errp);
>> }
>> 
>> -OBJECT_DEFINE_TYPE_WITH_INTERFACES(PefGuest,
>> -                                   pef_guest,
>> -                                   PEF_GUEST,
>> -                                   CONFIDENTIAL_GUEST_SUPPORT,
>> -                                   { TYPE_USER_CREATABLE },
>> -                                   { NULL })
>> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(PefGuest,
>> +                                          pef_guest,
>> +                                          PEF_GUEST,
>> +                                          CONFIDENTIAL_GUEST_SUPPORT,
>> +                                          { TYPE_USER_CREATABLE },
>> +                                          { NULL })
>
> Maybe also change the name while at it because the WITH_INTERFACES macros 
> usually allow to pass an interfaces list instead of the single hardcoded 
> USER_CREATABLE interface here so if such variant is added it would clash. 
> Better call this something that shows this is PEF specific or just use the 
> appropriate macro from qemu/object.h to define it.

Ignore this, I've found the rest of the series and now see better what 
it's about. Sorry for the noise.

Regards,
BALATON Zoltan

>> 
>> static void pef_guest_class_init(ObjectClass *oc, const void *data)
>> {
>> 
>
>

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

[BALATON Zoltan]

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

On Wed, 14 May 2025, Zhao Liu wrote:
> There're 2 changes:
> - For OBJECT_DECLARE_SIMPLE_TYPE:
>
>   Since a clase may not only has virtual method, but also field, thus

class may not only have virtual methods but also fields, clarify when 
class is not needed

>   mention when class is not needed for either, then there is no need
>   to have the class.
>
> - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
>
>   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
>   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
>   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
>   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> include/qom/object.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 1d5b03372429..14f84ae454d3 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -249,7 +249,7 @@ struct Object
>  * declared.
>  *
>  * This macro should be used unless the class struct needs to have
> - * virtual methods declared.
> + * virtual methods or fields declared.
>  */
> #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
>     typedef struct InstanceType InstanceType; \
> @@ -402,7 +402,8 @@ struct Object
>  *
>  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
>  * the case of a non-abstract type, with interfaces, and with no requirement
> - * for a class struct.
> + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
> + * then this is probably the right choice for defining it.

Is "probably" correct here? Is it a must or can still use other defining 
macros? If correct maybe saying "this might be the right choice" is 
simpler.

Regards,
BALATON Zoltan

>  */
> #define OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(ModuleObjName, \
>                                                   module_obj_name, \
>

Re: [PATCH 2/9] docs/devel/qom: Fix the doc about OBJECT_DECLARE_SIMPLE_TYPE

[BALATON Zoltan]

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

On Wed, 14 May 2025, Zhao Liu wrote:
> Currently, the expansion example of OBJECT_DECLARE_SIMPLE_TYPE "roughly"
> reflects what OBJECT_DECLARE_TYPE is doing.
>
> Why "roughly"? Because this line -
>
>>    G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
>
> - is also wrong for OBJECT_DECLARE_TYPE.
>
> Fix the expansion example of OBJECT_DECLARE_SIMPLE_TYPE, especially
> drop that definition of MyDeviceClass.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> docs/devel/qom.rst | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> index 5870745ba27b..185f4c2f5921 100644
> --- a/docs/devel/qom.rst
> +++ b/docs/devel/qom.rst
> @@ -326,21 +326,12 @@ This is equivalent to the following:
>    :caption: Expansion from declaring a simple type
>
>    typedef struct MyDevice MyDevice;
> -   typedef struct MyDeviceClass MyDeviceClass;
>
> -   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> +   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
>
> -   #define MY_DEVICE_GET_CLASS(void *obj) \
> -           OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> -   #define MY_DEVICE_CLASS(void *klass) \
> -           OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
>    #define MY_DEVICE(void *obj)
>            OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
>
> -   struct MyDeviceClass {
> -       DeviceClass parent_class;
> -   };
> -
> The 'struct MyDevice' needs to be declared separately.
> If the type requires virtual functions to be declared in the class
> struct, then the alternative OBJECT_DECLARE_TYPE() macro can be

Maybe you need to adjust the text here about OBJECT_DECLARE_TYPE here and 
show how to define Class sturct?

Regards,
BALATON Zoltan

Re: [PATCH 4/9] hw/char/sh_serial: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[BALATON Zoltan]

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

On Wed, 14 May 2025, Zhao Liu wrote:
> The QOM type of SHSerialState is declared by OBJECT_DECLARE_SIMPLE_TYPE,
> which means it doesn't need the class!
>
> Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement

Patch uses OBJECT_DEFINE_SIMPLE_TYPE not 
OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES so change commit message to 
match the patch.

Regards,
BALATON Zoltan

> the type, then there's no need for class definition.
>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> hw/char/sh_serial.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
> index 6abd80386fb4..8ccc2234ba4d 100644
> --- a/hw/char/sh_serial.c
> +++ b/hw/char/sh_serial.c
> @@ -78,9 +78,7 @@ struct SHSerialState {
>     qemu_irq bri;
> };
>
> -typedef struct {} SHSerialStateClass;
> -
> -OBJECT_DEFINE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
> +OBJECT_DEFINE_SIMPLE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
>
> static void sh_serial_clear_fifo(SHSerialState *s)
> {
>

Re: [PATCH 3/9] hw/acpi/pci: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Ani Sinha]

On Wed, May 14, 2025 at 1:59 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> The QOM types of AcpiGenericInitiator and AcpiGenericPort are declared
> by OBJECT_DECLARE_SIMPLE_TYPE, which means they don't need the class!
>
> Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
> the type, then there's no need for class definition.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
>  hw/acpi/pci.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index d511a8502954..acac6744525e 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -75,16 +75,12 @@ typedef struct AcpiGenericInitiator {
>      uint32_t node;
>  } AcpiGenericInitiator;
>
> -typedef struct AcpiGenericInitiatorClass {
> -    ObjectClass parent_class;
> -} AcpiGenericInitiatorClass;
> -
>  #define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
>
> -OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
> -                   ACPI_GENERIC_INITIATOR, OBJECT,
> -                   { TYPE_USER_CREATABLE },
> -                   { NULL })
> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
> +                                          ACPI_GENERIC_INITIATOR, OBJECT,
> +                                          { TYPE_USER_CREATABLE },
> +                                          { NULL })
>
>  OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericInitiator, ACPI_GENERIC_INITIATOR)
>
> @@ -191,16 +187,12 @@ typedef struct AcpiGenericPort {
>      uint32_t node;
>  } AcpiGenericPort;
>
> -typedef struct AcpiGenericPortClass {
> -    ObjectClass parent_class;
> -} AcpiGenericPortClass;
> -
>  #define TYPE_ACPI_GENERIC_PORT "acpi-generic-port"
>
> -OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
> -                   ACPI_GENERIC_PORT, OBJECT,
> -                   { TYPE_USER_CREATABLE },
> -                   { NULL })
> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
> +                                          ACPI_GENERIC_PORT, OBJECT,
> +                                          { TYPE_USER_CREATABLE },
> +                                          { NULL })
>
>  OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericPort, ACPI_GENERIC_PORT)
>
> --
> 2.34.1
>

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

[Zhao Liu]

Hi BALATON,

On Wed, May 14, 2025 at 02:02:48PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 14:02:48 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
>  simple type
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > There're 2 changes:
> > - For OBJECT_DECLARE_SIMPLE_TYPE:
> > 
> >   Since a clase may not only has virtual method, but also field, thus
> 
> class may not only have virtual methods but also fields, clarify when class
> is not needed

Thanks for the polish! It sounds better.

> >   mention when class is not needed for either, then there is no need
> >   to have the class.
> > 
> > - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
> > 
> >   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
> >   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
> >   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
> >   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
> > 
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Daniel P. Berrang?" <berrange@redhat.com>
> > Cc: Eduardo Habkost <eduardo@habkost.net>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> > include/qom/object.h | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index 1d5b03372429..14f84ae454d3 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -249,7 +249,7 @@ struct Object
> >  * declared.
> >  *
> >  * This macro should be used unless the class struct needs to have
> > - * virtual methods declared.
> > + * virtual methods or fields declared.
> >  */
> > #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
> >     typedef struct InstanceType InstanceType; \
> > @@ -402,7 +402,8 @@ struct Object
> >  *
> >  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
> >  * the case of a non-abstract type, with interfaces, and with no requirement
> > - * for a class struct.
> > + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
> > + * then this is probably the right choice for defining it.
> 
> Is "probably" correct here? Is it a must or can still use other defining
> macros?

Yes, because there's another choice: OBJECT_DEFINE_SIMPLE_TYPE.

> If correct maybe saying "this might be the right choice" is simpler.

:-) I copied this sentence from the documentation of
OBJECT_DEFINE_SIMPLE_TYPE. The difference between these 2 macros is
OBJECT_DEFINE_SIMPLE_TYPE doesn't support interface.

So I'd like to keep this sentence, consistent with the description of
OBJECT_DEFINE_SIMPLE_TYPE.

Thanks,
Zhao

Re: [PATCH 2/9] docs/devel/qom: Fix the doc about OBJECT_DECLARE_SIMPLE_TYPE

[Zhao Liu]

On Wed, May 14, 2025 at 02:06:14PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 14:06:14 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 2/9] docs/devel/qom: Fix the doc about
>  OBJECT_DECLARE_SIMPLE_TYPE
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > Currently, the expansion example of OBJECT_DECLARE_SIMPLE_TYPE "roughly"
> > reflects what OBJECT_DECLARE_TYPE is doing.
> > 
> > Why "roughly"? Because this line -
> > 
> > >    G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> > 
> > - is also wrong for OBJECT_DECLARE_TYPE.
> > 
> > Fix the expansion example of OBJECT_DECLARE_SIMPLE_TYPE, especially
> > drop that definition of MyDeviceClass.
> > 
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> > Cc: Eduardo Habkost <eduardo@habkost.net>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> > docs/devel/qom.rst | 11 +----------
> > 1 file changed, 1 insertion(+), 10 deletions(-)
> > 
> > diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> > index 5870745ba27b..185f4c2f5921 100644
> > --- a/docs/devel/qom.rst
> > +++ b/docs/devel/qom.rst
> > @@ -326,21 +326,12 @@ This is equivalent to the following:
> >    :caption: Expansion from declaring a simple type
> > 
> >    typedef struct MyDevice MyDevice;
> > -   typedef struct MyDeviceClass MyDeviceClass;
> > 
> > -   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> > +   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
> > 
> > -   #define MY_DEVICE_GET_CLASS(void *obj) \
> > -           OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> > -   #define MY_DEVICE_CLASS(void *klass) \
> > -           OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> >    #define MY_DEVICE(void *obj)
> >            OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
> > 
> > -   struct MyDeviceClass {
> > -       DeviceClass parent_class;
> > -   };
> > -
> > The 'struct MyDevice' needs to be declared separately.
> > If the type requires virtual functions to be declared in the class
> > struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
> 
> Maybe you need to adjust the text here about OBJECT_DECLARE_TYPE here and
> show how to define Class sturct?

Then it's not easy to organize the structure in this document, since
most of the content is now make "OBJECT_DECLARE_SIMPLE_TYPE" as well as
"OBJECT_DEFINE_SIMPLE_TYPE" as examples... I'm a bit unsure, and we can
wait and see what others would say.

BTW, I found I missed this sentence:

"(OBJECT_DECLARE_TYPE) This does the same as OBJECT_DECLARE_SIMPLE_TYPE(),
but without the 'struct MyDeviceClass' definition."

It should be: This does the same as OBJECT_DECLARE_SIMPLE_TYPE(),
but with the class type.

Thanks,
Zhao

Re: [PATCH 4/9] hw/char/sh_serial: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

On Wed, May 14, 2025 at 02:07:41PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 14:07:41 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 4/9] hw/char/sh_serial: Consolidate
>  OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > The QOM type of SHSerialState is declared by OBJECT_DECLARE_SIMPLE_TYPE,
> > which means it doesn't need the class!
> > 
> > Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
> 
> Patch uses OBJECT_DEFINE_SIMPLE_TYPE not
> OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES so change commit message to match
> the patch.

Good catch! Thanks!

Regards,
Zhao

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

> > +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
> > +                                          s390_pv_guest,
> > +                                          S390_PV_GUEST,
> > +                                          CONFIDENTIAL_GUEST_SUPPORT,
> > +                                          { TYPE_USER_CREATABLE },
> > +                                          { NULL })
> 
> I'll note that existing callers of OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> happily ignore the line limit and put it into a single line.
> 
> (which ends up looking better IMHO)

Ok, I'll onor the existing conventions (which I'll apply to other
patches as well).

> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!

Regards,
Zhao

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

[BALATON Zoltan]

On Wed, 14 May 2025, Zhao Liu wrote:
> Hi BALATON,

In case you did not know:
https://en.wikipedia.org/wiki/Name_order#Hungary

> On Wed, May 14, 2025 at 02:02:48PM +0200, BALATON Zoltan wrote:
>> Date: Wed, 14 May 2025 14:02:48 +0200
>> From: BALATON Zoltan <balaton@eik.bme.hu>
>> Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
>>  simple type
>>
>> On Wed, 14 May 2025, Zhao Liu wrote:
>>> There're 2 changes:
>>> - For OBJECT_DECLARE_SIMPLE_TYPE:
>>>
>>>   Since a clase may not only has virtual method, but also field, thus
>>
>> class may not only have virtual methods but also fields, clarify when class
>> is not needed
>
> Thanks for the polish! It sounds better.

It's also grammatically more correct. But I'm not a native English speaker 
so may still be wrong.

>>>   mention when class is not needed for either, then there is no need
>>>   to have the class.
>>>
>>> - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
>>>
>>>   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
>>>   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
>>>   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
>>>   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: "Daniel P. Berrang?" <berrange@redhat.com>
>>> Cc: Eduardo Habkost <eduardo@habkost.net>
>>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
>>> ---
>>> include/qom/object.h | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>> index 1d5b03372429..14f84ae454d3 100644
>>> --- a/include/qom/object.h
>>> +++ b/include/qom/object.h
>>> @@ -249,7 +249,7 @@ struct Object
>>>  * declared.
>>>  *
>>>  * This macro should be used unless the class struct needs to have
>>> - * virtual methods declared.
>>> + * virtual methods or fields declared.
>>>  */
>>> #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
>>>     typedef struct InstanceType InstanceType; \
>>> @@ -402,7 +402,8 @@ struct Object
>>>  *
>>>  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
>>>  * the case of a non-abstract type, with interfaces, and with no requirement
>>> - * for a class struct.
>>> + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
>>> + * then this is probably the right choice for defining it.
>>
>> Is "probably" correct here? Is it a must or can still use other defining
>> macros?
>
> Yes, because there's another choice: OBJECT_DEFINE_SIMPLE_TYPE.
>
>> If correct maybe saying "this might be the right choice" is simpler.
>
> :-) I copied this sentence from the documentation of
> OBJECT_DEFINE_SIMPLE_TYPE. The difference between these 2 macros is
> OBJECT_DEFINE_SIMPLE_TYPE doesn't support interface.
>
> So I'd like to keep this sentence, consistent with the description of
> OBJECT_DEFINE_SIMPLE_TYPE.

OK in that case no problem to keep consistency. It still sounds a bit 
unspecific for documentation to me because it leaves the question open of 
what is it when it's not the right choice but if this is already there 
then that's another unrelated question.

Regards,
BALATON Zoltan

Re: [PATCH 2/9] docs/devel/qom: Fix the doc about OBJECT_DECLARE_SIMPLE_TYPE

[BALATON Zoltan]

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

On Wed, 14 May 2025, Zhao Liu wrote:
> On Wed, May 14, 2025 at 02:06:14PM +0200, BALATON Zoltan wrote:
>> Date: Wed, 14 May 2025 14:06:14 +0200
>> From: BALATON Zoltan <balaton@eik.bme.hu>
>> Subject: Re: [PATCH 2/9] docs/devel/qom: Fix the doc about
>>  OBJECT_DECLARE_SIMPLE_TYPE
>>
>> On Wed, 14 May 2025, Zhao Liu wrote:
>>> Currently, the expansion example of OBJECT_DECLARE_SIMPLE_TYPE "roughly"
>>> reflects what OBJECT_DECLARE_TYPE is doing.
>>>
>>> Why "roughly"? Because this line -
>>>
>>>>    G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
>>>
>>> - is also wrong for OBJECT_DECLARE_TYPE.
>>>
>>> Fix the expansion example of OBJECT_DECLARE_SIMPLE_TYPE, especially
>>> drop that definition of MyDeviceClass.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>>> Cc: Eduardo Habkost <eduardo@habkost.net>
>>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
>>> ---
>>> docs/devel/qom.rst | 11 +----------
>>> 1 file changed, 1 insertion(+), 10 deletions(-)
>>>
>>> diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
>>> index 5870745ba27b..185f4c2f5921 100644
>>> --- a/docs/devel/qom.rst
>>> +++ b/docs/devel/qom.rst
>>> @@ -326,21 +326,12 @@ This is equivalent to the following:
>>>    :caption: Expansion from declaring a simple type
>>>
>>>    typedef struct MyDevice MyDevice;
>>> -   typedef struct MyDeviceClass MyDeviceClass;
>>>
>>> -   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
>>> +   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
>>>
>>> -   #define MY_DEVICE_GET_CLASS(void *obj) \
>>> -           OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
>>> -   #define MY_DEVICE_CLASS(void *klass) \
>>> -           OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
>>>    #define MY_DEVICE(void *obj)
>>>            OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
>>>
>>> -   struct MyDeviceClass {
>>> -       DeviceClass parent_class;
>>> -   };
>>> -
>>> The 'struct MyDevice' needs to be declared separately.
>>> If the type requires virtual functions to be declared in the class
>>> struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
>>
>> Maybe you need to adjust the text here about OBJECT_DECLARE_TYPE here and
>> show how to define Class sturct?
>
> Then it's not easy to organize the structure in this document, since
> most of the content is now make "OBJECT_DECLARE_SIMPLE_TYPE" as well as
> "OBJECT_DEFINE_SIMPLE_TYPE" as examples... I'm a bit unsure, and we can
> wait and see what others would say.
>
> BTW, I found I missed this sentence:
>
> "(OBJECT_DECLARE_TYPE) This does the same as OBJECT_DECLARE_SIMPLE_TYPE(),
> but without the 'struct MyDeviceClass' definition."
>
> It should be: This does the same as OBJECT_DECLARE_SIMPLE_TYPE(),
> but with the class type.

Yes that's what I meant. If you remove the class example then how will 
readers know how to define that so a new example for that may be needed 
but you can wait for others' opinion too.

Regards,
BALATON Zoltan

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[BALATON Zoltan]

On Wed, 14 May 2025, Zhao Liu wrote:
>>> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
>>> +                                          s390_pv_guest,
>>> +                                          S390_PV_GUEST,
>>> +                                          CONFIDENTIAL_GUEST_SUPPORT,
>>> +                                          { TYPE_USER_CREATABLE },
>>> +                                          { NULL })
>>
>> I'll note that existing callers of OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
>> happily ignore the line limit and put it into a single line.
>>
>> (which ends up looking better IMHO)
>
> Ok, I'll onor the existing conventions (which I'll apply to other
> patches as well).

There are two line limits. If something is clearer on one line you could 
exceed the normal 80 chars and put up to 90 chars on one line for which 
checkpatch will issue a warning that can be ignored for these cases. Over 
90 lines checkpatch will give an error and I think you should not ignore 
that. Maybe try to put as much on one line as possible instead of new line 
after each argument but without exceeding the 80 chars or if the whole 
line fits in 90 chars then use that. Or maybe do not indent second line 
under ( but with 4 spaces then you can fit it in two lines but lines over 
90 chars are undesirable.

Regards,
BALATON Zoltan

>> Reviewed-by: David Hildenbrand <david@redhat.com>
>
> Thanks!
>
> Regards,
> Zhao
>
>
>

Re: [PATCH 9/9] ui/console-vc: Consolidate OBJECT_DEFINE_SIMPLE_TYPE

[Philippe Mathieu-Daudé]

On 14/5/25 10:49, Zhao Liu wrote:
> The QOM types of QemuTextConsole and QemuFixedTextConsole are declared
> by OBJECT_DECLARE_SIMPLE_TYPE, which means they don't need the class!
> 
> Therefore, use OBJECT_DEFINE_SIMPLE_TYPE to implement the type, then
> there's no need for class definition.
> 
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   ui/console-vc.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 7/9] hw/core/resetcontainer: Consolidate OBJECT_DECLARE_SIMPLE_TYPE

[Philippe Mathieu-Daudé]

On 14/5/25 10:49, Zhao Liu wrote:
> The QOM type of ResettableContainer is defined by
> OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES, which means it doesn't need
> the class!
> 
> Therefore, use OBJECT_DECLARE_SIMPLE_TYPE to declare the type, then
> there's no need for class definition.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   include/hw/core/resetcontainer.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

On Wed, May 14, 2025 at 06:24:03PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 18:24:03 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
>  OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > > > +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
> > > > +                                          s390_pv_guest,
> > > > +                                          S390_PV_GUEST,
> > > > +                                          CONFIDENTIAL_GUEST_SUPPORT,
> > > > +                                          { TYPE_USER_CREATABLE },
> > > > +                                          { NULL })
> > > 
> > > I'll note that existing callers of OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> > > happily ignore the line limit and put it into a single line.
> > > 
> > > (which ends up looking better IMHO)
> > 
> > Ok, I'll onor the existing conventions (which I'll apply to other
> > patches as well).
> 
> There are two line limits. If something is clearer on one line you could
> exceed the normal 80 chars and put up to 90 chars on one line for which
> checkpatch will issue a warning that can be ignored for these cases. Over 90
> lines checkpatch will give an error and I think you should not ignore that.

Thank you. This makes sense!

> Maybe try to put as much on one line as possible instead of new line after
> each argument but without exceeding the 80 chars or if the whole line fits
> in 90 chars then use that. Or maybe do not indent second line under ( but
> with 4 spaces then you can fit it in two lines but lines over 90 chars are
> undesirable.

HMM, I understand you mean:

OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest, s390_pv_guest,
    S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT, { TYPE_USER_CREATABLE }, { NULL })

The second line is 82 chars and now I think this version is better.

Thanks,
Zhao

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

[Zhao Liu]

On Wed, May 14, 2025 at 06:13:33PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 18:13:33 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
>  simple type
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > Hi BALATON,
> 
> In case you did not know:
> https://en.wikipedia.org/wiki/Name_order#Hungary
> 
> > On Wed, May 14, 2025 at 02:02:48PM +0200, BALATON Zoltan wrote:
> > > Date: Wed, 14 May 2025 14:02:48 +0200
> > > From: BALATON Zoltan <balaton@eik.bme.hu>
> > > Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
> > >  simple type
> > > 
> > > On Wed, 14 May 2025, Zhao Liu wrote:
> > > > There're 2 changes:
> > > > - For OBJECT_DECLARE_SIMPLE_TYPE:
> > > > 
> > > >   Since a clase may not only has virtual method, but also field, thus
> > > 
> > > class may not only have virtual methods but also fields, clarify when class
> > > is not needed
> > 
> > Thanks for the polish! It sounds better.
> 
> It's also grammatically more correct. But I'm not a native English speaker
> so may still be wrong.

Neither am I. But I like this advice.

> > > >   mention when class is not needed for either, then there is no need
> > > >   to have the class.
> > > > 
> > > > - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
> > > > 
> > > >   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
> > > >   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
> > > >   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
> > > >   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
> > > > 
> > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > Cc: "Daniel P. Berrang?" <berrange@redhat.com>
> > > > Cc: Eduardo Habkost <eduardo@habkost.net>
> > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > > > ---
> > > > include/qom/object.h | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/include/qom/object.h b/include/qom/object.h
> > > > index 1d5b03372429..14f84ae454d3 100644
> > > > --- a/include/qom/object.h
> > > > +++ b/include/qom/object.h
> > > > @@ -249,7 +249,7 @@ struct Object
> > > >  * declared.
> > > >  *
> > > >  * This macro should be used unless the class struct needs to have
> > > > - * virtual methods declared.
> > > > + * virtual methods or fields declared.
> > > >  */
> > > > #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
> > > >     typedef struct InstanceType InstanceType; \
> > > > @@ -402,7 +402,8 @@ struct Object
> > > >  *
> > > >  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
> > > >  * the case of a non-abstract type, with interfaces, and with no requirement
> > > > - * for a class struct.
> > > > + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
> > > > + * then this is probably the right choice for defining it.
> > > 
> > > Is "probably" correct here? Is it a must or can still use other defining
> > > macros?
> > 
> > Yes, because there's another choice: OBJECT_DEFINE_SIMPLE_TYPE.
> > 
> > > If correct maybe saying "this might be the right choice" is simpler.
> > 
> > :-) I copied this sentence from the documentation of
> > OBJECT_DEFINE_SIMPLE_TYPE. The difference between these 2 macros is
> > OBJECT_DEFINE_SIMPLE_TYPE doesn't support interface.
> > 
> > So I'd like to keep this sentence, consistent with the description of
> > OBJECT_DEFINE_SIMPLE_TYPE.
> 
> OK in that case no problem to keep consistency. It still sounds a bit
> unspecific for documentation to me because it leaves the question open of
> what is it when it's not the right choice but if this is already there then
> that's another unrelated question.

I get your point.

Before this sentance, the doc said "This is a variant of
OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for the common case of a
non-abstract type, without any interfaces, and with no requirement for a
class struct."

IMO it's the pretty accurate description about in which case this macro
is used. This description could clarify your question :-).

Thanks,
Zhao

Re: [PATCH 2/9] docs/devel/qom: Fix the doc about OBJECT_DECLARE_SIMPLE_TYPE

[Zhao Liu]

On Wed, May 14, 2025 at 06:16:31PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 18:16:31 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 2/9] docs/devel/qom: Fix the doc about
>  OBJECT_DECLARE_SIMPLE_TYPE
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > On Wed, May 14, 2025 at 02:06:14PM +0200, BALATON Zoltan wrote:
> > > Date: Wed, 14 May 2025 14:06:14 +0200
> > > From: BALATON Zoltan <balaton@eik.bme.hu>
> > > Subject: Re: [PATCH 2/9] docs/devel/qom: Fix the doc about
> > >  OBJECT_DECLARE_SIMPLE_TYPE
> > > 
> > > On Wed, 14 May 2025, Zhao Liu wrote:
> > > > Currently, the expansion example of OBJECT_DECLARE_SIMPLE_TYPE "roughly"
> > > > reflects what OBJECT_DECLARE_TYPE is doing.
> > > > 
> > > > Why "roughly"? Because this line -
> > > > 
> > > > >    G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> > > > 
> > > > - is also wrong for OBJECT_DECLARE_TYPE.
> > > > 
> > > > Fix the expansion example of OBJECT_DECLARE_SIMPLE_TYPE, especially
> > > > drop that definition of MyDeviceClass.
> > > > 
> > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > Cc: "Daniel P. Berrang?" <berrange@redhat.com>
> > > > Cc: Eduardo Habkost <eduardo@habkost.net>
> > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > > > ---
> > > > docs/devel/qom.rst | 11 +----------
> > > > 1 file changed, 1 insertion(+), 10 deletions(-)
> > > > 
> > > > diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> > > > index 5870745ba27b..185f4c2f5921 100644
> > > > --- a/docs/devel/qom.rst
> > > > +++ b/docs/devel/qom.rst
> > > > @@ -326,21 +326,12 @@ This is equivalent to the following:
> > > >    :caption: Expansion from declaring a simple type
> > > > 
> > > >    typedef struct MyDevice MyDevice;
> > > > -   typedef struct MyDeviceClass MyDeviceClass;
> > > > 
> > > > -   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> > > > +   G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
> > > > 
> > > > -   #define MY_DEVICE_GET_CLASS(void *obj) \
> > > > -           OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> > > > -   #define MY_DEVICE_CLASS(void *klass) \
> > > > -           OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> > > >    #define MY_DEVICE(void *obj)
> > > >            OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
> > > > 
> > > > -   struct MyDeviceClass {
> > > > -       DeviceClass parent_class;
> > > > -   };
> > > > -
> > > > The 'struct MyDevice' needs to be declared separately.
> > > > If the type requires virtual functions to be declared in the class
> > > > struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
> > > 
> > > Maybe you need to adjust the text here about OBJECT_DECLARE_TYPE here and
> > > show how to define Class sturct?
> > 
> > Then it's not easy to organize the structure in this document, since
> > most of the content is now make "OBJECT_DECLARE_SIMPLE_TYPE" as well as
> > "OBJECT_DEFINE_SIMPLE_TYPE" as examples... I'm a bit unsure, and we can
> > wait and see what others would say.
> > 
> > BTW, I found I missed this sentence:
> > 
> > "(OBJECT_DECLARE_TYPE) This does the same as OBJECT_DECLARE_SIMPLE_TYPE(),
> > but without the 'struct MyDeviceClass' definition."
> > 
> > It should be: This does the same as OBJECT_DECLARE_SIMPLE_TYPE(),
> > but with the class type.
> 
> Yes that's what I meant. If you remove the class example then how will
> readers know how to define that so a new example for that may be needed but
> you can wait for others' opinion too.

The new example deserves another separate patch. I'll think about how to
describe it.

Thanks,
Zhao

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[BALATON Zoltan]

On Thu, 15 May 2025, Zhao Liu wrote:
> On Wed, May 14, 2025 at 06:24:03PM +0200, BALATON Zoltan wrote:
>> Date: Wed, 14 May 2025 18:24:03 +0200
>> From: BALATON Zoltan <balaton@eik.bme.hu>
>> Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
>>  OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
>>
>> On Wed, 14 May 2025, Zhao Liu wrote:
>>>>> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
>>>>> +                                          s390_pv_guest,
>>>>> +                                          S390_PV_GUEST,
>>>>> +                                          CONFIDENTIAL_GUEST_SUPPORT,
>>>>> +                                          { TYPE_USER_CREATABLE },
>>>>> +                                          { NULL })
>>>>
>>>> I'll note that existing callers of OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
>>>> happily ignore the line limit and put it into a single line.
>>>>
>>>> (which ends up looking better IMHO)
>>>
>>> Ok, I'll onor the existing conventions (which I'll apply to other
>>> patches as well).
>>
>> There are two line limits. If something is clearer on one line you could
>> exceed the normal 80 chars and put up to 90 chars on one line for which
>> checkpatch will issue a warning that can be ignored for these cases. Over 90
>> lines checkpatch will give an error and I think you should not ignore that.
>
> Thank you. This makes sense!
>
>> Maybe try to put as much on one line as possible instead of new line after
>> each argument but without exceeding the 80 chars or if the whole line fits
>> in 90 chars then use that. Or maybe do not indent second line under ( but
>> with 4 spaces then you can fit it in two lines but lines over 90 chars are
>> undesirable.
>
> HMM, I understand you mean:
>
> OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest, s390_pv_guest,
>    S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT, { TYPE_USER_CREATABLE }, { NULL })
>
> The second line is 82 chars and now I think this version is better.

Yes and maybe can even fit in 80 chars if using { } instead of { NULL }.

Regards,
BALATON Zoltan

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Daniel P. Berrangé]

On Thu, May 15, 2025 at 05:41:40PM +0200, BALATON Zoltan wrote:
> On Thu, 15 May 2025, Zhao Liu wrote:
> > On Wed, May 14, 2025 at 06:24:03PM +0200, BALATON Zoltan wrote:
> > > Date: Wed, 14 May 2025 18:24:03 +0200
> > > From: BALATON Zoltan <balaton@eik.bme.hu>
> > > Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
> > >  OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> > > 
> > > On Wed, 14 May 2025, Zhao Liu wrote:
> > > > > > +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
> > > > > > +                                          s390_pv_guest,
> > > > > > +                                          S390_PV_GUEST,
> > > > > > +                                          CONFIDENTIAL_GUEST_SUPPORT,
> > > > > > +                                          { TYPE_USER_CREATABLE },
> > > > > > +                                          { NULL })
> > > > > 
> > > > > I'll note that existing callers of OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> > > > > happily ignore the line limit and put it into a single line.
> > > > > 
> > > > > (which ends up looking better IMHO)
> > > > 
> > > > Ok, I'll onor the existing conventions (which I'll apply to other
> > > > patches as well).
> > > 
> > > There are two line limits. If something is clearer on one line you could
> > > exceed the normal 80 chars and put up to 90 chars on one line for which
> > > checkpatch will issue a warning that can be ignored for these cases. Over 90
> > > lines checkpatch will give an error and I think you should not ignore that.
> > 
> > Thank you. This makes sense!
> > 
> > > Maybe try to put as much on one line as possible instead of new line after
> > > each argument but without exceeding the 80 chars or if the whole line fits
> > > in 90 chars then use that. Or maybe do not indent second line under ( but
> > > with 4 spaces then you can fit it in two lines but lines over 90 chars are
> > > undesirable.
> > 
> > HMM, I understand you mean:
> > 
> > OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest, s390_pv_guest,
> >    S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT, { TYPE_USER_CREATABLE }, { NULL })
> > 
> > The second line is 82 chars and now I think this version is better.
> 
> Yes and maybe can even fit in 80 chars if using { } instead of { NULL }.

Personally, once you have to break the line, I would be inclined
to have *nothing* after the '(' on the first line, and then break
at the list of interfaces. ie

 OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(
    S390PVGuest, s390_pv_guest, S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT,
    { TYPE_USER_CREATABLE }, { NULL })
 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Zhao Liu]

On Thu, May 15, 2025 at 05:11:56PM +0100, Daniel P. Berrangé wrote:
> Date: Thu, 15 May 2025 17:11:56 +0100
> From: "Daniel P. Berrangé" <berrange@redhat.com>
> Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
>  OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> 
> On Thu, May 15, 2025 at 05:41:40PM +0200, BALATON Zoltan wrote:
> > On Thu, 15 May 2025, Zhao Liu wrote:
> > > On Wed, May 14, 2025 at 06:24:03PM +0200, BALATON Zoltan wrote:
> > > > Date: Wed, 14 May 2025 18:24:03 +0200
> > > > From: BALATON Zoltan <balaton@eik.bme.hu>
> > > > Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
> > > >  OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> > > > 
> > > > On Wed, 14 May 2025, Zhao Liu wrote:
> > > > > > > +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
> > > > > > > +                                          s390_pv_guest,
> > > > > > > +                                          S390_PV_GUEST,
> > > > > > > +                                          CONFIDENTIAL_GUEST_SUPPORT,
> > > > > > > +                                          { TYPE_USER_CREATABLE },
> > > > > > > +                                          { NULL })
> > > > > > 
> > > > > > I'll note that existing callers of OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> > > > > > happily ignore the line limit and put it into a single line.
> > > > > > 
> > > > > > (which ends up looking better IMHO)
> > > > > 
> > > > > Ok, I'll onor the existing conventions (which I'll apply to other
> > > > > patches as well).
> > > > 
> > > > There are two line limits. If something is clearer on one line you could
> > > > exceed the normal 80 chars and put up to 90 chars on one line for which
> > > > checkpatch will issue a warning that can be ignored for these cases. Over 90
> > > > lines checkpatch will give an error and I think you should not ignore that.
> > > 
> > > Thank you. This makes sense!
> > > 
> > > > Maybe try to put as much on one line as possible instead of new line after
> > > > each argument but without exceeding the 80 chars or if the whole line fits
> > > > in 90 chars then use that. Or maybe do not indent second line under ( but
> > > > with 4 spaces then you can fit it in two lines but lines over 90 chars are
> > > > undesirable.
> > > 
> > > HMM, I understand you mean:
> > > 
> > > OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest, s390_pv_guest,
> > >    S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT, { TYPE_USER_CREATABLE }, { NULL })
> > > 
> > > The second line is 82 chars and now I think this version is better.
> > 
> > Yes and maybe can even fit in 80 chars if using { } instead of { NULL }.
> 
> Personally, once you have to break the line, I would be inclined
> to have *nothing* after the '(' on the first line, and then break
> at the list of interfaces. ie
> 
>  OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(
>     S390PVGuest, s390_pv_guest, S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT,
>     { TYPE_USER_CREATABLE }, { NULL })
>  

This version looks better! Thank you all for your patience.

(How to elegantly break lines is indeed a matter of taste, and I've
improved in this area now with your help :-).)

Regards,
Zhao

Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[David Hildenbrand]

On 16.05.25 05:23, Zhao Liu wrote:
> On Thu, May 15, 2025 at 05:11:56PM +0100, Daniel P. Berrangé wrote:
>> Date: Thu, 15 May 2025 17:11:56 +0100
>> From: "Daniel P. Berrangé" <berrange@redhat.com>
>> Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
>>   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
>>
>> On Thu, May 15, 2025 at 05:41:40PM +0200, BALATON Zoltan wrote:
>>> On Thu, 15 May 2025, Zhao Liu wrote:
>>>> On Wed, May 14, 2025 at 06:24:03PM +0200, BALATON Zoltan wrote:
>>>>> Date: Wed, 14 May 2025 18:24:03 +0200
>>>>> From: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
>>>>>   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
>>>>>
>>>>> On Wed, 14 May 2025, Zhao Liu wrote:
>>>>>>>> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
>>>>>>>> +                                          s390_pv_guest,
>>>>>>>> +                                          S390_PV_GUEST,
>>>>>>>> +                                          CONFIDENTIAL_GUEST_SUPPORT,
>>>>>>>> +                                          { TYPE_USER_CREATABLE },
>>>>>>>> +                                          { NULL })
>>>>>>>
>>>>>>> I'll note that existing callers of OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
>>>>>>> happily ignore the line limit and put it into a single line.
>>>>>>>
>>>>>>> (which ends up looking better IMHO)
>>>>>>
>>>>>> Ok, I'll onor the existing conventions (which I'll apply to other
>>>>>> patches as well).
>>>>>
>>>>> There are two line limits. If something is clearer on one line you could
>>>>> exceed the normal 80 chars and put up to 90 chars on one line for which
>>>>> checkpatch will issue a warning that can be ignored for these cases. Over 90
>>>>> lines checkpatch will give an error and I think you should not ignore that.
>>>>
>>>> Thank you. This makes sense!
>>>>
>>>>> Maybe try to put as much on one line as possible instead of new line after
>>>>> each argument but without exceeding the 80 chars or if the whole line fits
>>>>> in 90 chars then use that. Or maybe do not indent second line under ( but
>>>>> with 4 spaces then you can fit it in two lines but lines over 90 chars are
>>>>> undesirable.
>>>>
>>>> HMM, I understand you mean:
>>>>
>>>> OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest, s390_pv_guest,
>>>>     S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT, { TYPE_USER_CREATABLE }, { NULL })
>>>>
>>>> The second line is 82 chars and now I think this version is better.
>>>
>>> Yes and maybe can even fit in 80 chars if using { } instead of { NULL }.
>>
>> Personally, once you have to break the line, I would be inclined
>> to have *nothing* after the '(' on the first line, and then break
>> at the list of interfaces. ie
>>
>>   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(
>>      S390PVGuest, s390_pv_guest, S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT,
>>      { TYPE_USER_CREATABLE }, { NULL })
>>   
> 
> This version looks better! Thank you all for your patience.
> 
> (How to elegantly break lines is indeed a matter of taste, and I've
> improved in this area now with your help :-).)

Sorry for starting this :) And thanks for your patience.

Back when I added a OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES, I did it 
just like everybody else:

Cram it all into a single line because it's all unbelievable unreadable 
already, no need to fake that breaking lines would improve that in any 
way ...

-- 
Cheers,

David / dhildenb

Re: [PATCH 5/9] hw/hyperv/hv-balloon: Consolidate OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES

[Maciej S. Szmigiero]

On 14.05.2025 10:49, Zhao Liu wrote:
> The QOM type of HvBalloon is declared by OBJECT_DECLARE_SIMPLE_TYPE,
> which means it doesn't need the class!
> 
> Therefore, use OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES to implement
> the type, then there's no need for class definition.
> 
> Cc: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

Thanks,
Maciej

Re: [PATCH 0/9] qom: Consolidate OBJECT_{DEFINE|DECLARE}_SIMPLE_TYPE*

[Michael Tokarev]

On 14.05.2025 11:49, Zhao Liu wrote:
> Hi,
> 
> When I read the qom doc (qom.rst) about OBJECT_DECLARE_SIMPLE_TYPE, I
> found there's the wrong example. So I tried to fix the doc and cleaned
> up the relevant code case to correctly show that for simple type, the
> class definition is not needed.
> 
> This series tries to:
> 
>   * fix & improve the documentation of OBJECT_{DEFINE|DECLARE}_SIMPLE_TYPE*,
> 
>   * and clean up several cases that could have used simple type macros
>     (mainly by dropping unneeded class definitions).

What's the status of this series?  It looks like it needs a respin?

Thanks,

/mjt