| Message ID | 20240522170107.289532-5-clg@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x/ccw: Error reporting cleanups | expand |
>-----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
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; > } > }
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; } }
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(-)