From cf449d6290ddd723a23f0451d0ce18ffc6099e15 Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 10 Feb 2021 18:26:53 +0000 Subject: 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 Reviewed-by: Swapnil Ingle --- include/libvfio-user.h | 22 +++++++++++++++------- lib/libvfio-user.c | 4 ++-- lib/migration.c | 6 ++++++ samples/gpio-pci-idio-16.c | 12 ++++-------- samples/server.c | 19 +++++++++++-------- 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 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); -- cgit v1.1