Re: [PATCH v2 12/19] util/vfio-helpers: Let qemu_vfio_do_mapping() propagate Error

[Auger Eric]

Hi Philippe,

On 10/26/20 11:54 AM, Philippe Mathieu-Daudé wrote:
> Pass qemu_vfio_do_mapping() an Error* argument so it can propagate
> any error to callers. Replace error_report() which only report
> to the monitor by the more generic error_setg_errno().
> 
> Reviewed-by: Fam Zheng <fam@euphon.net>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  util/vfio-helpers.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> index c03fe0b7156..2c4598d7faa 100644
> --- a/util/vfio-helpers.c
> +++ b/util/vfio-helpers.c
> @@ -609,7 +609,7 @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState 
> *s,
>  
>  /* Do the DMA mapping with VFIO. */
>  static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size,
> -                                uint64_t iova)
> +                                uint64_t iova, Error **errp)
>  {
>      struct vfio_iommu_type1_dma_map dma_map = {
>          .argsz = sizeof(dma_map),
> @@ -621,7 +621,7 @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void 
> *host, size_t size,
>      trace_qemu_vfio_do_mapping(s, host, iova, size);
>  
>      if (ioctl(s->container, VFIO_IOMMU_MAP_DMA, &dma_map)) {
> -        error_report("VFIO_MAP_DMA failed: %s", strerror(errno));
> +        error_setg_errno(errp, errno, "VFIO_MAP_DMA failed");
>          return -errno;
>      }
>      return 0;
> @@ -757,7 +757,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, 
> size_t size,
There are other error cases in qemu_vfio_dma_map() where you must set
errp. Otherwise the caller may, in the future test the returned value
and incorrectly handle the errp.

Thanks

Eric
>                  goto out;
>              }
>              assert(qemu_vfio_verify_mappings(s));
> -            ret = qemu_vfio_do_mapping(s, host, size, iova0);
> +            ret = qemu_vfio_do_mapping(s, host, size, iova0, errp);
>              if (ret) {
>                  qemu_vfio_undo_mapping(s, mapping, NULL);
>                  goto out;
> @@ -768,7 +768,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, 
> size_t size,
>                  ret = -ENOMEM;
>                  goto out;
>              }
> -            ret = qemu_vfio_do_mapping(s, host, size, iova0);
> +            ret = qemu_vfio_do_mapping(s, host, size, iova0, errp);
>              if (ret) {
>                  goto out;
>              }
>