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

Commit Message

Philippe Mathieu-Daudé Sept. 2, 2021, 7 a.m. UTC

Both qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova()
return an errno which is unused (or overwritten). Have them propagate
eventual errors to callers, returning a boolean (which is what the
Error API recommends, see commit e3fe3988d78 "error: Document Error
API usage rules" for rationale).

Suggested-by: Klaus Jensen <[e-mail]>
Reviewed-by: Klaus Jensen <[e-mail]>
Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
---
 util/vfio-helpers.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

Klaus Jensen Sept. 2, 2021, 7:15 a.m. UTC | #1

On Sep  2 09:00, Philippe Mathieu-Daudé wrote:
> Both qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova()
> return an errno which is unused (or overwritten). Have them propagate
> eventual errors to callers, returning a boolean (which is what the
> Error API recommends, see commit e3fe3988d78 "error: Document Error
> API usage rules" for rationale).
> 
> Suggested-by: Klaus Jensen <[e-mail]>
> Reviewed-by: Klaus Jensen <[e-mail]>
> Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>

The switch to bool LGTM.

Reviewed and Acked :)

Patch

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 306b5a83e48..b93a3d35787 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -677,8 +677,8 @@  static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)
     return true;
 }
 
-static int
-qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
+static bool qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size,
+                                      uint64_t *iova, Error **errp)
 {
     int i;
 
@@ -693,14 +693,16 @@  qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
             s->usable_iova_ranges[i].end - s->low_water_mark + 1 == 0) {
             *iova = s->low_water_mark;
             s->low_water_mark += size;
-            return 0;
+            return true;
         }
     }
-    return -ENOMEM;
+    error_setg(errp, "fixed iova range not found");
+
+    return false;
 }
 
-static int
-qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
+static bool qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size,
+                                     uint64_t *iova, Error **errp)
 {
     int i;
 
@@ -715,10 +717,12 @@  qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
             s->high_water_mark - s->usable_iova_ranges[i].start + 1 == 0) {
             *iova = s->high_water_mark - size;
             s->high_water_mark = *iova;
-            return 0;
+            return true;
         }
     }
-    return -ENOMEM;
+    error_setg(errp, "temporary iova range not found");
+
+    return false;
 }
 
 /**
@@ -762,7 +766,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)) {
                 ret = -ENOMEM;
                 goto out;
             }
@@ -776,7 +780,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;
             }