>-----Original Message-----
>From: Cédric Le Goater <c...@redhat.com>
>Subject: Re: [PATCH 02/16] vfio/display: Make vfio_display_*() return bool
>
>On 5/15/24 10:20, Zhenzhong Duan wrote:
>> This is to follow the coding standand in qapi/error.h to return bool
>> for bool-valued functions.
>>
>> Suggested-by: Cédric Le Goater <c...@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>
>
>Reviewed-by: Cédric Le Goater <c...@redhat.com>
>
>One comment below,
>
>> ---
>> hw/vfio/pci.h | 2 +-
>> hw/vfio/display.c | 20 ++++++++++----------
>> hw/vfio/pci.c | 3 +--
>> 3 files changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>> index 92cd62d115..a5ac9efd4b 100644
>> --- a/hw/vfio/pci.h
>> +++ b/hw/vfio/pci.h
>> @@ -232,7 +232,7 @@ int vfio_pci_igd_opregion_init(VFIOPCIDevice
>*vdev,
>> Error **errp);
>>
>> void vfio_display_reset(VFIOPCIDevice *vdev);
>> -int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
>> +bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
>> void vfio_display_finalize(VFIOPCIDevice *vdev);
>>
>> extern const VMStateDescription vfio_display_vmstate;
>> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
>> index 57c5ae0b2a..b562f4be74 100644
>> --- a/hw/vfio/display.c
>> +++ b/hw/vfio/display.c
>> @@ -346,11 +346,11 @@ static const GraphicHwOps
>vfio_display_dmabuf_ops = {
>> .ui_info = vfio_display_edid_ui_info,
>> };
>>
>> -static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>> +static bool vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>> {
>> if (!display_opengl) {
>> error_setg(errp, "vfio-display-dmabuf: opengl not available");
>> - return -1;
>> + return false;
>> }
>>
>> vdev->dpy = g_new0(VFIODisplay, 1);
>> @@ -360,11 +360,11 @@ static int
>vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>> if (vdev->enable_ramfb) {
>> vdev->dpy->ramfb = ramfb_setup(errp);
>> if (!vdev->dpy->ramfb) {
>> - return -EINVAL;
>> + return false;
>> }
>> }
>> vfio_display_edid_init(vdev);
>
>vfio_display_edid_init() can fail for many reasons and does it silently.
>It would be good to report the error in a future patch.
Yes, that deserve a fix. Will address it with a future patch.
Thanks
Zhenzhong
