| Message ID | 20230703071510.160712-6-zhenzhong.duan@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | VFIO migration related refactor and bug fix | expand |
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 */
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 */
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 */
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(-)