diff --git a/CHANGELOG.md b/CHANGELOG.md index d6fca8bd4..d9abcc11c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## v21.01: (Upcoming Release) +### bdev + +An `opts_size`element was added in the `spdk_bdev_opts` structure to solve the +ABI compatiblity issue between different SPDK version. And also add `opts_size` +parameter in spdk_bdev_get_opts function. + ### event The pci_whitelist and pci_blacklist members of struct spdk_app_opts have been diff --git a/include/spdk/bdev.h b/include/spdk/bdev.h index 6a9db5564..d799356bd 100644 --- a/include/spdk/bdev.h +++ b/include/spdk/bdev.h @@ -187,9 +187,22 @@ struct spdk_bdev_opts { uint32_t bdev_io_pool_size; uint32_t bdev_io_cache_size; bool bdev_auto_examine; + /** + * The size of spdk_bdev_opts according to the caller of this library is used for ABI + * compatibility. The library uses this field to know how many fields in this + * structure are valid. And the library will populate any remaining fields with default values. + * New added fields should be put at the end of the struct. + */ + size_t opts_size; }; -void spdk_bdev_get_opts(struct spdk_bdev_opts *opts); +/** + * Get the options for the bdev module. + * + * \param opts Output parameter for options. + * \param opts_size sizeof(*opts) + */ +void spdk_bdev_get_opts(struct spdk_bdev_opts *opts, size_t opts_size); int spdk_bdev_set_opts(struct spdk_bdev_opts *opts); diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 125852850..df4809ca8 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -371,9 +371,34 @@ static bool bdev_abort_queued_io(bdev_io_tailq_t *queue, struct spdk_bdev_io *bi static bool bdev_abort_buf_io(bdev_io_stailq_t *queue, struct spdk_bdev_io *bio_to_abort); void -spdk_bdev_get_opts(struct spdk_bdev_opts *opts) +spdk_bdev_get_opts(struct spdk_bdev_opts *opts, size_t opts_size) { - *opts = g_bdev_opts; + if (!opts) { + SPDK_ERRLOG("opts should not be NULL\n"); + return; + } + + if (!opts_size) { + SPDK_ERRLOG("opts_size should not be zero value\n"); + return; + } + + opts->opts_size = opts_size; + +#define SET_FIELD(field) \ + if (offsetof(struct spdk_bdev_opts, field) + sizeof(opts->field) <= opts_size) { \ + opts->field = g_bdev_opts.field; \ + } \ + + SET_FIELD(bdev_io_pool_size); + SET_FIELD(bdev_io_cache_size); + SET_FIELD(bdev_auto_examine); + + /* Do not remove this statement, you should always update this statement when you adding a new field, + * and do not forget to add the SET_FIELD statement for your added field. */ + SPDK_STATIC_ASSERT(sizeof(struct spdk_bdev_opts) == 24, "Incorrect size"); + +#undef SET_FIELD } int @@ -381,6 +406,16 @@ spdk_bdev_set_opts(struct spdk_bdev_opts *opts) { uint32_t min_pool_size; + if (!opts) { + SPDK_ERRLOG("opts cannot be NULL\n"); + return -1; + } + + if (!opts->opts_size) { + SPDK_ERRLOG("opts_size inside opts cannot be zero value\n"); + return -1; + } + /* * Add 1 to the thread count to account for the extra mgmt_ch that gets created during subsystem * initialization. A second mgmt_ch will be created on the same thread when the application starts @@ -395,7 +430,19 @@ spdk_bdev_set_opts(struct spdk_bdev_opts *opts) return -1; } - g_bdev_opts = *opts; +#define SET_FIELD(field) \ + if (offsetof(struct spdk_bdev_opts, field) + sizeof(opts->field) <= opts->opts_size) { \ + g_bdev_opts.field = opts->field; \ + } \ + + SET_FIELD(bdev_io_pool_size); + SET_FIELD(bdev_io_cache_size); + SET_FIELD(bdev_auto_examine); + + g_bdev_opts.opts_size = opts->opts_size; + +#undef SET_FIELD + return 0; } diff --git a/lib/bdev/bdev_rpc.c b/lib/bdev/bdev_rpc.c index 6e56041a2..35025be64 100644 --- a/lib/bdev/bdev_rpc.c +++ b/lib/bdev/bdev_rpc.c @@ -75,7 +75,7 @@ rpc_bdev_set_options(struct spdk_jsonrpc_request *request, const struct spdk_jso } } - spdk_bdev_get_opts(&bdev_opts); + spdk_bdev_get_opts(&bdev_opts, sizeof(bdev_opts)); if (rpc_opts.bdev_io_pool_size != UINT32_MAX) { bdev_opts.bdev_io_pool_size = rpc_opts.bdev_io_pool_size; } diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index 407b7a9d0..684665958 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -904,6 +904,7 @@ bdev_io_types_test(void) }; int rc; + bdev_opts.opts_size = sizeof(bdev_opts); rc = spdk_bdev_set_opts(&bdev_opts); CU_ASSERT(rc == 0); spdk_bdev_initialize(bdev_init_cb, NULL); @@ -948,6 +949,7 @@ bdev_io_wait_test(void) struct bdev_ut_io_wait_entry io_wait_entry2; int rc; + bdev_opts.opts_size = sizeof(bdev_opts); rc = spdk_bdev_set_opts(&bdev_opts); CU_ASSERT(rc == 0); spdk_bdev_initialize(bdev_init_cb, NULL); @@ -1091,6 +1093,7 @@ bdev_io_boundary_split_test(void) uint64_t i; int rc; + bdev_opts.opts_size = sizeof(bdev_opts); rc = spdk_bdev_set_opts(&bdev_opts); CU_ASSERT(rc == 0); spdk_bdev_initialize(bdev_init_cb, NULL); @@ -2545,6 +2548,7 @@ bdev_io_split_with_io_wait(void) struct ut_expected_io *expected_io; int rc; + bdev_opts.opts_size = sizeof(bdev_opts); rc = spdk_bdev_set_opts(&bdev_opts); CU_ASSERT(rc == 0); spdk_bdev_initialize(bdev_init_cb, NULL); @@ -2681,6 +2685,7 @@ bdev_io_alignment(void) int iovcnt; uint64_t alignment; + bdev_opts.opts_size = sizeof(bdev_opts); rc = spdk_bdev_set_opts(&bdev_opts); CU_ASSERT(rc == 0); spdk_bdev_initialize(bdev_init_cb, NULL); @@ -2901,6 +2906,7 @@ bdev_io_alignment_with_boundary(void) int iovcnt; uint64_t alignment; + bdev_opts.opts_size = sizeof(bdev_opts); rc = spdk_bdev_set_opts(&bdev_opts); CU_ASSERT(rc == 0); spdk_bdev_initialize(bdev_init_cb, NULL); @@ -4081,6 +4087,7 @@ bdev_io_abort(void) uint64_t io_ctx1 = 0, io_ctx2 = 0, i; int rc; + bdev_opts.opts_size = sizeof(bdev_opts); rc = spdk_bdev_set_opts(&bdev_opts); CU_ASSERT(rc == 0); spdk_bdev_initialize(bdev_init_cb, NULL);