From b9bcc3531e069445be1f57e499fcef1a7e19ac31 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Tue, 16 May 2017 13:25:03 -0700 Subject: [PATCH] bdev: make enum spdk_bdev_io_status private The user should not see the bdev_io status directly; the NVMe and SCSI error code wrappers provide the ability to translate to the desired format regardless of what kind of error is stored inside the bdev_io. Replace the spdk_bdev_io_completion_cb status parameter with a bool simply indiciating whether the I/O completed successfully. Change-Id: Iad18c2dac4374112c41b7a656154ed3ae1a68569 Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/362047 Tested-by: Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- include/spdk/bdev.h | 20 ++++++++-------- include/spdk_internal/bdev.h | 9 +++++++ lib/bdev/bdev.c | 2 +- lib/bdev/error/vbdev_error.c | 2 +- lib/blob/bdev/blob_bdev.c | 4 ++-- lib/nvmf/virtual.c | 2 +- lib/scsi/scsi_bdev.c | 6 ++--- test/lib/bdev/bdevio/bdevio.c | 40 +++++++++++++++---------------- test/lib/bdev/bdevperf/bdevperf.c | 12 +++++----- 9 files changed, 53 insertions(+), 44 deletions(-) diff --git a/include/spdk/bdev.h b/include/spdk/bdev.h index b0cc916f2..e4408e7ba 100644 --- a/include/spdk/bdev.h +++ b/include/spdk/bdev.h @@ -84,15 +84,6 @@ enum spdk_bdev_io_type { SPDK_BDEV_IO_TYPE_RESET, }; -/** Blockdev I/O completion status */ -enum spdk_bdev_io_status { - SPDK_BDEV_IO_STATUS_SCSI_ERROR = -3, - SPDK_BDEV_IO_STATUS_NVME_ERROR = -2, - SPDK_BDEV_IO_STATUS_FAILED = -1, - SPDK_BDEV_IO_STATUS_PENDING = 0, - SPDK_BDEV_IO_STATUS_SUCCESS = 1, -}; - /** Blockdev reset operation type */ enum spdk_bdev_reset_type { /** @@ -110,8 +101,17 @@ enum spdk_bdev_reset_type { SPDK_BDEV_RESET_SOFT, }; +/** + * Block device completion callback + * + * \param bdev_io Block device I/O that has completed. + * \param success true if I/O completed successfully or false if it failed; additional error + * information may be retrieved from bdev_io by calling + * spdk_bdev_io_get_nvme_status() or spdk_bdev_io_get_scsi_status(). + * \param cb_arg Callback argument specified when bdev_io was submitted. + */ typedef void (*spdk_bdev_io_completion_cb)(struct spdk_bdev_io *bdev_io, - enum spdk_bdev_io_status status, + bool success, void *cb_arg); struct spdk_bdev *spdk_bdev_get_by_name(const char *bdev_name); diff --git a/include/spdk_internal/bdev.h b/include/spdk_internal/bdev.h index be5095e47..78a42bfd8 100644 --- a/include/spdk_internal/bdev.h +++ b/include/spdk_internal/bdev.h @@ -148,6 +148,15 @@ struct spdk_bdev_fn_table { int (*dump_config_json)(void *ctx, struct spdk_json_write_ctx *w); }; +/** Blockdev I/O completion status */ +enum spdk_bdev_io_status { + SPDK_BDEV_IO_STATUS_SCSI_ERROR = -3, + SPDK_BDEV_IO_STATUS_NVME_ERROR = -2, + SPDK_BDEV_IO_STATUS_FAILED = -1, + SPDK_BDEV_IO_STATUS_PENDING = 0, + SPDK_BDEV_IO_STATUS_SUCCESS = 1, +}; + struct spdk_bdev { /** User context passed in by the backend */ void *ctxt; diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 24ccd570c..2768d709f 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -943,7 +943,7 @@ spdk_bdev_io_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status sta bdev_io->status = status; assert(bdev_io->cb != NULL); - bdev_io->cb(bdev_io, status, bdev_io->caller_ctx); + bdev_io->cb(bdev_io, status == SPDK_BDEV_IO_STATUS_SUCCESS, bdev_io->caller_ctx); } void diff --git a/lib/bdev/error/vbdev_error.c b/lib/bdev/error/vbdev_error.c index aa03e037f..270a3b6be 100644 --- a/lib/bdev/error/vbdev_error.c +++ b/lib/bdev/error/vbdev_error.c @@ -69,7 +69,7 @@ spdk_vbdev_inject_error(uint32_t io_type_mask, uint32_t error_num) } static void -vbdev_error_task_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, +vbdev_error_task_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct spdk_bdev_io *bdevio = (struct spdk_bdev_io *)cb_arg; diff --git a/lib/blob/bdev/blob_bdev.c b/lib/blob/bdev/blob_bdev.c index 75cb45a29..0eb34f369 100644 --- a/lib/blob/bdev/blob_bdev.c +++ b/lib/blob/bdev/blob_bdev.c @@ -52,12 +52,12 @@ __get_bdev(struct spdk_bs_dev *dev) } static void -bdev_blob_io_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, void *arg) +bdev_blob_io_complete(struct spdk_bdev_io *bdev_io, bool success, void *arg) { struct spdk_bs_dev_cb_args *cb_args = arg; int bserrno; - if (status == SPDK_BDEV_IO_STATUS_SUCCESS) { + if (success) { bserrno = 0; } else { bserrno = -EIO; diff --git a/lib/nvmf/virtual.c b/lib/nvmf/virtual.c index 2ffdff442..f4e03b341 100644 --- a/lib/nvmf/virtual.c +++ b/lib/nvmf/virtual.c @@ -119,7 +119,7 @@ nvmf_virtual_ctrlr_poll_for_completions(struct spdk_nvmf_subsystem *subsystem) } static void -nvmf_virtual_ctrlr_complete_cmd(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, +nvmf_virtual_ctrlr_complete_cmd(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct spdk_nvmf_request *req = cb_arg; diff --git a/lib/scsi/scsi_bdev.c b/lib/scsi/scsi_bdev.c index 15998fa9e..430818583 100644 --- a/lib/scsi/scsi_bdev.c +++ b/lib/scsi/scsi_bdev.c @@ -1235,7 +1235,7 @@ spdk_bdev_scsi_mode_select_page(struct spdk_bdev *bdev, } static void -spdk_bdev_scsi_task_complete_cmd(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, +spdk_bdev_scsi_task_complete_cmd(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct spdk_scsi_task *task = cb_arg; @@ -1247,12 +1247,12 @@ spdk_bdev_scsi_task_complete_cmd(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io } static void -spdk_bdev_scsi_task_complete_mgmt(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, +spdk_bdev_scsi_task_complete_mgmt(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct spdk_scsi_task *task = cb_arg; - if (status == SPDK_BDEV_IO_STATUS_SUCCESS) { + if (success) { task->response = SPDK_SCSI_TASK_MGMT_RESP_SUCCESS; } diff --git a/test/lib/bdev/bdevio/bdevio.c b/test/lib/bdev/bdevio/bdevio.c index 3f1752f60..bb3cdc16c 100644 --- a/test/lib/bdev/bdevio/bdevio.c +++ b/test/lib/bdev/bdevio/bdevio.c @@ -160,7 +160,7 @@ bdevio_cleanup_targets(void) } } -static enum spdk_bdev_io_status g_completion_status; +static bool g_completion_success; static void initialize_buffer(char **buf, int pattern, int size) @@ -170,9 +170,9 @@ initialize_buffer(char **buf, int pattern, int size) } static void -quick_test_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, void *arg) +quick_test_complete(struct spdk_bdev_io *bdev_io, bool success, void *arg) { - g_completion_status = status; + g_completion_success = success; spdk_bdev_free_io(bdev_io); wake_ut_thread(); } @@ -193,7 +193,7 @@ __blockdev_write(void *arg1, void *arg2) } if (!bdev_io) { - g_completion_status = SPDK_BDEV_IO_STATUS_FAILED; + g_completion_success = false; wake_ut_thread(); } } @@ -234,7 +234,7 @@ blockdev_write(struct io_target *target, char *tx_buf, req.offset = offset; sgl_chop_buffer(&req, iov_len); - g_completion_status = SPDK_BDEV_IO_STATUS_FAILED; + g_completion_success = false; execute_spdk_function(__blockdev_write, &req, NULL); } @@ -255,7 +255,7 @@ __blockdev_read(void *arg1, void *arg2) } if (!bdev_io) { - g_completion_status = SPDK_BDEV_IO_STATUS_FAILED; + g_completion_success = false; wake_ut_thread(); } } @@ -273,7 +273,7 @@ blockdev_read(struct io_target *target, char *rx_buf, req.iovcnt = 0; sgl_chop_buffer(&req, iov_len); - g_completion_status = SPDK_BDEV_IO_STATUS_FAILED; + g_completion_success = false; execute_spdk_function(__blockdev_read, &req, NULL); } @@ -312,20 +312,20 @@ blockdev_write_read(uint32_t data_length, uint32_t iov_len, int pattern, uint64_ blockdev_write(target, tx_buf, offset, data_length, iov_len); if (expected_rc == 0) { - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_SUCCESS); + CU_ASSERT_EQUAL(g_completion_success, true); } else { - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_FAILED); + CU_ASSERT_EQUAL(g_completion_success, false); } blockdev_read(target, rx_buf, offset, data_length, iov_len); if (expected_rc == 0) { - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_SUCCESS); + CU_ASSERT_EQUAL(g_completion_success, true); } else { - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_FAILED); + CU_ASSERT_EQUAL(g_completion_success, false); } - if (g_completion_status == SPDK_BDEV_IO_STATUS_SUCCESS) { + if (g_completion_success) { rc = blockdev_write_read_data_match(rx_buf, tx_buf, data_length); /* Assert the write by comparing it with values read * from each blockdev */ @@ -547,10 +547,10 @@ blockdev_write_read_offset_plus_nbytes_equals_bdev_size(void) initialize_buffer(&rx_buf, 0, block_size); blockdev_write(target, tx_buf, offset, block_size, 0); - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_SUCCESS); + CU_ASSERT_EQUAL(g_completion_success, true); blockdev_read(target, rx_buf, offset, block_size, 0); - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_SUCCESS); + CU_ASSERT_EQUAL(g_completion_success, true); rc = blockdev_write_read_data_match(rx_buf, tx_buf, block_size); /* Assert the write by comparing it with values read @@ -590,10 +590,10 @@ blockdev_write_read_offset_plus_nbytes_gt_bdev_size(void) initialize_buffer(&rx_buf, 0, data_length); blockdev_write(target, tx_buf, offset, data_length, 0); - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_FAILED); + CU_ASSERT_EQUAL(g_completion_success, false); blockdev_read(target, rx_buf, offset, data_length, 0); - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_FAILED); + CU_ASSERT_EQUAL(g_completion_success, false); target = target->next; } @@ -662,7 +662,7 @@ __blockdev_reset(void *arg1, void *arg2) rc = spdk_bdev_reset(target->bdev, *reset_type, quick_test_complete, NULL); if (rc < 0) { - g_completion_status = SPDK_BDEV_IO_STATUS_FAILED; + g_completion_success = false; wake_ut_thread(); } } @@ -674,7 +674,7 @@ blockdev_reset(struct io_target *target, enum spdk_bdev_reset_type reset_type) req.target = target; - g_completion_status = SPDK_BDEV_IO_STATUS_FAILED; + g_completion_success = false; execute_spdk_function(__blockdev_reset, &req, &reset_type); } @@ -687,10 +687,10 @@ blockdev_test_reset(void) target = g_io_targets; while (target != NULL) { blockdev_reset(target, SPDK_BDEV_RESET_HARD); - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_SUCCESS); + CU_ASSERT_EQUAL(g_completion_success, true); blockdev_reset(target, SPDK_BDEV_RESET_SOFT); - CU_ASSERT_EQUAL(g_completion_status, SPDK_BDEV_IO_STATUS_SUCCESS); + CU_ASSERT_EQUAL(g_completion_success, true); target = target->next; } diff --git a/test/lib/bdev/bdevperf/bdevperf.c b/test/lib/bdev/bdevperf/bdevperf.c index 87a644877..4f2c9ed2d 100644 --- a/test/lib/bdev/bdevperf/bdevperf.c +++ b/test/lib/bdev/bdevperf/bdevperf.c @@ -187,7 +187,7 @@ end_run(void *arg1, void *arg2) struct rte_mempool *task_pool; static void -bdevperf_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, void *cb_arg) +bdevperf_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct io_target *target; struct bdevperf_task *task = cb_arg; @@ -197,7 +197,7 @@ bdevperf_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, target = task->target; - if (status != SPDK_BDEV_IO_STATUS_SUCCESS) { + if (!success) { if (!g_reset) { target->is_draining = true; g_run_failed = true; @@ -235,7 +235,7 @@ bdevperf_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, } static void -bdevperf_unmap_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, void *cb_arg) +bdevperf_unmap_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct io_target *target; struct bdevperf_task *task = cb_arg; @@ -254,7 +254,7 @@ bdevperf_unmap_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status s } static void -bdevperf_verify_write_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, +bdevperf_verify_write_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct io_target *target; @@ -366,12 +366,12 @@ end_target(void *arg) static void reset_target(void *arg); static void -reset_cb(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status status, void *cb_arg) +reset_cb(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { struct bdevperf_task *task = cb_arg; struct io_target *target = task->target; - if (status != SPDK_BDEV_IO_STATUS_SUCCESS) { + if (!success) { printf("Reset blockdev=%s failed\n", spdk_bdev_get_name(target->bdev)); target->is_draining = true; g_run_failed = true;