1. Patchew
  2. QEMU
  3. [PATCH v6 0/7] VFIO migration related refactor and bug fix
  4. View patch

[PATCH v6 7/7] vfio/migration: Return bool type for vfio_migration_realize()

Zhenzhong Duan posted 7 patches 2 years, 7 months ago
Only 5 patches received!
[PATCH v6 7/7] vfio/migration: Return bool type for vfio_migration_realize()
Posted by Zhenzhong Duan 2 years, 7 months ago 
Make vfio_migration_realize() adhere to the convention of other realize()
callbacks(like qdev_realize) by returning bool instead of int.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Suggested-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/migration.c           | 15 ++++++++++-----
 hw/vfio/pci.c                 |  3 +--
 include/hw/vfio/vfio-common.h |  2 +-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index e3954570c853..2674f4bc472d 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -846,7 +846,12 @@ void vfio_reset_bytes_transferred(void)
     bytes_transferred = 0;
 }
 
-int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
+/*
+ * Return true when either migration initialized or blocker registered.
+ * Currently only return false when adding blocker fails which will
+ * de-register vfio device.
+ */
+bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
 {
     Error *err = NULL;
     int ret;
@@ -854,7 +859,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
     if (vbasedev->enable_migration == ON_OFF_AUTO_OFF) {
         error_setg(&err, "%s: Migration is disabled for VFIO device",
                    vbasedev->name);
-        return vfio_block_migration(vbasedev, err, errp);
+        return !vfio_block_migration(vbasedev, err, errp);
     }
 
     ret = vfio_migration_init(vbasedev);
@@ -869,7 +874,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
                        vbasedev->name, ret, strerror(-ret));
         }
 
-        return vfio_block_migration(vbasedev, err, errp);
+        return !vfio_block_migration(vbasedev, err, errp);
     }
 
     if (!vbasedev->dirty_pages_supported) {
@@ -896,7 +901,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
     }
 
     trace_vfio_migration_realize(vbasedev->name);
-    return 0;
+    return true;
 
 add_blocker:
     ret = vfio_block_migration(vbasedev, err, errp);
@@ -904,7 +909,7 @@ out_deinit:
     if (ret) {
         vfio_migration_deinit(vbasedev);
     }
