Re: [PATCH 4/7] s390x/css: Make S390CCWDeviceClass::realize return bool

[Anthony Krowiak]

On 5/22/24 1:01 PM, Cédric Le Goater wrote:
> Since the realize() handler of S390CCWDeviceClass takes an 'Error **'
> argument, best practices suggest to return a bool. See the api/error.h
> Rules section. While at it, modify the call in vfio_ccw_realize().
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>


Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>


> ---
>   include/hw/s390x/s390-ccw.h | 2 +-
>   hw/s390x/s390-ccw.c         | 7 ++++---
>   hw/vfio/ccw.c               | 3 +--
>   3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/s390x/s390-ccw.h b/include/hw/s390x/s390-ccw.h
> index 
> 2c807ee3a1ae8d85460fe65be8a62c64f212fe4b..2e0a70998132070996d6b0d083b8ddba5b9b87dc
>  100644
> --- a/include/hw/s390x/s390-ccw.h
> +++ b/include/hw/s390x/s390-ccw.h
> @@ -31,7 +31,7 @@ struct S390CCWDevice {
>   
>   struct S390CCWDeviceClass {
>       CCWDeviceClass parent_class;
> -    void (*realize)(S390CCWDevice *dev, char *sysfsdev, Error **errp);
> +    bool (*realize)(S390CCWDevice *dev, char *sysfsdev, Error **errp);
>       void (*unrealize)(S390CCWDevice *dev);
>       IOInstEnding (*handle_request) (SubchDev *sch);
>       int (*handle_halt) (SubchDev *sch);
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index 
> b3d14c61d732880a651edcf28a040ca723cb9f5b..3c0975055089c3629dd76ce2e1484a4ef66d8d41
>  100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -108,7 +108,7 @@ static bool s390_ccw_get_dev_info(S390CCWDevice *cdev,
>       return true;
>   }
>   
> -static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
> +static bool s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
>   {
>       CcwDevice *ccw_dev = CCW_DEVICE(cdev);
>       CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
> @@ -117,7 +117,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
> *sysfsdev, Error **errp)
>       int ret;
>   
>       if (!s390_ccw_get_dev_info(cdev, sysfsdev, errp)) {
> -        return;
> +        return false;
>       }
>   
>       sch = css_create_sch(ccw_dev->devno, errp);
> @@ -142,7 +142,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
> *sysfsdev, Error **errp)
>   
>       css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
>                             parent->hotplugged, 1);
> -    return;
> +    return true;
>   
>   out_err:
>       css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
> @@ -150,6 +150,7 @@ out_err:
>       g_free(sch);
>   out_mdevid_free:
>       g_free(cdev->mdevid);
> +    return false;
>   }
>   
>   static void s390_ccw_unrealize(S390CCWDevice *cdev)
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 
> 2600e62e37238779800dc2b3a0bd315d7633017b..9a8e052711fe2f7c067c52808b2af30d0ebfee0c
>  100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -582,8 +582,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
>   
>       /* Call the class init function for subchannel. */
>       if (cdc->realize) {
> -        cdc->realize(cdev, vcdev->vdev.sysfsdev, &err);
> -        if (err) {
> +        if (!cdc->realize(cdev, vcdev->vdev.sysfsdev, &err)) {
>               goto out_err_propagate;
>           }
>       }