Re: [Qemu-devel] [PATCH v4 13/17] vfio/platform: Pass an error object to vfio_base_device_init

[Markus Armbruster <armbru@redhat.com>]

Eric Auger <eric.auger@redhat.com> writes:

> This patch propagates errors encountered during vfio_base_device_init
> up to the realize function.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v3: creation
> ---
>  hw/vfio/platform.c | 49 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 484e31f..fc06b45 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -541,13 +541,14 @@ static VFIODeviceOps vfio_platform_ops = {
>  /**
>   * vfio_base_device_init - perform preliminary VFIO setup
>   * @vbasedev: the VFIO device handle
> + * @errp: error object
>   *
>   * Implement the VFIO command sequence that allows to discover
>   * assigned device resources: group extraction, device
>   * fd retrieval, resource query.
>   * Precondition: the device name must be initialized
>   */
> -static int vfio_base_device_init(VFIODevice *vbasedev)
> +static int vfio_base_device_init(VFIODevice *vbasedev, Error **errp)
>  {
>      VFIOGroup *group;
>      VFIODevice *vbasedev_iter;
> @@ -555,7 +556,6 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
>      ssize_t len;
>      struct stat st;
>      int groupid;
> -    Error *err = NULL;
>      int ret;
>  
>      /* @sysfsdev takes precedence over @host */
> @@ -564,6 +564,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
>          vbasedev->name = g_strdup(basename(vbasedev->sysfsdev));
>      } else {
>          if (!vbasedev->name || strchr(vbasedev->name, '/')) {
> +            error_setg(errp, "wrong host device name");

Error is no longer silent.  Worth mentioning in the commit message?

>              return -EINVAL;
>          }
>  
> @@ -572,8 +573,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
>      }
>  
>      if (stat(vbasedev->sysfsdev, &st) < 0) {
> -        error_report("vfio: error: no such host device: %s",
> -                     vbasedev->sysfsdev);
> +        error_setg_errno(errp, errno, "no such host device");

"no such host device" is correct and now redundant when errno is ENOENT,
and misleading when it isn't.  Suggest to rephrase the message.

>          return -errno;
>      }
>  
[...]