Re: [PATCH v2 03/11] vfio: Make VFIOIOMMUClass::attach_device() and its wrapper return bool
On 5/7/24 09:34, Duan, Zhenzhong wrote: -Original Message- From: Cédric Le Goater 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 Signed-off-by: Zhenzhong Duan --- 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 = >vdev; @@ -165,9 +164,8 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) return; } -ret = vfio_attach_device(vbasedev->name, vbasedev, - _space_memory, errp); -if (ret) { +if (!vfio_attach_device(vbasedev->name, vbasedev, +_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 = >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, - _space_memory, errp); -if (ret) { +if (!vfio_attach_device(cdev->mdevid, vbasedev, +_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/ Tha
RE: [PATCH v2 03/11] vfio: Make VFIOIOMMUClass::attach_device() and its wrapper return bool
>-Original Message- >From: Cédric Le Goater >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 >> Signed-off-by: Zhenzhong Duan >> --- >> 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 = >vdev; >> @@ -165,9 +164,8 @@ static void vfio_ap_realize(DeviceState *dev, Error >**errp) >> return; >> } >> >> -ret = vfio_attach_device(vbasedev->name, vbasedev, >> - _space_memory, errp); >> -if (ret) { >> +if (!vfio_attach_device(vbasedev->name, vbasedev, >> +_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 = >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, >> - _space_memory, errp); >> -if (ret) { >> +if (!vfio_attach_device(cdev->mdevid, vbasedev, >> +_space_memory, errp)) { &g
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 Signed-off-by: Zhenzhong Duan --- 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 = >vdev; @@ -165,9 +164,8 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) return; } -ret = vfio_attach_device(vbasedev->name, vbasedev, - _space_memory, errp); -if (ret) { +if (!vfio_attach_device(vbasedev->name, vbasedev, +_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 = >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, - _space_memory, errp); -if (ret) { +if (!vfio_attach_device(cdev->mdevid, vbasedev, +_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