-    return ret;
+    return !ret;
 }
 
 void vfio_migration_exit(VFIODevice *vbasedev)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index eefd4ec330d9..68dd99283620 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3207,8 +3207,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     }
 
     if (!pdev->failover_pair_id) {
-        ret = vfio_migration_realize(vbasedev, errp);
-        if (ret) {
+        if (!vfio_migration_realize(vbasedev, errp)) {
             goto out_deregister;
         }
     }
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 45167c8a8a54..da43d273524e 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -252,7 +252,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
 int vfio_spapr_remove_window(VFIOContainer *container,
                              hwaddr offset_within_address_space);
 
-int vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
+bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
 void vfio_migration_exit(VFIODevice *vbasedev);
 
 #endif /* HW_VFIO_VFIO_COMMON_H */
-- 
2.34.1
Re: [PATCH v6 7/7] vfio/migration: Return bool type for vfio_migration_realize()
Posted by Cédric Le Goater 2 years, 7 months ago 
On 7/3/23 09:15, Zhenzhong Duan wrote:
> Make vfio_migration_realize() adhere to the convention of other realize()
> callbacks(like qdev_realize) by returning bool instead of int.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Suggested-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


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

Thanks,

C.


> ---
>   hw/vfio/migration.c           | 15 ++++++++++-----
>   hw/vfio/pci.c                 |  3 +--
>   include/hw/vfio/vfio-common.h |  2 +-
>   3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index e3954570c853..2674f4bc472d 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -846,7 +846,12 @@ void vfio_reset_bytes_transferred(void)
>       bytes_transferred = 0;
>   }
>   
> -int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
> +/*
> + * Return true when either migration initialized or blocker registered.
> + * Currently only return false when adding blocker fails which will
> + * de-register vfio device.
> + */
> +bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>   {
>       Error *err = NULL;
>       int ret;
> @@ -854,7 +859,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>       if (vbasedev->enable_migration == ON_OFF_AUTO_OFF) {
>           error_setg(&err, "%s: Migration is disabled for VFIO device",
>                      vbasedev->name);
> -        return vfio_block_migration(vbasedev, err, errp);
> +        return !vfio_block_migration(vbasedev, err, errp);
>       }
>   
>       ret = vfio_migration_init(vbasedev);
> @@ -869,7 +874,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>                          vbasedev->name, ret, strerror(-ret));
>           }
>   
> -        return vfio_block_migration(vbasedev, err, errp);
> +        return !vfio_block_migration(vbasedev, err, errp);
>       }
>   
>       if (!vbasedev->dirty_pages_supported) {
> @@ -896,7 +901,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>       }
>   
>       trace_vfio_migration_realize(vbasedev->name);
> -    return 0;
> +    return true;
>   
>   add_blocker:
>       ret = vfio_block_migration(vbasedev, err, errp);
> @@ -904,7 +909,7 @@ out_deinit:
>       if (ret) {
>           vfio_migration_deinit(vbasedev);
>       }
> -    return ret;
> +    return !ret;
>   }
>   
>   void vfio_migration_exit(VFIODevice *vbasedev)
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index eefd4ec330d9..68dd99283620 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3207,8 +3207,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>       }
>   
>       if (!pdev->failover_pair_id) {
> -        ret = vfio_migration_realize(vbasedev, errp);
> -        if (ret) {
> +        if (!vfio_migration_realize(vbasedev, errp)) {
>               goto out_deregister;
>           }
>       }
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 45167c8a8a54..da43d273524e 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -252,7 +252,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
>   int vfio_spapr_remove_window(VFIOContainer *container,
>                                hwaddr offset_within_address_space);
>   
> -int vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
> +bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
>   void vfio_migration_exit(VFIODevice *vbasedev);
>   
>   #endif /* HW_VFIO_VFIO_COMMON_H */
Re: [PATCH v6 7/7] vfio/migration: Return bool type for vfio_migration_realize()
Posted by Joao Martins 2 years, 7 months ago 

On 03/07/2023 08:15, Zhenzhong Duan wrote:
> Make vfio_migration_realize() adhere to the convention of other realize()
> callbacks(like qdev_realize) by returning bool instead of int.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Suggested-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

> ---
>  hw/vfio/migration.c           | 15 ++++++++++-----
>  hw/vfio/pci.c                 |  3 +--
>  include/hw/vfio/vfio-common.h |  2 +-
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index e3954570c853..2674f4bc472d 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -846,7 +846,12 @@ void vfio_reset_bytes_transferred(void)
>      bytes_transferred = 0;
>  }
>  
> -int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
> +/*
> + * Return true when either migration initialized or blocker registered.
> + * Currently only return false when adding blocker fails which will
> + * de-register vfio device.
> + */
> +bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>  {
>      Error *err = NULL;
>      int ret;
> @@ -854,7 +859,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>      if (vbasedev->enable_migration == ON_OFF_AUTO_OFF) {
>          error_setg(&err, "%s: Migration is disabled for VFIO device",
>                     vbasedev->name);
> -        return vfio_block_migration(vbasedev, err, errp);
> +        return !vfio_block_migration(vbasedev, err, errp);
>      }
>  
>      ret = vfio_migration_init(vbasedev);
> @@ -869,7 +874,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>                         vbasedev->name, ret, strerror(-ret));
>          }
>  
> -        return vfio_block_migration(vbasedev, err, errp);
> +        return !vfio_block_migration(vbasedev, err, errp);
>      }
>  
>      if (!vbasedev->dirty_pages_supported) {
> @@ -896,7 +901,7 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>      }
>  
>      trace_vfio_migration_realize(vbasedev->name);
> -    return 0;
> +    return true;
>  
>  add_blocker:
>      ret = vfio_block_migration(vbasedev, err, errp);
> @@ -904,7 +909,7 @@ out_deinit:
>      if (ret) {
>          vfio_migration_deinit(vbasedev);
>      }
> -    return ret;
> +    return !ret;
>  }
>  
>  void vfio_migration_exit(VFIODevice *vbasedev)
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index eefd4ec330d9..68dd99283620 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3207,8 +3207,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>      }
>  
>      if (!pdev->failover_pair_id) {
> -        ret = vfio_migration_realize(vbasedev, errp);
> -        if (ret) {
> +        if (!vfio_migration_realize(vbasedev, errp)) {
>              goto out_deregister;
>          }
>      }
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 45167c8a8a54..da43d273524e 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -252,7 +252,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
>  int vfio_spapr_remove_window(VFIOContainer *container,
>                               hwaddr offset_within_address_space);
>  
> -int vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
> +bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
>  void vfio_migration_exit(VFIODevice *vbasedev);
>  
>  #endif /* HW_VFIO_VFIO_COMMON_H */

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.