From b47f0f20b7aa966da2822c309f9c994fdd128da3 Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Thu, 24 Dec 2020 00:56:38 +0800 Subject: [PATCH] blob: Make the ABI compatibility of spdk_blob_opts structure. Change-Id: I1b1806864783e944d8f55c9393228a1954051236 Signed-off-by: Ziye Yang Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5687 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Aleksey Marchuk Reviewed-by: Shuhei Matsumoto --- CHANGELOG.md | 4 ++ include/spdk/blob.h | 11 +++- lib/blob/blobstore.c | 95 +++++++++++++++++++++++------ lib/lvol/lvol.c | 2 +- test/unit/lib/blob/blob.c/blob_ut.c | 2 +- test/unit/lib/lvol/lvol.c/lvol_ut.c | 3 +- 6 files changed, 95 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee45419f4..a75237f57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ An `opts_size` element was added in the `spdk_bs_opts` structure to solve the ABI compatiblity issue between different SPDK version. And also add `opts_size` parameter in `spdk_bs_opts_init` function. +An `opts_size` element was added in the `spdk_blob_opts` structure to solve the +ABI compatiblity issue between different SPDK version. And also add `opts_size` +parameter in `spdk_blob_opts_init` function. + ### event The pci_whitelist and pci_blacklist members of struct spdk_app_opts have been diff --git a/include/spdk/blob.h b/include/spdk/blob.h index 0674d011b..f9e686bb2 100644 --- a/include/spdk/blob.h +++ b/include/spdk/blob.h @@ -426,14 +426,23 @@ struct spdk_blob_opts { /** Enable separate extent pages in metadata */ bool use_extent_table; + + /** + * The size of spdk_blob_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; }; /** * Initialize a spdk_blob_opts structure to the default blob option values. * * \param opts spdk_blob_opts structure to initialize. + * \param opts_size It must be the size of spdk_blob_opts structure. */ -void spdk_blob_opts_init(struct spdk_blob_opts *opts); +void spdk_blob_opts_init(struct spdk_blob_opts *opts, size_t opts_size); /** * Create a new blob with options on the given blobstore. The new blob id will diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index 164bb6814..f5d3d8b98 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -203,13 +203,41 @@ blob_xattrs_init(struct spdk_blob_xattr_opts *xattrs) } void -spdk_blob_opts_init(struct spdk_blob_opts *opts) +spdk_blob_opts_init(struct spdk_blob_opts *opts, size_t opts_size) { - opts->num_clusters = 0; - opts->thin_provision = false; - opts->clear_method = BLOB_CLEAR_WITH_DEFAULT; - blob_xattrs_init(&opts->xattrs); - opts->use_extent_table = true; + 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; + } + + memset(opts, 0, opts_size); + opts->opts_size = opts_size; + +#define FIELD_OK(field) \ + offsetof(struct spdk_blob_opts, field) + sizeof(opts->field) <= opts_size + +#define SET_FIELD(field, value) \ + if (FIELD_OK(field)) { \ + opts->field = value; \ + } \ + + SET_FIELD(num_clusters, 0); + SET_FIELD(thin_provision, false); + SET_FIELD(clear_method, BLOB_CLEAR_WITH_DEFAULT); + + if (FIELD_OK(xattrs)) { + blob_xattrs_init(&opts->xattrs); + } + + SET_FIELD(use_extent_table, true); + +#undef FIELD_OK +#undef SET_FIELD } void @@ -4300,7 +4328,7 @@ bs_load_super_cpl(spdk_bs_sequence_t *seq, void *cb_arg, int bserrno) } } -static int +static inline int bs_opts_copy(struct spdk_bs_opts *src, struct spdk_bs_opts *dst) { @@ -5308,6 +5336,37 @@ blob_set_xattrs(struct spdk_blob *blob, const struct spdk_blob_xattr_opts *xattr return 0; } +static void +blob_opts_copy(const struct spdk_blob_opts *src, struct spdk_blob_opts *dst) +{ +#define FIELD_OK(field) \ + offsetof(struct spdk_blob_opts, field) + sizeof(src->field) <= src->opts_size + +#define SET_FIELD(field) \ + if (FIELD_OK(field)) { \ + dst->field = src->field; \ + } \ + + SET_FIELD(num_clusters); + SET_FIELD(thin_provision); + SET_FIELD(clear_method); + + if (FIELD_OK(xattrs)) { + memcpy(&dst->xattrs, &src->xattrs, sizeof(src->xattrs)); + } + + SET_FIELD(use_extent_table); + + dst->opts_size = src->opts_size; + + /* You should not remove this statement, but need to update the assert statement + * if you add a new field, and also add a corresponding SET_FIELD statement */ + SPDK_STATIC_ASSERT(sizeof(struct spdk_blob_opts) == 64, "Incorrect size"); + +#undef FIELD_OK +#undef SET_FIELD +} + static void bs_create_blob(struct spdk_blob_store *bs, const struct spdk_blob_opts *opts, @@ -5317,7 +5376,7 @@ bs_create_blob(struct spdk_blob_store *bs, struct spdk_blob *blob; uint32_t page_idx; struct spdk_bs_cpl cpl; - struct spdk_blob_opts opts_default; + struct spdk_blob_opts opts_local; struct spdk_blob_xattr_opts internal_xattrs_default; spdk_bs_sequence_t *seq; spdk_blob_id id; @@ -5345,12 +5404,12 @@ bs_create_blob(struct spdk_blob_store *bs, return; } - if (!opts) { - spdk_blob_opts_init(&opts_default); - opts = &opts_default; + spdk_blob_opts_init(&opts_local, sizeof(opts_local)); + if (opts) { + blob_opts_copy(opts, &opts_local); } - blob->use_extent_table = opts->use_extent_table; + blob->use_extent_table = opts_local.use_extent_table; if (blob->use_extent_table) { blob->invalid_flags |= SPDK_BLOB_EXTENT_TABLE; } @@ -5360,7 +5419,7 @@ bs_create_blob(struct spdk_blob_store *bs, internal_xattrs = &internal_xattrs_default; } - rc = blob_set_xattrs(blob, &opts->xattrs, false); + rc = blob_set_xattrs(blob, &opts_local.xattrs, false); if (rc < 0) { blob_free(blob); spdk_bit_array_clear(bs->used_blobids, page_idx); @@ -5378,13 +5437,13 @@ bs_create_blob(struct spdk_blob_store *bs, return; } - if (opts->thin_provision) { + if (opts_local.thin_provision) { blob_set_thin_provision(blob); } - blob_set_clear_method(blob, opts->clear_method); + blob_set_clear_method(blob, opts_local.clear_method); - rc = blob_resize(blob, opts->num_clusters); + rc = blob_resize(blob, opts_local.num_clusters); if (rc < 0) { blob_free(blob); spdk_bit_array_clear(bs->used_blobids, page_idx); @@ -5769,7 +5828,7 @@ bs_snapshot_origblob_open_cpl(void *cb_arg, struct spdk_blob *_blob, int bserrno _blob->locked_operation_in_progress = true; - spdk_blob_opts_init(&opts); + spdk_blob_opts_init(&opts, sizeof(opts)); blob_xattrs_init(&internal_xattrs); /* Change the size of new blob to the same as in original blob, @@ -5880,7 +5939,7 @@ bs_clone_origblob_open_cpl(void *cb_arg, struct spdk_blob *_blob, int bserrno) _blob->locked_operation_in_progress = true; - spdk_blob_opts_init(&opts); + spdk_blob_opts_init(&opts, sizeof(opts)); blob_xattrs_init(&internal_xattrs); opts.thin_provision = true; diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index d60945f3c..f1c6b1469 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -1063,7 +1063,7 @@ spdk_lvol_create(struct spdk_lvol_store *lvs, const char *name, uint64_t sz, spdk_uuid_fmt_lower(lvol->uuid_str, sizeof(lvol->uuid_str), &lvol->uuid); req->lvol = lvol; - spdk_blob_opts_init(&opts); + spdk_blob_opts_init(&opts, sizeof(opts)); opts.thin_provision = thin_provision; opts.num_clusters = num_clusters; opts.clear_method = lvol->clear_method; diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 8d5739f01..19eec2106 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -131,7 +131,7 @@ _get_snapshots_count(struct spdk_blob_store *bs) static void ut_spdk_blob_opts_init(struct spdk_blob_opts *opts) { - spdk_blob_opts_init(opts); + spdk_blob_opts_init(opts, sizeof(*opts)); opts->use_extent_table = g_use_extent_table; } diff --git a/test/unit/lib/lvol/lvol.c/lvol_ut.c b/test/unit/lib/lvol/lvol.c/lvol_ut.c index f779b985c..d12e49aac 100644 --- a/test/unit/lib/lvol/lvol.c/lvol_ut.c +++ b/test/unit/lib/lvol/lvol.c/lvol_ut.c @@ -399,8 +399,9 @@ spdk_bs_open_blob(struct spdk_blob_store *bs, spdk_blob_id blobid, DEFINE_STUB(spdk_bs_free_cluster_count, uint64_t, (struct spdk_blob_store *bs), BS_FREE_CLUSTERS); void -spdk_blob_opts_init(struct spdk_blob_opts *opts) +spdk_blob_opts_init(struct spdk_blob_opts *opts, size_t opts_size) { + opts->opts_size = opts_size; opts->num_clusters = 0; opts->thin_provision = false; opts->xattrs.count = 0;