don't expose -errno in public API (#327)

Regardless of what we do internally, most of our API uses standard mechanisms
for reporting errors. Fix vfu_run_ctx() to do so properly as well, and fix a
couple of other references for user-provided callbacks.

This will require a small fix to SPDK.

Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>

5 files changed, 38 insertions, 25 deletions

diff --git a/include/libvfio-user.h b/include/libvfio-user.h
index 62d4e77..e48514a 100644
--- a/include/libvfio-user.h
+++ b/include/libvfio-user.h
@@ -140,7 +140,7 @@ vfu_attach_ctx(vfu_ctx_t *vfu_ctx);
  *
  * @vfu_ctx: The libvfio-user context to poll
  *
- * @returns 0 on success, -errno on failure.
+ * @returns 0 on success, -1 on error. Sets errno.
  */
 int
 vfu_run_ctx(vfu_ctx_t *vfu_ctx);
@@ -193,7 +193,7 @@ vfu_setup_log(vfu_ctx_t *vfu_ctx, vfu_log_fn_t *log, int level);
  * @offset: byte offset within the region
  * @is_write: whether or not this is a write
  *
- * @returns the number of bytes read or written, or a negative integer on error
+ * @returns the number of bytes read or written, or -1 on error, setting errno.
  */
 typedef ssize_t (vfu_region_access_cb_t)(vfu_ctx_t *vfu_ctx, char *buf,
                                          size_t count, loff_t offset,
@@ -400,9 +400,14 @@ typedef struct {
      */
     int version;
 
-    /* migration state transition callback */
-    /* TODO rename to vfu_migration_state_transition_callback */
-    /* FIXME maybe we should create a single callback and pass the state? */
+    /*
+     * Migration state transition callback.
+     *
+     * Returns -1 on error, setting errno.
+     *
+     * TODO rename to vfu_migration_state_transition_callback
+     * FIXME maybe we should create a single callback and pass the state?
+     */
     int (*transition)(vfu_ctx_t *vfu_ctx, vfu_migr_state_t state);
 
     /* Callbacks for saving device state */
@@ -433,7 +438,9 @@ typedef struct {
     /*
      * Function that is called to read migration data. offset and size can be
      * any subrange on the offset and size previously returned by prepare_data.
-     * The function must return the amount of data read or -errno on error.
+     * The function must return the amount of data read or -1 on error, setting
+     * errno.
+     *
      * This function can be called even if the migration data can be memory
      * mapped.
      *
@@ -446,7 +453,8 @@ typedef struct {
 
     /*
      * Fuction that is called for writing previously stored device state. The
-     * function must return the amount of data written or -errno on error.
+     * function must return the amount of data written or -1 on error, setting
+     * errno.
      */
     ssize_t (*write_data)(vfu_ctx_t *vfu_ctx, void *buf, __u64 count,
                           __u64 offset);
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index fc427f5..e25787d 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -1057,9 +1057,9 @@ vfu_run_ctx(vfu_ctx_t *vfu_ctx)
     blocking = !(vfu_ctx->flags & LIBVFIO_USER_FLAG_ATTACH_NB);
     do {
         err = process_request(vfu_ctx);
-    } while (err >= 0 && blocking);
+    } while (err == 0 && blocking);
 
-    return err >= 0 ? 0 : err;
+    return err == 0 ? 0 : ERROR_INT(-err);
 }
 
 static void
diff --git a/lib/migration.c b/lib/migration.c
index 0e1b2a6..ec07961 100644
--- a/lib/migration.c
+++ b/lib/migration.c
@@ -528,6 +528,9 @@ migration_region_access(vfu_ctx_t *vfu_ctx, char *buf, size_t count,
         pos -= migr->data_offset;
         if (is_write) {
             ret = migr->callbacks.write_data(vfu_ctx, buf, count, pos);
+            if (ret == -1) {
+                ret = -errno;
+            }
         } else {
             /*
              * FIXME <linux/vfio.h> says:
@@ -538,6 +541,9 @@ migration_region_access(vfu_ctx_t *vfu_ctx, char *buf, size_t count,
              * Does this mean that partial reads are not allowed?
              */
             ret = migr->callbacks.read_data(vfu_ctx, buf, count, pos);
+            if (ret == -1) {
+                ret = -errno;
+            }
         }
     }
 
diff --git a/samples/gpio-pci-idio-16.c b/samples/gpio-pci-idio-16.c
index 350d11f..771de3a 100644
--- a/samples/gpio-pci-idio-16.c
+++ b/samples/gpio-pci-idio-16.c
@@ -122,8 +122,7 @@ main(int argc, char *argv[])
     ret = vfu_setup_region(vfu_ctx, VFU_PCI_DEV_BAR2_REGION_IDX, 0x100,
                            &bar2_access, VFU_REGION_FLAG_RW, NULL, 0, -1);
     if (ret < 0) {
-        fprintf(stderr, "failed to setup region\n");
-        goto out;
+        err(EXIT_FAILURE, "failed to setup region");
     }
 
     ret = vfu_setup_device_nr_irqs(vfu_ctx, VFU_DEV_INTX_IRQ, 1);
@@ -143,16 +142,13 @@ main(int argc, char *argv[])
 
     ret = vfu_run_ctx(vfu_ctx);
     if (ret != 0) {
-        if (ret != -ENOTCONN && ret != -EINTR) {
-            fprintf(stderr, "failed to realize device emulation\n");
-            goto out;
+        if (errno != ENOTCONN && errno != EINTR) {
+            err(EXIT_FAILURE, "failed to realize device emulation");
         }
-        ret = 0;
     }
 
-out:
     vfu_destroy_ctx(vfu_ctx);
-    return ret;
+    return EXIT_SUCCESS;
 }
 
 /* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/samples/server.c b/samples/server.c
index 9c2ee37..72d4241 100644
--- a/samples/server.c
+++ b/samples/server.c
@@ -81,7 +81,7 @@ arm_timer(vfu_ctx_t *vfu_ctx, time_t t)
             new.it_value.tv_sec);
     if (setitimer(ITIMER_REAL, &new, NULL) != 0) {
         vfu_log(vfu_ctx, LOG_ERR, "failed to arm timer: %m");
-        return -errno;
+        return -1;
     }
     return 0;
 }
@@ -339,7 +339,8 @@ migration_read_data(vfu_ctx_t *vfu_ctx, void *buf, __u64 size, __u64 offset)
      */
 
     if (offset != 0 || size != server_data->migration.pending_bytes) {
-        return -EINVAL;
+        errno = EINVAL;
+        return -1;
     }
 
     memcpy(buf, server_data->bar1, server_data->bar1_size);
@@ -365,7 +366,8 @@ migration_write_data(vfu_ctx_t *vfu_ctx, void *data, __u64 size, __u64 offset)
     if (offset != 0 || size < server_data->bar1_size) {
         vfu_log(vfu_ctx, LOG_DEBUG, "XXX bad migration data write %#llx-%#llx",
                 offset, offset + size - 1);
-        return -EINVAL;
+        errno = EINVAL;
+        return -1;
     }
 
     memcpy(server_data->bar1, buf, server_data->bar1_size);
@@ -375,7 +377,8 @@ migration_write_data(vfu_ctx_t *vfu_ctx, void *data, __u64 size, __u64 offset)
         return 0;
     }
     if (size != sizeof server_data->bar0) {
-        return -EINVAL;
+        errno = EINVAL;
+        return -1;
     }
     memcpy(&server_data->bar0, buf, sizeof server_data->bar0);
     ret = bar0_access(vfu_ctx, buf, sizeof server_data->bar0, 0, true);
@@ -574,7 +577,7 @@ int main(int argc, char *argv[])
 
     do {
         ret = vfu_run_ctx(vfu_ctx);
-        if (ret == -EINTR) {
+        if (ret == -1 && errno == EINTR) {
             if (irq_triggered) {
                 irq_triggered = false;
                 vfu_irq_trigger(vfu_ctx, 0);
@@ -601,9 +604,9 @@ int main(int argc, char *argv[])
         }
     } while (ret == 0);
 
-    if (ret != -ENOTCONN && ret != -EINTR && ret != -ESHUTDOWN) {
-        errx(EXIT_FAILURE, "failed to realize device emulation: %s\n",
-             strerror(-ret));
+    if (ret == -1 &&
+        errno != ENOTCONN && errno != EINTR && errno != ESHUTDOWN) {
+        errx(EXIT_FAILURE, "failed to realize device emulation");
     }
 
     vfu_destroy_ctx(vfu_ctx);