vfio: improve use of QOM and coding guidelines

[PATCH 01/22] vfio/vfio-container-base.h: update VFIOContainerBase declaration

Posted by Mark Cave-Ayland 4 months ago

Update the VFIOContainerBase declaration to match our current coding
guidelines: remove the explicit typedef (this is already handled by the
OBJECT_DECLARE_TYPE() macro), add a blank line after the parent object,
rename parent to parent_obj, and move the macro declaration next to the
VFIOContainerBase struct declaration.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 include/hw/vfio/vfio-container-base.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index bded6e993f..acbd48a18a 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -33,8 +33,9 @@ typedef struct VFIOAddressSpace {
 /*
  * This is the base object for vfio container backends
  */
-typedef struct VFIOContainerBase {
-    Object parent;
+struct VFIOContainerBase {
+    Object parent_obj;
+
     VFIOAddressSpace *space;
     MemoryListener listener;
     Error *error;
@@ -51,7 +52,10 @@ typedef struct VFIOContainerBase {
     QLIST_HEAD(, VFIODevice) device_list;
     GList *iova_ranges;
     NotifierWithReturn cpr_reboot_notifier;
-} VFIOContainerBase;
+};
+
+#define TYPE_VFIO_IOMMU "vfio-iommu"
+OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
 
 typedef struct VFIOGuestIOMMU {
     VFIOContainerBase *bcontainer;
@@ -105,14 +109,11 @@ vfio_container_get_page_size_mask(const VFIOContainerBase *bcontainer)
     return bcontainer->pgsizes;
 }
 
-#define TYPE_VFIO_IOMMU "vfio-iommu"
 #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
 #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
 #define TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
 #define TYPE_VFIO_IOMMU_USER TYPE_VFIO_IOMMU "-user"
 
-OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
-
 struct VFIOIOMMUClass {
     ObjectClass parent_class;
 
-- 
2.43.0

Re: [PATCH 01/22] vfio/vfio-container-base.h: update VFIOContainerBase declaration

Posted by Cédric Le Goater 3 months ago

One more thing we could do :

> @@ -105,14 +109,11 @@ vfio_container_get_page_size_mask(const VFIOContainerBase *bcontainer)
>       return bcontainer->pgsizes;
>   }
>   
> -#define TYPE_VFIO_IOMMU "vfio-iommu"
>   #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>   #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
>   #define TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
>   #define TYPE_VFIO_IOMMU_USER TYPE_VFIO_IOMMU "-user"

These type definitions would be better placed in the header files
defining the other software structures to which they relate.

   TYPE_VFIO_IOMMU_LEGACY  -> hw/vfio/vfio-container.h
   TYPE_VFIO_IOMMU_SPAPR   -> hw/vfio/vfio-container.h ?
   TYPE_VFIO_IOMMU_IOMMUFD -> hw/vfio/vfio-iommufd.h
   TYPE_VFIO_IOMMU_USER    -> hw/vfio-user/container.h

I don't know how possible that would be.

Thanks,

C.

Re: [PATCH 01/22] vfio/vfio-container-base.h: update VFIOContainerBase declaration

Posted by Mark Cave-Ayland 2 months, 2 weeks ago

On 11/08/2025 12:56, Cédric Le Goater wrote:

> One more thing we could do :
> 
>> @@ -105,14 +109,11 @@ vfio_container_get_page_size_mask(const 
>> VFIOContainerBase *bcontainer)
>>       return bcontainer->pgsizes;
>>   }
>> -#define TYPE_VFIO_IOMMU "vfio-iommu"
>>   #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>>   #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
>>   #define TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
>>   #define TYPE_VFIO_IOMMU_USER TYPE_VFIO_IOMMU "-user"
> 
> These type definitions would be better placed in the header files
> defining the other software structures to which they relate.
> 
>    TYPE_VFIO_IOMMU_LEGACY  -> hw/vfio/vfio-container.h
>    TYPE_VFIO_IOMMU_SPAPR   -> hw/vfio/vfio-container.h ?
>    TYPE_VFIO_IOMMU_IOMMUFD -> hw/vfio/vfio-iommufd.h
>    TYPE_VFIO_IOMMU_USER    -> hw/vfio-user/container.h
> 
> I don't know how possible that would be.

I can have a look at how feasible this is if you like? This pattern 
tends to be used a lot elsewhere and feels more intuitive to me.


ATB,

Mark.

Re: [PATCH 01/22] vfio/vfio-container-base.h: update VFIOContainerBase declaration

Posted by Cédric Le Goater 3 months ago

On 7/15/25 11:25, Mark Cave-Ayland wrote:
> Update the VFIOContainerBase declaration to match our current coding
> guidelines: remove the explicit typedef (this is already handled by the
> OBJECT_DECLARE_TYPE() macro), add a blank line after the parent object,
> rename parent to parent_obj, and move the macro declaration next to the
> VFIOContainerBase struct declaration.
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/vfio/vfio-container-base.h | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index bded6e993f..acbd48a18a 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -33,8 +33,9 @@ typedef struct VFIOAddressSpace {
>   /*
>    * This is the base object for vfio container backends
>    */
> -typedef struct VFIOContainerBase {
> -    Object parent;
> +struct VFIOContainerBase {
> +    Object parent_obj;
> +
>       VFIOAddressSpace *space;
>       MemoryListener listener;
>       Error *error;
> @@ -51,7 +52,10 @@ typedef struct VFIOContainerBase {
>       QLIST_HEAD(, VFIODevice) device_list;
>       GList *iova_ranges;
>       NotifierWithReturn cpr_reboot_notifier;
> -} VFIOContainerBase;
> +};
> +
> +#define TYPE_VFIO_IOMMU "vfio-iommu"
> +OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
>   
>   typedef struct VFIOGuestIOMMU {
>       VFIOContainerBase *bcontainer;
> @@ -105,14 +109,11 @@ vfio_container_get_page_size_mask(const VFIOContainerBase *bcontainer)
>       return bcontainer->pgsizes;
>   }
>   
> -#define TYPE_VFIO_IOMMU "vfio-iommu"
>   #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>   #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
>   #define TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
>   #define TYPE_VFIO_IOMMU_USER TYPE_VFIO_IOMMU "-user"
>   
> -OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
> -
>   struct VFIOIOMMUClass {
>       ObjectClass parent_class;
>