Re: [Qemu-devel] [PATCH v4 14/17] vfio/pci: Conversion to realize

[Markus Armbruster <armbru@redhat.com>]

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

> This patch converts VFIO PCI to realize function.
>
> Also original initfn errors now are propagated using QEMU
> error objects. All errors are formatted with the same pattern:
> "vfio: %s: the error description"
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
> v2 -> v3:
> - use errp directly in all cases
>
> v1 -> v2:
> - correct error_setg_errno with positive error values
> ---
>  hw/vfio/pci.c        | 68 ++++++++++++++++++++++------------------------------
>  hw/vfio/trace-events |  2 +-
>  2 files changed, 29 insertions(+), 41 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 40ff4a7..b316e13 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2513,13 +2513,12 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
>      vdev->req_enabled = false;
>  }
>  
> -static int vfio_initfn(PCIDevice *pdev)
> +static void vfio_realize(PCIDevice *pdev, Error **errp)
>  {
>      VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
>      VFIODevice *vbasedev_iter;
>      VFIOGroup *group;
>      char *tmp, group_path[PATH_MAX], *group_name;
> -    Error *err = NULL;
>      ssize_t len;
>      struct stat st;
>      int groupid;
> @@ -2533,9 +2532,9 @@ static int vfio_initfn(PCIDevice *pdev)
>      }
>  
>      if (stat(vdev->vbasedev.sysfsdev, &st) < 0) {
> -        error_setg_errno(&err, errno, "no such host device");
> -        ret = -errno;
> -        goto error;
> +        error_setg_errno(errp, errno, "no such host device");
> +        error_prepend(errp, ERR_PREFIX, vdev->vbasedev.sysfsdev);
> +        return;
>      }
>  
>      vdev->vbasedev.name = g_strdup(basename(vdev->vbasedev.sysfsdev));
> @@ -2547,8 +2546,8 @@ static int vfio_initfn(PCIDevice *pdev)
>      g_free(tmp);
>  
>      if (len <= 0 || len >= sizeof(group_path)) {
> -        ret = len < 0 ? -errno : -ENAMETOOLONG;
> -        error_setg_errno(&err, -ret, "no iommu_group found");
> +        error_setg_errno(errp, len < 0 ? errno : ENAMETOOLONG,
> +                         "no iommu_group found");
>          goto error;
>      }
>  
> @@ -2556,36 +2555,33 @@ static int vfio_initfn(PCIDevice *pdev)
>  
>      group_name = basename(group_path);
>      if (sscanf(group_name, "%d", &groupid) != 1) {
> -        error_setg_errno(&err, errno, "failed to read %s", group_path);
> -        ret = -errno;
> +        error_setg_errno(errp, errno, "failed to read %s", group_path);
>          goto error;
>      }
>  
> -    trace_vfio_initfn(vdev->vbasedev.name, groupid);
> +    trace_vfio_realize(vdev->vbasedev.name, groupid);
>  
> -    group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), &err);
> +    group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), errp);
>      if (!group) {
> -        ret = -ENOENT;
>          goto error;
>      }
>  
>      QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
>          if (strcmp(vbasedev_iter->name, vdev->vbasedev.name) == 0) {
> -            error_setg(&err, "device is already attached");
> +            error_setg(errp, "device is already attached");
>              vfio_put_group(group);
> -            ret = -EBUSY;
>              goto error;
>          }
>      }
>  
> -    ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev, &err);
> +    ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev, errp);
>      if (ret) {
>          vfio_put_group(group);
>          goto error;
>      }
>  
> -    ret = vfio_populate_device(vdev, &err);
> -    if (err) {
> +    ret = vfio_populate_device(vdev, errp);
> +    if (ret) {

The if (err) comes from PATCH 03.  You could reduce churn by checking
ret from the start.  More of the same below.  Your choice.

>          goto error;
>      }
>  
[...]