bdev: Solve the ABI compatibility issue related with spdk_bdev_opts

This change prepares for the potential change in spdk-bdev_opts
in the future in order to maintain the ABI compatibility

Change-Id: I8ce24299173854c14c697bf7e28cf365c23f005f
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5597
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>

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

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);
 

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;
 }
 

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;
 	}

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);