From 6f227249fadd81299b7dac1a269f0349addad82e Mon Sep 17 00:00:00 2001 From: GangCao Date: Fri, 29 Sep 2017 12:14:00 -0400 Subject: [PATCH] nvme: add a new opts_size parameter for default ctrlr opts Add a new parameter for the default ctrlr opts initialization. This is to make sure future compatibility when SPDK components are built as a shared library. User's version and SPDK's version may be in different size. The change here is to make sure the backward compatibility when new fields are added in the struct spdk_nvme_ctrlr_opts. Change-Id: Icfc9640993cb06063b825d4df5835d920dd374e5 Signed-off-by: GangCao Reviewed-on: https://review.gerrithub.io/380846 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris --- lib/nvme/nvme.c | 2 +- lib/nvme/nvme_ctrlr.c | 68 +++++++++++++++---- lib/nvme/nvme_internal.h | 2 +- lib/nvme/nvme_rdma.c | 2 +- test/unit/lib/nvme/nvme.c/nvme_ut.c | 2 +- .../lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c | 31 ++++++++- .../lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c | 2 +- 7 files changed, 89 insertions(+), 20 deletions(-) diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index b6500c926..afbaaf708 100644 --- a/lib/nvme/nvme.c +++ b/lib/nvme/nvme.c @@ -320,7 +320,7 @@ nvme_ctrlr_probe(const struct spdk_nvme_transport_id *trid, void *devhandle, struct spdk_nvme_ctrlr *ctrlr; struct spdk_nvme_ctrlr_opts opts; - spdk_nvme_ctrlr_opts_set_defaults(&opts); + spdk_nvme_ctrlr_opts_set_defaults(&opts, sizeof(opts)); if (probe_cb(cb_ctx, trid, &opts)) { ctrlr = nvme_transport_ctrlr_construct(trid, &opts, devhandle); diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 04209a15a..804fb3c9f 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -78,23 +78,63 @@ nvme_ctrlr_set_cc(struct spdk_nvme_ctrlr *ctrlr, const union spdk_nvme_cc_regist } void -spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts) +spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts, size_t opts_size) { char host_id_str[37]; - opts->num_io_queues = DEFAULT_MAX_IO_QUEUES; - opts->use_cmb_sqs = true; - opts->arb_mechanism = SPDK_NVME_CC_AMS_RR; - opts->keep_alive_timeout_ms = 10 * 1000; - opts->io_queue_size = DEFAULT_IO_QUEUE_SIZE; - opts->io_queue_requests = DEFAULT_IO_QUEUE_REQUESTS; - memset(opts->src_addr, 0, sizeof(opts->src_addr)); - memset(opts->src_svcid, 0, sizeof(opts->src_svcid)); - memset(opts->host_id, 0, sizeof(opts->host_id)); - memcpy(opts->extended_host_id, g_spdk_nvme_driver->default_extended_host_id, - sizeof(opts->extended_host_id)); - uuid_unparse(opts->extended_host_id, host_id_str); - snprintf(opts->hostnqn, sizeof(opts->hostnqn), "2014-08.org.nvmexpress:uuid:%s", host_id_str); + assert(opts); + + memset(opts, 0, opts_size); + +#define FIELD_OK(field) \ + offsetof(struct spdk_nvme_ctrlr_opts, field) + sizeof(opts->field) <= opts_size + + if (FIELD_OK(num_io_queues)) { + opts->num_io_queues = DEFAULT_MAX_IO_QUEUES; + } + + if (FIELD_OK(use_cmb_sqs)) { + opts->use_cmb_sqs = true; + } + + if (FIELD_OK(arb_mechanism)) { + opts->arb_mechanism = SPDK_NVME_CC_AMS_RR; + } + + if (FIELD_OK(keep_alive_timeout_ms)) { + opts->keep_alive_timeout_ms = 10 * 1000; + } + + if (FIELD_OK(io_queue_size)) { + opts->io_queue_size = DEFAULT_IO_QUEUE_SIZE; + } + + if (FIELD_OK(io_queue_requests)) { + opts->io_queue_requests = DEFAULT_IO_QUEUE_REQUESTS; + } + + if (FIELD_OK(host_id)) { + memset(opts->host_id, 0, sizeof(opts->host_id)); + } + + if (FIELD_OK(extended_host_id)) { + memcpy(opts->extended_host_id, g_spdk_nvme_driver->default_extended_host_id, + sizeof(opts->extended_host_id)); + } + + if (FIELD_OK(hostnqn)) { + uuid_unparse(g_spdk_nvme_driver->default_extended_host_id, host_id_str); + snprintf(opts->hostnqn, sizeof(opts->hostnqn), "2014-08.org.nvmexpress:uuid:%s", host_id_str); + } + + if (FIELD_OK(src_addr)) { + memset(opts->src_addr, 0, sizeof(opts->src_addr)); + } + + if (FIELD_OK(src_svcid)) { + memset(opts->src_svcid, 0, sizeof(opts->src_svcid)); + } +#undef FIELD_OK } /** diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 26d854f6e..87695c3e8 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -587,7 +587,7 @@ void nvme_free_request(struct nvme_request *req); void nvme_request_remove_child(struct nvme_request *parent, struct nvme_request *child); uint64_t nvme_get_quirks(const struct spdk_pci_id *id); -void spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts); +void spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts, size_t opts_size); int nvme_robust_mutex_init_shared(pthread_mutex_t *mtx); int nvme_robust_mutex_init_recursive_shared(pthread_mutex_t *mtx); diff --git a/lib/nvme/nvme_rdma.c b/lib/nvme/nvme_rdma.c index 0afab2705..336fdea31 100644 --- a/lib/nvme/nvme_rdma.c +++ b/lib/nvme/nvme_rdma.c @@ -1192,7 +1192,7 @@ nvme_rdma_ctrlr_scan(const struct spdk_nvme_transport_id *discovery_trid, return rc; } - spdk_nvme_ctrlr_opts_set_defaults(&discovery_opts); + spdk_nvme_ctrlr_opts_set_defaults(&discovery_opts, sizeof(discovery_opts)); /* For discovery_ctrlr set the timeout to 0 */ discovery_opts.keep_alive_timeout_ms = 0; diff --git a/test/unit/lib/nvme/nvme.c/nvme_ut.c b/test/unit/lib/nvme/nvme.c/nvme_ut.c index c739848d0..3d0972ccb 100644 --- a/test/unit/lib/nvme/nvme.c/nvme_ut.c +++ b/test/unit/lib/nvme/nvme.c/nvme_ut.c @@ -95,7 +95,7 @@ nvme_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr) } void -spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts) +spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts, size_t opts_size) { memset(opts, 0, sizeof(*opts)); } diff --git a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c index 693c7791c..cc5e32f5e 100644 --- a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c @@ -1411,16 +1411,45 @@ test_ctrlr_opts_set_defaults(void) SPDK_CU_ASSERT_FATAL(sizeof(uuid) == sizeof(g_spdk_nvme_driver->default_extended_host_id)); memcpy(g_spdk_nvme_driver->default_extended_host_id, uuid, sizeof(uuid)); - spdk_nvme_ctrlr_opts_set_defaults(&opts); + memset(&opts, 0, sizeof(opts)); + + /* set a smaller opts_size */ + CU_ASSERT(sizeof(opts) > 8); + spdk_nvme_ctrlr_opts_set_defaults(&opts, 8); + CU_ASSERT_EQUAL(opts.num_io_queues, DEFAULT_MAX_IO_QUEUES); + CU_ASSERT_TRUE(opts.use_cmb_sqs); + /* check below fields are not initialized by default value */ + CU_ASSERT_EQUAL(opts.arb_mechanism, 0); + CU_ASSERT_EQUAL(opts.keep_alive_timeout_ms, 0); + CU_ASSERT_EQUAL(opts.io_queue_size, 0); + CU_ASSERT_EQUAL(opts.io_queue_requests, 0); + for (int i = 0; i < 8; i++) { + CU_ASSERT(opts.host_id[i] == 0); + } + for (int i = 0; i < 16; i++) { + CU_ASSERT(opts.extended_host_id[i] == 0); + } + CU_ASSERT(strlen(opts.hostnqn) == 0); + CU_ASSERT(strlen(opts.src_addr) == 0); + CU_ASSERT(strlen(opts.src_svcid) == 0); + + /* set a consistent opts_size */ + spdk_nvme_ctrlr_opts_set_defaults(&opts, sizeof(opts)); CU_ASSERT_EQUAL(opts.num_io_queues, DEFAULT_MAX_IO_QUEUES); CU_ASSERT_TRUE(opts.use_cmb_sqs); CU_ASSERT_EQUAL(opts.arb_mechanism, SPDK_NVME_CC_AMS_RR); CU_ASSERT_EQUAL(opts.keep_alive_timeout_ms, 10 * 1000); CU_ASSERT_EQUAL(opts.io_queue_size, DEFAULT_IO_QUEUE_SIZE); + CU_ASSERT_EQUAL(opts.io_queue_requests, DEFAULT_IO_QUEUE_REQUESTS); + for (int i = 0; i < 8; i++) { + CU_ASSERT(opts.host_id[i] == 0); + } CU_ASSERT_STRING_EQUAL(opts.hostnqn, "2014-08.org.nvmexpress:uuid:e53e9258-c93b-48b5-be1a-f025af6d232a"); CU_ASSERT(memcmp(opts.extended_host_id, g_spdk_nvme_driver->default_extended_host_id, sizeof(opts.extended_host_id)) == 0); + CU_ASSERT(strlen(opts.src_addr) == 0); + CU_ASSERT(strlen(opts.src_svcid) == 0); } #if 0 /* TODO: move to PCIe-specific unit test */ diff --git a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c index cd2312a02..15a8af25f 100644 --- a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c +++ b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c @@ -135,7 +135,7 @@ spdk_pci_addr_compare(const struct spdk_pci_addr *a1, const struct spdk_pci_addr } void -spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts) +spdk_nvme_ctrlr_opts_set_defaults(struct spdk_nvme_ctrlr_opts *opts, size_t opts_size) { memset(opts, 0, sizeof(*opts)); }