Re: [PATCH 03/18] vfio/common: Fix wrong %m usages

[Cédric Le Goater]

On 1/26/23 19:49, Avihai Horon wrote:
> There are several places where the %m conversion is used if one of
> vfio_dma_map(), vfio_dma_unmap() or vfio_get_dirty_bitmap() fail.
>
> The %m usage in these places is wrong since %m relies on errno value while
> the above functions don't report errors via errno.
>
> Fix it by using strerror() with the returned value instead.
>
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/vfio/common.c | 29 ++++++++++++++++-------------
>   1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index d892609cf1..643418f6f1 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -656,17 +656,17 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
> IOMMUTLBEntry *iotlb)
>                              read_only);
>           if (ret) {
>               error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
> -                         "0x%"HWADDR_PRIx", %p) = %d (%m)",
> +                         "0x%"HWADDR_PRIx", %p) = %d (%s)",
>                            container, iova,
> -                         iotlb->addr_mask + 1, vaddr, ret);
> +                         iotlb->addr_mask + 1, vaddr, ret, strerror(-ret));
>           }
>       } else {
>           ret = vfio_dma_unmap(container, iova, iotlb->addr_mask + 1, iotlb);
>           if (ret) {
>               error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
> -                         "0x%"HWADDR_PRIx") = %d (%m)",
> +                         "0x%"HWADDR_PRIx") = %d (%s)",
>                            container, iova,
> -                         iotlb->addr_mask + 1, ret);
> +                         iotlb->addr_mask + 1, ret, strerror(-ret));
>           }
>       }
>   out:
> @@ -1048,8 +1048,9 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>                          vaddr, section->readonly);
>       if (ret) {
>           error_setg(&err, "vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
> -                   "0x%"HWADDR_PRIx", %p) = %d (%m)",
> -                   container, iova, int128_get64(llsize), vaddr, ret);
> +                   "0x%"HWADDR_PRIx", %p) = %d (%s)",
> +                   container, iova, int128_get64(llsize), vaddr, ret,
> +                   strerror(-ret));
>           if (memory_region_is_ram_device(section->mr)) {
>               /* Allow unexpected mappings not to be fatal for RAM devices */
>               error_report_err(err);
> @@ -1181,16 +1182,18 @@ static void vfio_listener_region_del(MemoryListener 
> *listener,
>               ret = vfio_dma_unmap(container, iova, int128_get64(llsize), NULL);
>               if (ret) {
>                   error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
> -                             "0x%"HWADDR_PRIx") = %d (%m)",
> -                             container, iova, int128_get64(llsize), ret);
> +                             "0x%"HWADDR_PRIx") = %d (%s)",
> +                             container, iova, int128_get64(llsize), ret,
> +                             strerror(-ret));
>               }
>               iova += int128_get64(llsize);
>           }
>           ret = vfio_dma_unmap(container, iova, int128_get64(llsize), NULL);
>           if (ret) {
>               error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
> -                         "0x%"HWADDR_PRIx") = %d (%m)",
> -                         container, iova, int128_get64(llsize), ret);
> +                         "0x%"HWADDR_PRIx") = %d (%s)",
> +                         container, iova, int128_get64(llsize), ret,
> +                         strerror(-ret));
>           }
>       }
>   
> @@ -1337,9 +1340,9 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>                                       translated_addr);
>           if (ret) {
>               error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
> -                         "0x%"HWADDR_PRIx") = %d (%m)",
> -                         container, iova,
> -                         iotlb->addr_mask + 1, ret);
> +                         "0x%"HWADDR_PRIx") = %d (%s)",
> +                         container, iova, iotlb->addr_mask + 1, ret,
> +                         strerror(-ret));
>           }
>       }
>       rcu_read_unlock();