s390x/ccw: Error reporting cleanups

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

Posted by Cédric Le Goater 1 year, 8 months ago

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>
---
 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;
         }
     }
-- 
2.45.1

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

Posted by Anthony Krowiak 1 year, 8 months ago

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;
>           }
>       }

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

Posted by Duan, Zhenzhong 1 year, 8 months ago


>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: [PATCH 4/7] s390x/css: Make S390CCWDeviceClass::realize return
>bool
>
>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: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>---
> 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..2e0a70998132070996d6b
>0d083b8ddba5b9b87dc 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..3c0975055089c3629dd76
>ce2e1484a4ef66d8d41 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..9a8e052711fe2f7c067c
>52808b2af30d0ebfee0c 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;
>         }
>     }
>--
>2.45.1

[PATCH 1/7] hw/s390x/ccw: Make s390_ccw_get_dev_info() return a bool
[PATCH 2/7] s390x/css: Make CCWDeviceClass::realize return bool
[PATCH 3/7] hw/s390x/ccw: Remove local Error variable from s390_ccw_realize()
[PATCH 4/7] s390x/css: Make S390CCWDeviceClass::realize return bool
[PATCH 5/7] vfio/ccw: Use the 'Error **errp' argument of vfio_ccw_realize()
[PATCH 6/7] vfio/ccw: Fix the missed unrealize() call in error path
[PATCH 7/7] vfio/{ap, ccw}: Use warn_report_err() for IRQ notifier registration errors