diff mbox series

[v2,08/11] util/vfio-helpers: Use error_setg in qemu_vfio_find_[fixed/temp]_iova

Message ID 20210826195014.2180369-9-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series block/nvme: Rework error reporting | expand

Commit Message

Philippe Mathieu-Daudé Aug. 26, 2021, 7:50 p.m. UTC
Have qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova()
propagate eventual errors to callers.

Suggested-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 util/vfio-helpers.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Klaus Jensen Aug. 27, 2021, 5:57 a.m. UTC | #1
On Aug 26 21:50, Philippe Mathieu-Daudé wrote:
> Have qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova()
> propagate eventual errors to callers.
> 
> Suggested-by: Klaus Jensen <k.jensen@samsung.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  util/vfio-helpers.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> index 306b5a83e48..7de5081dbd3 100644
> --- a/util/vfio-helpers.c
> +++ b/util/vfio-helpers.c
> @@ -678,7 +678,8 @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)
>  }
>  
>  static int
> -qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
> +qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova,
> +                          Error **errp)
>  {
>      int i;
>  
> @@ -696,11 +697,14 @@ qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
>              return 0;
>          }
>      }
> +    error_setg(errp, "fixed iova range not found");
> +
>      return -ENOMEM;
>  }
>  
>  static int
> -qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
> +qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova,
> +                         Error **errp)
>  {
>      int i;
>  
> @@ -718,6 +722,8 @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
>              return 0;
>          }
>      }
> +    error_setg(errp, "temporary iova range not found");
> +
>      return -ENOMEM;
>  }
>  
> @@ -762,7 +768,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
>              goto out;
>          }
>          if (!temporary) {
> -            if (qemu_vfio_find_fixed_iova(s, size, &iova0)) {
> +            if (qemu_vfio_find_fixed_iova(s, size, &iova0, errp) < 0) {
>                  ret = -ENOMEM;
>                  goto out;
>              }
> @@ -776,7 +782,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
>              }
>              qemu_vfio_dump_mappings(s);
>          } else {
> -            if (qemu_vfio_find_temp_iova(s, size, &iova0)) {
> +            if (qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
>                  ret = -ENOMEM;
>                  goto out;
>              }

Not related to this patch, but it is slightly confusing that these
functions actually already return a negative errno, but then we
overwrite it.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Philippe Mathieu-Daudé Aug. 27, 2021, 6:25 a.m. UTC | #2
On 8/27/21 7:57 AM, Klaus Jensen wrote:
> On Aug 26 21:50, Philippe Mathieu-Daudé wrote:
>> Have qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova()
>> propagate eventual errors to callers.
>>
>> Suggested-by: Klaus Jensen <k.jensen@samsung.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  util/vfio-helpers.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
>> index 306b5a83e48..7de5081dbd3 100644
>> --- a/util/vfio-helpers.c
>> +++ b/util/vfio-helpers.c

>>  static int
>> -qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
>> +qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova,
>> +                         Error **errp)
>>  {
>>      int i;
>>  
>> @@ -718,6 +722,8 @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
>>              return 0;
>>          }
>>      }
>> +    error_setg(errp, "temporary iova range not found");
>> +
>>      return -ENOMEM;
>>  }

>> @@ -776,7 +782,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
>>              }
>>              qemu_vfio_dump_mappings(s);
>>          } else {
>> -            if (qemu_vfio_find_temp_iova(s, size, &iova0)) {
>> +            if (qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
>>                  ret = -ENOMEM;
>>                  goto out;
>>              }
> 
> Not related to this patch, but it is slightly confusing that these
> functions actually already return a negative errno, but then we
> overwrite it.

Good point. I'll make it return a boolean.
diff mbox series

Patch

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 306b5a83e48..7de5081dbd3 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -678,7 +678,8 @@  static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)
 }
 
 static int
-qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
+qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova,
+                          Error **errp)
 {
     int i;
 
@@ -696,11 +697,14 @@  qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
             return 0;
         }
     }
+    error_setg(errp, "fixed iova range not found");
+
     return -ENOMEM;
 }
 
 static int
-qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
+qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova,
+                         Error **errp)
 {
     int i;
 
@@ -718,6 +722,8 @@  qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
             return 0;
         }
     }
+    error_setg(errp, "temporary iova range not found");
+
     return -ENOMEM;
 }
 
@@ -762,7 +768,7 @@  int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
             goto out;
         }
         if (!temporary) {
-            if (qemu_vfio_find_fixed_iova(s, size, &iova0)) {
+            if (qemu_vfio_find_fixed_iova(s, size, &iova0, errp) < 0) {
                 ret = -ENOMEM;
                 goto out;
             }
@@ -776,7 +782,7 @@  int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
             }
             qemu_vfio_dump_mappings(s);
         } else {
-            if (qemu_vfio_find_temp_iova(s, size, &iova0)) {
+            if (qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
                 ret = -ENOMEM;
                 goto out;
             }