| Message ID | 20240507064252.457884-4-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VFIO: misc cleanups | expand |
On 5/7/24 08:42, Zhenzhong Duan wrote: > Make VFIOIOMMUClass::attach_device() and its wrapper function > vfio_attach_device() return bool. > > This is to follow the coding standand to return bool if 'Error **' > is used to pass error. > > Suggested-by: Cédric Le Goater <clg@redhat.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > include/hw/vfio/vfio-common.h | 4 ++-- > include/hw/vfio/vfio-container-base.h | 4 ++-- > hw/vfio/ap.c | 6 ++---- > hw/vfio/ccw.c | 6 ++---- > hw/vfio/common.c | 4 ++-- > hw/vfio/container.c | 14 +++++++------- > hw/vfio/iommufd.c | 11 +++++------ > hw/vfio/pci.c | 5 ++--- > hw/vfio/platform.c | 7 +++---- > 9 files changed, 27 insertions(+), 34 deletions(-) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index b9da6c08ef..a7b6fc8f46 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -198,8 +198,8 @@ void vfio_region_exit(VFIORegion *region); > void vfio_region_finalize(VFIORegion *region); > void vfio_reset_handler(void *opaque); > struct vfio_device_info *vfio_get_device_info(int fd); > -int vfio_attach_device(char *name, VFIODevice *vbasedev, > - AddressSpace *as, Error **errp); > +bool vfio_attach_device(char *name, VFIODevice *vbasedev, > + AddressSpace *as, Error **errp); > void vfio_detach_device(VFIODevice *vbasedev); > > int vfio_kvm_device_add_fd(int fd, Error **errp); > diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h > index 3582d5f97a..c839cfd9cb 100644 > --- a/include/hw/vfio/vfio-container-base.h > +++ b/include/hw/vfio/vfio-container-base.h > @@ -118,8 +118,8 @@ struct VFIOIOMMUClass { > int (*dma_unmap)(const VFIOContainerBase *bcontainer, > hwaddr iova, ram_addr_t size, > IOMMUTLBEntry *iotlb); > - int (*attach_device)(const char *name, VFIODevice *vbasedev, > - AddressSpace *as, Error **errp); > + bool (*attach_device)(const char *name, VFIODevice *vbasedev, > + AddressSpace *as, Error **errp); > void (*detach_device)(VFIODevice *vbasedev); > /* migration feature */ > int (*set_dirty_page_tracking)(const VFIOContainerBase *bcontainer, > diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c > index 7c4caa5938..d50600b702 100644 > --- a/hw/vfio/ap.c > +++ b/hw/vfio/ap.c > @@ -156,7 +156,6 @@ static void vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev, > static void vfio_ap_realize(DeviceState *dev, Error **errp) > { > ERRP_GUARD(); > - int ret; > Error *err = NULL; > VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev); > VFIODevice *vbasedev = &vapdev->vdev; > @@ -165,9 +164,8 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) > return; > } > > - ret = vfio_attach_device(vbasedev->name, vbasedev, > - &address_space_memory, errp); > - if (ret) { > + if (!vfio_attach_device(vbasedev->name, vbasedev, > + &address_space_memory, errp)) { > goto error; > } > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index 90e4a53437..782bd4bed7 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -580,7 +580,6 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) > S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev); > VFIODevice *vbasedev = &vcdev->vdev; > Error *err = NULL; > - int ret; > > /* Call the class init function for subchannel. */ > if (cdc->realize) { > @@ -594,9 +593,8 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) > return; > } > > - ret = vfio_attach_device(cdev->mdevid, vbasedev, > - &address_space_memory, errp); > - if (ret) { > + if (!vfio_attach_device(cdev->mdevid, vbasedev, > + &address_space_memory, errp)) { > goto out_attach_dev_err; > } > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 8f9cbdc026..890d30910e 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -1492,8 +1492,8 @@ retry: > return info; > } > > -int vfio_attach_device(char *name, VFIODevice *vbasedev, > - AddressSpace *as, Error **errp) > +bool vfio_attach_device(char *name, VFIODevice *vbasedev, > + AddressSpace *as, Error **errp) > { > const VFIOIOMMUClass *ops = > VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY)); This is still broken. No need to resend. I will update the code. Thanks, C. > diff --git a/hw/vfio/container.c b/hw/vfio/container.c > index 77bdec276e..ea3b145913 100644 > --- a/hw/vfio/container.c > +++ b/hw/vfio/container.c > @@ -908,8 +908,8 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp) > * @name and @vbasedev->name are likely to be different depending > * on the type of the device, hence the need for passing @name > */ > -static int vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, > - AddressSpace *as, Error **errp) > +static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, > + AddressSpace *as, Error **errp) > { > int groupid = vfio_device_groupid(vbasedev, errp); > VFIODevice *vbasedev_iter; > @@ -918,27 +918,27 @@ static int vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, > int ret; > > if (groupid < 0) { > - return groupid; > + return false; > } > > trace_vfio_attach_device(vbasedev->name, groupid); > > group = vfio_get_group(groupid, as, errp); > if (!group) { > - return -ENOENT; > + return false; > } > > QLIST_FOREACH(vbasedev_iter, &group->device_list, next) { > if (strcmp(vbasedev_iter->name, vbasedev->name) == 0) { > error_setg(errp, "device is already attached"); > vfio_put_group(group); > - return -EBUSY; > + return false; > } > } > ret = vfio_get_device(group, name, vbasedev, errp); > if (ret) { > vfio_put_group(group); > - return ret; > + return false; > } > > bcontainer = &group->container->bcontainer; > @@ -946,7 +946,7 @@ static int vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, > QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next); > QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next); > > - return ret; > + return true; > } > > static void vfio_legacy_detach_device(VFIODevice *vbasedev) > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c > index c644127972..4c6992fca1 100644 > --- a/hw/vfio/iommufd.c > +++ b/hw/vfio/iommufd.c > @@ -299,8 +299,8 @@ error: > return ret; > } > > -static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, > - AddressSpace *as, Error **errp) > +static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, > + AddressSpace *as, Error **errp) > { > VFIOContainerBase *bcontainer; > VFIOIOMMUFDContainer *container; > @@ -315,7 +315,7 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, > if (vbasedev->fd < 0) { > devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp); > if (devfd < 0) { > - return devfd; > + return false; > } > vbasedev->fd = devfd; > } else { > @@ -392,7 +392,6 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, > memory_listener_register(&bcontainer->listener, bcontainer->space->as); > > if (bcontainer->error) { > - ret = -1; > error_propagate_prepend(errp, bcontainer->error, > "memory listener initialization failed: "); > goto err_listener_register; > @@ -431,7 +430,7 @@ found_container: > > trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev->num_irqs, > vbasedev->num_regions, vbasedev->flags); > - return 0; > + return true; > > err_listener_register: > iommufd_cdev_ram_block_discard_disable(false); > @@ -444,7 +443,7 @@ err_alloc_ioas: > iommufd_cdev_unbind_and_disconnect(vbasedev); > err_connect_bind: > close(vbasedev->fd); > - return ret; > + return false; > } > > static void iommufd_cdev_detach(VFIODevice *vbasedev) > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 576b21e2bb..d3beb15514 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -3001,9 +3001,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) > name = g_strdup(vbasedev->name); > } > > - ret = vfio_attach_device(name, vbasedev, > - pci_device_iommu_address_space(pdev), errp); > - if (ret) { > + if (!vfio_attach_device(name, vbasedev, > + pci_device_iommu_address_space(pdev), errp)) { > goto error; > } > > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c > index dcd2365fb3..2bd16096bb 100644 > --- a/hw/vfio/platform.c > +++ b/hw/vfio/platform.c > @@ -552,10 +552,9 @@ static int vfio_base_device_init(VFIODevice *vbasedev, Error **errp) > return ret; > } > > - ret = vfio_attach_device(vbasedev->name, vbasedev, > - &address_space_memory, errp); > - if (ret) { > - return ret; > + if (!vfio_attach_device(vbasedev->name, vbasedev, > + &address_space_memory, errp)) { > + return -EINVAL; > } > > ret = vfio_populate_device(vbasedev, errp);
>-----Original Message----- >From: Cédric Le Goater <clg@redhat.com> >Subject: Re: [PATCH v2 03/11] vfio: Make VFIOIOMMUClass::attach_device() >and its wrapper return bool > >On 5/7/24 08:42, Zhenzhong Duan wrote: >> Make VFIOIOMMUClass::attach_device() and its wrapper function >> vfio_attach_device() return bool. >> >> This is to follow the coding standand to return bool if 'Error **' >> is used to pass error. >> >> Suggested-by: Cédric Le Goater <clg@redhat.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> include/hw/vfio/vfio-common.h | 4 ++-- >> include/hw/vfio/vfio-container-base.h | 4 ++-- >> hw/vfio/ap.c | 6 ++---- >> hw/vfio/ccw.c | 6 ++---- >> hw/vfio/common.c | 4 ++-- >> hw/vfio/container.c | 14 +++++++------- >> hw/vfio/iommufd.c | 11 +++++------ >> hw/vfio/pci.c | 5 ++--- >> hw/vfio/platform.c | 7 +++---- >> 9 files changed, 27 insertions(+), 34 deletions(-) >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >common.h >> index b9da6c08ef..a7b6fc8f46 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -198,8 +198,8 @@ void vfio_region_exit(VFIORegion *region); >> void vfio_region_finalize(VFIORegion *region); >> void vfio_reset_handler(void *opaque); >> struct vfio_device_info *vfio_get_device_info(int fd); >> -int vfio_attach_device(char *name, VFIODevice *vbasedev, >> - AddressSpace *as, Error **errp); >> +bool vfio_attach_device(char *name, VFIODevice *vbasedev, >> + AddressSpace *as, Error **errp); >> void vfio_detach_device(VFIODevice *vbasedev); >> >> int vfio_kvm_device_add_fd(int fd, Error **errp); >> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio- >container-base.h >> index 3582d5f97a..c839cfd9cb 100644 >> --- a/include/hw/vfio/vfio-container-base.h >> +++ b/include/hw/vfio/vfio-container-base.h >> @@ -118,8 +118,8 @@ struct VFIOIOMMUClass { >> int (*dma_unmap)(const VFIOContainerBase *bcontainer, >> hwaddr iova, ram_addr_t size, >> IOMMUTLBEntry *iotlb); >> - int (*attach_device)(const char *name, VFIODevice *vbasedev, >> - AddressSpace *as, Error **errp); >> + bool (*attach_device)(const char *name, VFIODevice *vbasedev, >> + AddressSpace *as, Error **errp); >> void (*detach_device)(VFIODevice *vbasedev); >> /* migration feature */ >> int (*set_dirty_page_tracking)(const VFIOContainerBase *bcontainer, >> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c >> index 7c4caa5938..d50600b702 100644 >> --- a/hw/vfio/ap.c >> +++ b/hw/vfio/ap.c >> @@ -156,7 +156,6 @@ static void >vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev, >> static void vfio_ap_realize(DeviceState *dev, Error **errp) >> { >> ERRP_GUARD(); >> - int ret; >> Error *err = NULL; >> VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev); >> VFIODevice *vbasedev = &vapdev->vdev; >> @@ -165,9 +164,8 @@ static void vfio_ap_realize(DeviceState *dev, Error >**errp) >> return; >> } >> >> - ret = vfio_attach_device(vbasedev->name, vbasedev, >> - &address_space_memory, errp); >> - if (ret) { >> + if (!vfio_attach_device(vbasedev->name, vbasedev, >> + &address_space_memory, errp)) { >> goto error; >> } >> >> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c >> index 90e4a53437..782bd4bed7 100644 >> --- a/hw/vfio/ccw.c >> +++ b/hw/vfio/ccw.c >> @@ -580,7 +580,6 @@ static void vfio_ccw_realize(DeviceState *dev, >Error **errp) >> S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev); >> VFIODevice *vbasedev = &vcdev->vdev; >> Error *err = NULL; >> - int ret; >> >> /* Call the class init function for subchannel. */ >> if (cdc->realize) { >> @@ -594,9 +593,8 @@ static void vfio_ccw_realize(DeviceState *dev, >Error **errp) >> return; >> } >> >> - ret = vfio_attach_device(cdev->mdevid, vbasedev, >> - &address_space_memory, errp); >> - if (ret) { >> + if (!vfio_attach_device(cdev->mdevid, vbasedev, >> + &address_space_memory, errp)) { >> goto out_attach_dev_err; >> } >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index 8f9cbdc026..890d30910e 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -1492,8 +1492,8 @@ retry: >> return info; >> } >> >> -int vfio_attach_device(char *name, VFIODevice *vbasedev, >> - AddressSpace *as, Error **errp) >> +bool vfio_attach_device(char *name, VFIODevice *vbasedev, >> + AddressSpace *as, Error **errp) >> { >> const VFIOIOMMUClass *ops = >> >VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY)); > >This is still broken. No need to resend. I will update the code. I put this series before preq v4, so you don't see that change. See https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_nesting_rfcv2_preq_v4_has_vfio_cleanup/ That change is in https://github.com/yiliu1765/qemu/commit/0bec499456a9aa5a079ed2335ce1581d79e2850d Thanks Zhenzhong
On 5/7/24 09:34, Duan, Zhenzhong wrote: > > >> -----Original Message----- >> From: Cédric Le Goater <clg@redhat.com> >> Subject: Re: [PATCH v2 03/11] vfio: Make VFIOIOMMUClass::attach_device() >> and its wrapper return bool >> >> On 5/7/24 08:42, Zhenzhong Duan wrote: >>> Make VFIOIOMMUClass::attach_device() and its wrapper function >>> vfio_attach_device() return bool. >>> >>> This is to follow the coding standand to return bool if 'Error **' >>> is used to pass error. >>> >>> Suggested-by: Cédric Le Goater <clg@redhat.com> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>> --- >>> include/hw/vfio/vfio-common.h | 4 ++-- >>> include/hw/vfio/vfio-container-base.h | 4 ++-- >>> hw/vfio/ap.c | 6 ++---- >>> hw/vfio/ccw.c | 6 ++---- >>> hw/vfio/common.c | 4 ++-- >>> hw/vfio/container.c | 14 +++++++------- >>> hw/vfio/iommufd.c | 11 +++++------ >>> hw/vfio/pci.c | 5 ++--- >>> hw/vfio/platform.c | 7 +++---- >>> 9 files changed, 27 insertions(+), 34 deletions(-) >>> >>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >> common.h >>> index b9da6c08ef..a7b6fc8f46 100644 >>> --- a/include/hw/vfio/vfio-common.h >>> +++ b/include/hw/vfio/vfio-common.h >>> @@ -198,8 +198,8 @@ void vfio_region_exit(VFIORegion *region); >>> void vfio_region_finalize(VFIORegion *region); >>> void vfio_reset_handler(void *opaque); >>> struct vfio_device_info *vfio_get_device_info(int fd); >>> -int vfio_attach_device(char *name, VFIODevice *vbasedev, >>> - AddressSpace *as, Error **errp); >>> +bool vfio_attach_device(char *name, VFIODevice *vbasedev, >>> + AddressSpace *as, Error **errp); >>> void vfio_detach_device(VFIODevice *vbasedev); >>> >>> int vfio_kvm_device_add_fd(int fd, Error **errp); >>> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio- >> container-base.h >>> index 3582d5f97a..c839cfd9cb 100644 >>> --- a/include/hw/vfio/vfio-container-base.h >>> +++ b/include/hw/vfio/vfio-container-base.h >>> @@ -118,8 +118,8 @@ struct VFIOIOMMUClass { >>> int (*dma_unmap)(const VFIOContainerBase *bcontainer, >>> hwaddr iova, ram_addr_t size, >>> IOMMUTLBEntry *iotlb); >>> - int (*attach_device)(const char *name, VFIODevice *vbasedev, >>> - AddressSpace *as, Error **errp); >>> + bool (*attach_device)(const char *name, VFIODevice *vbasedev, >>> + AddressSpace *as, Error **errp); >>> void (*detach_device)(VFIODevice *vbasedev); >>> /* migration feature */ >>> int (*set_dirty_page_tracking)(const VFIOContainerBase *bcontainer, >>> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c >>> index 7c4caa5938..d50600b702 100644 >>> --- a/hw/vfio/ap.c >>> +++ b/hw/vfio/ap.c >>> @@ -156,7 +156,6 @@ static void >> vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev, >>> static void vfio_ap_realize(DeviceState *dev, Error **errp) >>> { >>> ERRP_GUARD(); >>> - int ret; >>> Error *err = NULL; >>> VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev); >>> VFIODevice *vbasedev = &vapdev->vdev; >>> @@ -165,9 +164,8 @@ static void vfio_ap_realize(DeviceState *dev, Error >> **errp) >>> return; >>> } >>> >>> - ret = vfio_attach_device(vbasedev->name, vbasedev, >>> - &address_space_memory, errp); >>> - if (ret) { >>> + if (!vfio_attach_device(vbasedev->name, vbasedev, >>> + &address_space_memory, errp)) { >>> goto error; >>> } >>> >>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c >>> index 90e4a53437..782bd4bed7 100644 >>> --- a/hw/vfio/ccw.c >>> +++ b/hw/vfio/ccw.c >>> @@ -580,7 +580,6 @@ static void vfio_ccw_realize(DeviceState *dev, >> Error **errp) >>> S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev); >>> VFIODevice *vbasedev = &vcdev->vdev; >>> Error *err = NULL; >>> - int ret; >>> >>> /* Call the class init function for subchannel. */ >>> if (cdc->realize) { >>> @@ -594,9 +593,8 @@ static void vfio_ccw_realize(DeviceState *dev, >> Error **errp) >>> return; >>> } >>> >>> - ret = vfio_attach_device(cdev->mdevid, vbasedev, >>> - &address_space_memory, errp); >>> - if (ret) { >>> + if (!vfio_attach_device(cdev->mdevid, vbasedev, >>> + &address_space_memory, errp)) { >>> goto out_attach_dev_err; >>> } >>> >>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >>> index 8f9cbdc026..890d30910e 100644 >>> --- a/hw/vfio/common.c >>> +++ b/hw/vfio/common.c >>> @@ -1492,8 +1492,8 @@ retry: >>> return info; >>> } >>> >>> -int vfio_attach_device(char *name, VFIODevice *vbasedev, >>> - AddressSpace *as, Error **errp) >>> +bool vfio_attach_device(char *name, VFIODevice *vbasedev, >>> + AddressSpace *as, Error **errp) >>> { >>> const VFIOIOMMUClass *ops = >>> >> VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY)); >> >> This is still broken. No need to resend. I will update the code. > > I put this series before preq v4, so you don't see that change. > See https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_nesting_rfcv2_preq_v4_has_vfio_cleanup/ > > That change is in https://github.com/yiliu1765/qemu/commit/0bec499456a9aa5a079ed2335ce1581d79e2850d OK looks good. Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C.
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index b9da6c08ef..a7b6fc8f46 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -198,8 +198,8 @@ void vfio_region_exit(VFIORegion *region); void vfio_region_finalize(VFIORegion *region); void vfio_reset_handler(void *opaque); struct vfio_device_info *vfio_get_device_info(int fd); -int vfio_attach_device(char *name, VFIODevice *vbasedev, - AddressSpace *as, Error **errp); +bool vfio_attach_device(char *name, VFIODevice *vbasedev, + AddressSpace *as, Error **errp); void vfio_detach_device(VFIODevice *vbasedev); int vfio_kvm_device_add_fd(int fd, Error **errp); diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h index 3582d5f97a..c839cfd9cb 100644 --- a/include/hw/vfio/vfio-container-base.h +++ b/include/hw/vfio/vfio-container-base.h @@ -118,8 +118,8 @@ struct VFIOIOMMUClass { int (*dma_unmap)(const VFIOContainerBase *bcontainer, hwaddr iova, ram_addr_t size, IOMMUTLBEntry *iotlb); - int (*attach_device)(const char *name, VFIODevice *vbasedev, - AddressSpace *as, Error **errp); + bool (*attach_device)(const char *name, VFIODevice *vbasedev, + AddressSpace *as, Error **errp); void (*detach_device)(VFIODevice *vbasedev); /* migration feature */ int (*set_dirty_page_tracking)(const VFIOContainerBase *bcontainer, diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c index 7c4caa5938..d50600b702 100644 --- a/hw/vfio/ap.c +++ b/hw/vfio/ap.c @@ -156,7 +156,6 @@ static void vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev, static void vfio_ap_realize(DeviceState *dev, Error **errp) { ERRP_GUARD(); - int ret; Error *err = NULL; VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev); VFIODevice *vbasedev = &vapdev->vdev; @@ -165,9 +164,8 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) return; } - ret = vfio_attach_device(vbasedev->name, vbasedev, - &address_space_memory, errp); - if (ret) { + if (!vfio_attach_device(vbasedev->name, vbasedev, + &address_space_memory, errp)) { goto error; } diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index 90e4a53437..782bd4bed7 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -580,7 +580,6 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev); VFIODevice *vbasedev = &vcdev->vdev; Error *err = NULL; - int ret; /* Call the class init function for subchannel. */ if (cdc->realize) { @@ -594,9 +593,8 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) return; } - ret = vfio_attach_device(cdev->mdevid, vbasedev, - &address_space_memory, errp); - if (ret) { + if (!vfio_attach_device(cdev->mdevid, vbasedev, + &address_space_memory, errp)) { goto out_attach_dev_err; } diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 8f9cbdc026..890d30910e 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -1492,8 +1492,8 @@ retry: return info; } -int vfio_attach_device(char *name, VFIODevice *vbasedev, - AddressSpace *as, Error **errp) +bool vfio_attach_device(char *name, VFIODevice *vbasedev, + AddressSpace *as, Error **errp) { const VFIOIOMMUClass *ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY)); diff --git a/hw/vfio/container.c b/hw/vfio/container.c index 77bdec276e..ea3b145913 100644 --- a/hw/vfio/container.c +++ b/hw/vfio/container.c @@ -908,8 +908,8 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp) * @name and @vbasedev->name are likely to be different depending * on the type of the device, hence the need for passing @name */ -static int vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, - AddressSpace *as, Error **errp) +static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, + AddressSpace *as, Error **errp) { int groupid = vfio_device_groupid(vbasedev, errp); VFIODevice *vbasedev_iter; @@ -918,27 +918,27 @@ static int vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, int ret; if (groupid < 0) { - return groupid; + return false; } trace_vfio_attach_device(vbasedev->name, groupid); group = vfio_get_group(groupid, as, errp); if (!group) { - return -ENOENT; + return false; } QLIST_FOREACH(vbasedev_iter, &group->device_list, next) { if (strcmp(vbasedev_iter->name, vbasedev->name) == 0) { error_setg(errp, "device is already attached"); vfio_put_group(group); - return -EBUSY; + return false; } } ret = vfio_get_device(group, name, vbasedev, errp); if (ret) { vfio_put_group(group); - return ret; + return false; } bcontainer = &group->container->bcontainer; @@ -946,7 +946,7 @@ static int vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev, QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next); QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next); - return ret; + return true; } static void vfio_legacy_detach_device(VFIODevice *vbasedev) diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c index c644127972..4c6992fca1 100644 --- a/hw/vfio/iommufd.c +++ b/hw/vfio/iommufd.c @@ -299,8 +299,8 @@ error: return ret; } -static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, - AddressSpace *as, Error **errp) +static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, + AddressSpace *as, Error **errp) { VFIOContainerBase *bcontainer; VFIOIOMMUFDContainer *container; @@ -315,7 +315,7 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, if (vbasedev->fd < 0) { devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp); if (devfd < 0) { - return devfd; + return false; } vbasedev->fd = devfd; } else { @@ -392,7 +392,6 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, memory_listener_register(&bcontainer->listener, bcontainer->space->as); if (bcontainer->error) { - ret = -1; error_propagate_prepend(errp, bcontainer->error, "memory listener initialization failed: "); goto err_listener_register; @@ -431,7 +430,7 @@ found_container: trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev->num_irqs, vbasedev->num_regions, vbasedev->flags); - return 0; + return true; err_listener_register: iommufd_cdev_ram_block_discard_disable(false); @@ -444,7 +443,7 @@ err_alloc_ioas: iommufd_cdev_unbind_and_disconnect(vbasedev); err_connect_bind: close(vbasedev->fd); - return ret; + return false; } static void iommufd_cdev_detach(VFIODevice *vbasedev) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 576b21e2bb..d3beb15514 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -3001,9 +3001,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) name = g_strdup(vbasedev->name); } - ret = vfio_attach_device(name, vbasedev, - pci_device_iommu_address_space(pdev), errp); - if (ret) { + if (!vfio_attach_device(name, vbasedev, + pci_device_iommu_address_space(pdev), errp)) { goto error; } diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index dcd2365fb3..2bd16096bb 100644 --- a/hw/vfio/platform.c +++ b/hw/vfio/platform.c @@ -552,10 +552,9 @@ static int vfio_base_device_init(VFIODevice *vbasedev, Error **errp) return ret; } - ret = vfio_attach_device(vbasedev->name, vbasedev, - &address_space_memory, errp); - if (ret) { - return ret; + if (!vfio_attach_device(vbasedev->name, vbasedev, + &address_space_memory, errp)) { + return -EINVAL; } ret = vfio_populate_device(vbasedev, errp);
Make VFIOIOMMUClass::attach_device() and its wrapper function vfio_attach_device() return bool. This is to follow the coding standand to return bool if 'Error **' is used to pass error. Suggested-by: Cédric Le Goater <clg@redhat.com> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- include/hw/vfio/vfio-common.h | 4 ++-- include/hw/vfio/vfio-container-base.h | 4 ++-- hw/vfio/ap.c | 6 ++---- hw/vfio/ccw.c | 6 ++---- hw/vfio/common.c | 4 ++-- hw/vfio/container.c | 14 +++++++------- hw/vfio/iommufd.c | 11 +++++------ hw/vfio/pci.c | 5 ++--- hw/vfio/platform.c | 7 +++---- 9 files changed, 27 insertions(+), 34 deletions(-)