From c894388d4b51654470988e1198c356719b55b741 Mon Sep 17 00:00:00 2001 From: Mike Gerdts Date: Fri, 6 Jan 2023 16:29:52 -0600 Subject: [PATCH] lib/lvol: make spdk_lvs_opts extensible This updates spdk_lvs_opts to be consistent with opther options structures in that it can now be extended with additional fields. The fields in spdk_lvs_opts are now documented as well. Signed-off-by: Mike Gerdts Change-Id: Ibd93c3a4aa1d2a33ac550d7056a69afece4dc592 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16172 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- include/spdk/lvol.h | 18 ++++++++++++- lib/lvol/lvol.c | 62 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/include/spdk/lvol.h b/include/spdk/lvol.h index b21f0d712..cebbab419 100644 --- a/include/spdk/lvol.h +++ b/include/spdk/lvol.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: BSD-3-Clause * Copyright (C) 2017 Intel Corporation. * All rights reserved. + * Copyright (c) 2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved. */ /** \file @@ -42,12 +43,27 @@ enum lvs_clear_method { * Parameters for lvolstore initialization. */ struct spdk_lvs_opts { + /** Size of cluster in bytes. Must be multiple of 4KiB page size. */ uint32_t cluster_sz; + + /** Clear method */ enum lvs_clear_method clear_method; + + /** Name of the lvolstore */ char name[SPDK_LVS_NAME_MAX]; + /** num_md_pages_per_cluster_ratio = 100 means 1 page per cluster */ uint32_t num_md_pages_per_cluster_ratio; -}; + + /** + * The size of spdk_lvol_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. After that, new added fields should be put in the end of the struct. + */ + uint32_t opts_size; +} __attribute__((packed)); +SPDK_STATIC_ASSERT(sizeof(struct spdk_lvs_opts) == 80, "Incorrect size"); /** * Initialize an spdk_lvs_opts structure to the defaults. diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index 2adbaa35e..14fe93004 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: BSD-3-Clause * Copyright (C) 2017 Intel Corporation. * All rights reserved. - * Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * Copyright (c) 2022-2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved. */ #include "spdk_internal/lvolstore.h" @@ -21,6 +21,7 @@ SPDK_LOG_REGISTER_COMPONENT(lvol) static TAILQ_HEAD(, spdk_lvol_store) g_lvol_stores = TAILQ_HEAD_INITIALIZER(g_lvol_stores); static pthread_mutex_t g_lvol_stores_mutex = PTHREAD_MUTEX_INITIALIZER; +static inline int lvs_opts_copy(const struct spdk_lvs_opts *src, struct spdk_lvs_opts *dst); static int add_lvs_to_list(struct spdk_lvol_store *lvs) { @@ -561,10 +562,46 @@ lvs_init_cb(void *cb_arg, struct spdk_blob_store *bs, int lvserrno) void spdk_lvs_opts_init(struct spdk_lvs_opts *o) { + memset(o, 0, sizeof(*o)); o->cluster_sz = SPDK_LVS_OPTS_CLUSTER_SZ; o->clear_method = LVS_CLEAR_WITH_UNMAP; o->num_md_pages_per_cluster_ratio = 100; - memset(o->name, 0, sizeof(o->name)); + o->opts_size = sizeof(*o); +} + +static inline int +lvs_opts_copy(const struct spdk_lvs_opts *src, struct spdk_lvs_opts *dst) +{ + if (src->opts_size == 0) { + SPDK_ERRLOG("opts_size should not be zero value\n"); + return -1; + } +#define FIELD_OK(field) \ + offsetof(struct spdk_lvs_opts, field) + sizeof(src->field) <= src->opts_size + +#define SET_FIELD(field) \ + if (FIELD_OK(field)) { \ + dst->field = src->field; \ + } \ + + SET_FIELD(cluster_sz); + SET_FIELD(clear_method); + if (FIELD_OK(name)) { + memcpy(&dst->name, &src->name, sizeof(dst->name)); + } + SET_FIELD(num_md_pages_per_cluster_ratio); + SET_FIELD(opts_size); + + 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_lvs_opts) == 80, "Incorrect size"); + +#undef FIELD_OK +#undef SET_FIELD + + return 0; } static void @@ -584,6 +621,7 @@ spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o, struct spdk_lvol_store *lvs; struct spdk_lvs_with_handle_req *lvs_req; struct spdk_bs_opts opts = {}; + struct spdk_lvs_opts lvs_opts; uint32_t total_clusters; int rc; @@ -597,21 +635,27 @@ spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o, return -EINVAL; } - if (o->cluster_sz < bs_dev->blocklen) { - SPDK_ERRLOG("Cluster size %" PRIu32 " is smaller than blocklen %" PRIu32 "\n", - o->cluster_sz, bs_dev->blocklen); + spdk_lvs_opts_init(&lvs_opts); + if (lvs_opts_copy(o, &lvs_opts) != 0) { + SPDK_ERRLOG("spdk_lvs_opts invalid\n"); return -EINVAL; } - total_clusters = bs_dev->blockcnt / (o->cluster_sz / bs_dev->blocklen); + + if (lvs_opts.cluster_sz < bs_dev->blocklen) { + SPDK_ERRLOG("Cluster size %" PRIu32 " is smaller than blocklen %" PRIu32 "\n", + lvs_opts.cluster_sz, bs_dev->blocklen); + return -EINVAL; + } + total_clusters = bs_dev->blockcnt / (lvs_opts.cluster_sz / bs_dev->blocklen); setup_lvs_opts(&opts, o, total_clusters); - if (strnlen(o->name, SPDK_LVS_NAME_MAX) == SPDK_LVS_NAME_MAX) { + if (strnlen(lvs_opts.name, SPDK_LVS_NAME_MAX) == SPDK_LVS_NAME_MAX) { SPDK_ERRLOG("Name has no null terminator.\n"); return -EINVAL; } - if (strnlen(o->name, SPDK_LVS_NAME_MAX) == 0) { + if (strnlen(lvs_opts.name, SPDK_LVS_NAME_MAX) == 0) { SPDK_ERRLOG("No name specified.\n"); return -EINVAL; } @@ -623,7 +667,7 @@ spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o, } spdk_uuid_generate(&lvs->uuid); - snprintf(lvs->name, sizeof(lvs->name), "%s", o->name); + snprintf(lvs->name, sizeof(lvs->name), "%s", lvs_opts.name); rc = add_lvs_to_list(lvs); if (rc) {