diff --git a/include/spdk/lvol.h b/include/spdk/lvol.h index 3376e00d6..6019c6a78 100644 --- a/include/spdk/lvol.h +++ b/include/spdk/lvol.h @@ -45,8 +45,12 @@ struct spdk_lvol_store; struct spdk_lvol; +/* Must include null terminator. */ +#define SPDK_LVS_NAME_MAX 64 + struct spdk_lvs_opts { - uint32_t cluster_sz; + uint32_t cluster_sz; + char name[SPDK_LVS_NAME_MAX]; }; /* Initialize an spdk_lvs_opts structure to the default logical volume store option values. */ diff --git a/include/spdk_internal/lvolstore.h b/include/spdk_internal/lvolstore.h index b5382661f..f202647a5 100644 --- a/include/spdk_internal/lvolstore.h +++ b/include/spdk_internal/lvolstore.h @@ -88,6 +88,7 @@ struct spdk_lvol_store { TAILQ_HEAD(, spdk_lvol) lvols; bool on_list; TAILQ_ENTRY(spdk_lvol_store) link; + char name[SPDK_LVS_NAME_MAX]; }; struct spdk_lvol { diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 12442d85c..05d947236 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -107,6 +107,7 @@ vbdev_lvs_create(struct spdk_bdev *base_bdev, uint32_t cluster_sz, struct spdk_bs_dev *bs_dev; struct spdk_lvs_with_handle_req *lvs_req; struct spdk_lvs_opts opts; + uuid_t uuid; int rc; if (base_bdev == NULL) { @@ -119,6 +120,13 @@ vbdev_lvs_create(struct spdk_bdev *base_bdev, uint32_t cluster_sz, opts.cluster_sz = cluster_sz; } + /* + * This is temporary until the RPCs take a name parameter for creating + * an lvolstore. + */ + uuid_generate(uuid); + uuid_unparse(uuid, opts.name); + lvs_req = calloc(1, sizeof(*lvs_req)); if (!lvs_req) { SPDK_ERRLOG("Cannot alloc memory for vbdev lvol store request pointer\n"); diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index 669ba5cf4..199d1b4a4 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -54,12 +54,22 @@ divide_round_up(size_t num, size_t divisor) static int _spdk_add_lvs_to_list(struct spdk_lvol_store *lvs) { + struct spdk_lvol_store *tmp; + bool name_conflict = false; + pthread_mutex_lock(&g_lvol_stores_mutex); - lvs->on_list = true; - TAILQ_INSERT_TAIL(&g_lvol_stores, lvs, link); + TAILQ_FOREACH(tmp, &g_lvol_stores, link) { + if (!strncmp(lvs->name, tmp->name, SPDK_LVS_NAME_MAX)) { + name_conflict = true; + } + } + if (!name_conflict) { + lvs->on_list = true; + TAILQ_INSERT_TAIL(&g_lvol_stores, lvs, link); + } pthread_mutex_unlock(&g_lvol_stores_mutex); - return 0; + return name_conflict ? -1 : 0; } static void @@ -259,12 +269,21 @@ _spdk_lvs_read_uuid(void *cb_arg, struct spdk_blob *blob, int lvolerrno) return; } - /* - * Wait to put lvs on the global list until this point, because a future patch - * will add lvolstore name uniqueness checking - and we don't know a loaded - * lvolstore's name until this point. - */ - _spdk_add_lvs_to_list(lvs); + rc = spdk_bs_md_get_xattr_value(blob, "name", (const void **)&attr, &value_len); + if (rc != 0 || value_len > SPDK_LVS_NAME_MAX) { + SPDK_INFOLOG(SPDK_TRACE_LVOL, "missing or invalid name\n"); + spdk_bs_md_close_blob(&blob, _spdk_close_super_blob_with_error_cb, req); + return; + } + + strncpy(lvs->name, attr, value_len); + + rc = _spdk_add_lvs_to_list(lvs); + if (rc) { + SPDK_INFOLOG(SPDK_TRACE_LVOL, "lvolstore with name %s already exists\n", lvs->name); + spdk_bs_md_close_blob(&blob, _spdk_close_super_blob_with_error_cb, req); + return; + } lvs->super_blob_id = spdk_blob_get_id(blob); @@ -402,6 +421,7 @@ _spdk_super_blob_init_cb(void *cb_arg, int lvolerrno) uuid_unparse(lvs->uuid, uuid); spdk_blob_md_set_xattr(blob, "uuid", uuid, UUID_STRING_LEN); + spdk_blob_md_set_xattr(blob, "name", lvs->name, strnlen(lvs->name, SPDK_LVS_NAME_MAX) + 1); spdk_bs_md_sync_blob(blob, _spdk_super_blob_set_cb, req); } @@ -476,6 +496,7 @@ void spdk_lvs_opts_init(struct spdk_lvs_opts *o) { o->cluster_sz = SPDK_LVS_OPTS_CLUSTER_SZ; + memset(o->name, 0, sizeof(o->name)); } static void @@ -493,6 +514,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 = {}; + int rc; if (bs_dev == NULL) { SPDK_ERRLOG("Blobstore device does not exist\n"); @@ -506,6 +528,16 @@ spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o, _spdk_setup_lvs_opts(&opts, o); + if (strnlen(o->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) { + SPDK_ERRLOG("No name specified.\n"); + return -EINVAL; + } + lvs = calloc(1, sizeof(*lvs)); if (!lvs) { SPDK_ERRLOG("Cannot alloc memory for lvol store base pointer\n"); @@ -513,6 +545,15 @@ spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o, } uuid_generate_time(lvs->uuid); + strncpy(lvs->name, o->name, SPDK_LVS_NAME_MAX); + + rc = _spdk_add_lvs_to_list(lvs); + + if (rc) { + SPDK_ERRLOG("lvolstore with name %s already exists\n", lvs->name); + _spdk_lvs_free(lvs); + return -EINVAL; + } _spdk_add_lvs_to_list(lvs); diff --git a/test/unit/lib/lvol/lvol.c/lvol_ut.c b/test/unit/lib/lvol/lvol.c/lvol_ut.c index b842e08d7..4e2a729ca 100644 --- a/test/unit/lib/lvol/lvol.c/lvol_ut.c +++ b/test/unit/lib/lvol/lvol.c/lvol_ut.c @@ -61,6 +61,7 @@ struct spdk_blob { int open_status; TAILQ_ENTRY(spdk_blob) link; char uuid[UUID_STRING_LEN]; + char name[SPDK_LVS_NAME_MAX]; }; int g_lvolerrno; @@ -164,6 +165,9 @@ spdk_blob_md_set_xattr(struct spdk_blob *blob, const char *name, const void *val if (!strcmp(name, "uuid")) { CU_ASSERT(value_len == UUID_STRING_LEN); memcpy(blob->uuid, value, UUID_STRING_LEN); + } else if (!strcmp(name, "name")) { + CU_ASSERT(value_len <= SPDK_LVS_NAME_MAX); + memcpy(blob->name, value, value_len); } return 0; @@ -178,6 +182,10 @@ spdk_bs_md_get_xattr_value(struct spdk_blob *blob, const char *name, *value = blob->uuid; *value_len = UUID_STRING_LEN; return 0; + } else if (!strcmp(name, "name") && strnlen(blob->name, SPDK_LVS_NAME_MAX) != 0) { + *value = blob->name; + *value_len = strnlen(blob->name, SPDK_LVS_NAME_MAX) + 1; + return 0; } return -ENOENT; @@ -394,6 +402,7 @@ lvs_init_unload_success(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); g_lvserrno = -1; @@ -443,6 +452,7 @@ lvs_init_destroy_success(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); g_lvserrno = -1; @@ -489,6 +499,8 @@ lvs_init_opts_success(void) g_lvserrno = -1; + spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); opts.cluster_sz = 8192; rc = spdk_lvs_init(&dev.bs_dev, &opts, lvol_store_op_with_handle_complete, NULL); CU_ASSERT(rc == 0); @@ -522,6 +534,115 @@ lvs_unload_lvs_is_null_fail(void) spdk_free_thread(); } +static void +lvs_names(void) +{ + struct lvol_ut_bs_dev dev_x, dev_y, dev_x2; + struct spdk_lvs_opts opts_none, opts_x, opts_y, opts_full; + struct spdk_lvol_store *lvs_x, *lvs_y, *lvs_x2; + int rc = 0; + + init_dev(&dev_x); + init_dev(&dev_y); + init_dev(&dev_x2); + + spdk_allocate_thread(_lvol_send_msg, NULL, NULL); + + spdk_lvs_opts_init(&opts_none); + spdk_lvs_opts_init(&opts_x); + opts_x.name[0] = 'x'; + spdk_lvs_opts_init(&opts_y); + opts_y.name[0] = 'y'; + spdk_lvs_opts_init(&opts_full); + memset(opts_full.name, 'a', sizeof(opts_full.name)); + + /* Test that opts with no name fails spdk_lvs_init(). */ + CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); + rc = spdk_lvs_init(&dev_x.bs_dev, &opts_none, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc != 0); + CU_ASSERT(g_lvol_store == NULL); + CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); + + /* Test that opts with no null terminator for name fails spdk_lvs_init(). */ + CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); + rc = spdk_lvs_init(&dev_x.bs_dev, &opts_full, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc != 0); + CU_ASSERT(g_lvol_store == NULL); + CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); + + /* Test that we can create an lvolstore with name 'x'. */ + CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); + g_lvol_store = NULL; + rc = spdk_lvs_init(&dev_x.bs_dev, &opts_x, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(!TAILQ_EMPTY(&g_lvol_stores)); + SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); + lvs_x = g_lvol_store; + + /* Test that we can create an lvolstore with name 'y'. */ + g_lvol_store = NULL; + rc = spdk_lvs_init(&dev_y.bs_dev, &opts_y, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc == 0); + SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); + lvs_y = g_lvol_store; + + /* Test that we cannot create another lvolstore with name 'x'. */ + rc = spdk_lvs_init(&dev_x2.bs_dev, &opts_x, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc == -EINVAL); + + /* Now destroy lvolstore 'x' and then confirm we can create a new lvolstore with name 'x'. */ + g_lvserrno = -1; + rc = spdk_lvs_destroy(lvs_x, false, lvol_store_op_complete, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(g_lvserrno == 0); + g_lvol_store = NULL; + rc = spdk_lvs_init(&dev_x.bs_dev, &opts_x, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc == 0); + SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); + lvs_x = g_lvol_store; + + /* + * Unload lvolstore 'x'. Then we should be able to create another lvolstore with name 'x'. + */ + g_lvserrno = -1; + rc = spdk_lvs_unload(lvs_x, lvol_store_op_complete, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(g_lvserrno == 0); + g_lvol_store = NULL; + rc = spdk_lvs_init(&dev_x2.bs_dev, &opts_x, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc == 0); + SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); + lvs_x2 = g_lvol_store; + + /* Confirm that we cannot load the first lvolstore 'x'. */ + g_lvserrno = 0; + spdk_lvs_load(&dev_x.bs_dev, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(g_lvserrno != 0); + + /* Destroy the second lvolstore 'x'. Then we should be able to load the first lvolstore 'x'. */ + g_lvserrno = -1; + rc = spdk_lvs_destroy(lvs_x2, false, lvol_store_op_complete, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(g_lvserrno == 0); + g_lvserrno = -1; + spdk_lvs_load(&dev_x.bs_dev, lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(g_lvserrno == 0); + SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); + lvs_x = g_lvol_store; + + g_lvserrno = -1; + rc = spdk_lvs_destroy(lvs_x, false, lvol_store_op_complete, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(g_lvserrno == 0); + + g_lvserrno = -1; + rc = spdk_lvs_destroy(lvs_y, false, lvol_store_op_complete, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(g_lvserrno == 0); + + spdk_free_thread(); +} + static void lvol_create_destroy_success(void) { @@ -534,6 +655,7 @@ lvol_create_destroy_success(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); g_lvserrno = -1; rc = spdk_lvs_init(&dev.bs_dev, &opts, lvol_store_op_with_handle_complete, NULL); @@ -573,6 +695,7 @@ lvol_create_fail(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); g_lvol_store = NULL; g_lvserrno = 0; @@ -616,6 +739,7 @@ lvol_destroy_fail(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); rc = spdk_lvs_init(&dev.bs_dev, &opts, lvol_store_op_with_handle_complete, NULL); CU_ASSERT(rc == 0); @@ -654,6 +778,7 @@ lvol_close_fail(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); rc = spdk_lvs_init(&dev.bs_dev, &opts, lvol_store_op_with_handle_complete, NULL); CU_ASSERT(rc == 0); @@ -690,6 +815,7 @@ lvol_close_success(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); g_lvserrno = -1; rc = spdk_lvs_init(&dev.bs_dev, &opts, lvol_store_op_with_handle_complete, NULL); @@ -727,6 +853,7 @@ lvol_resize(void) spdk_allocate_thread(_lvol_send_msg, NULL, NULL); spdk_lvs_opts_init(&opts); + strncpy(opts.name, "lvs", sizeof(opts.name)); g_resize_rc = 0; g_lvserrno = -1; @@ -848,9 +975,17 @@ lvs_load(void) CU_ASSERT(g_lvol_store == NULL); CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); - /* Fail on closing super blob */ + /* Fail on getting name */ g_lvserrno = 0; spdk_blob_md_set_xattr(super_blob, "uuid", uuid, UUID_STRING_LEN); + spdk_lvs_load(&dev.bs_dev, lvol_store_op_with_handle_complete, req); + CU_ASSERT(g_lvserrno == -ENODEV); + CU_ASSERT(g_lvol_store == NULL); + CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); + + /* Fail on closing super blob */ + g_lvserrno = 0; + spdk_blob_md_set_xattr(super_blob, "name", "lvs", strnlen("lvs", SPDK_LVS_NAME_MAX) + 1); super_blob->close_status = -1; spdk_lvs_load(&dev.bs_dev, lvol_store_op_with_handle_complete, req); CU_ASSERT(g_lvserrno == -ENODEV); @@ -897,6 +1032,7 @@ lvols_load(void) SPDK_CU_ASSERT_FATAL(super_blob != NULL); super_blob->id = 0x100; spdk_blob_md_set_xattr(super_blob, "uuid", uuid, UUID_STRING_LEN); + spdk_blob_md_set_xattr(super_blob, "name", "lvs", strnlen("lvs", SPDK_LVS_NAME_MAX) + 1); TAILQ_INSERT_TAIL(&dev.bs->blobs, super_blob, link); dev.bs->super_blobid = 0x100; @@ -983,6 +1119,7 @@ lvol_open(void) SPDK_CU_ASSERT_FATAL(super_blob != NULL); super_blob->id = 0x100; spdk_blob_md_set_xattr(super_blob, "uuid", uuid, UUID_STRING_LEN); + spdk_blob_md_set_xattr(super_blob, "name", "lvs", strnlen("lvs", SPDK_LVS_NAME_MAX) + 1); TAILQ_INSERT_TAIL(&dev.bs->blobs, super_blob, link); dev.bs->super_blobid = 0x100; @@ -1072,6 +1209,7 @@ int main(int argc, char **argv) CU_add_test(suite, "lvs_init_destroy_success", lvs_init_destroy_success) == NULL || CU_add_test(suite, "lvs_init_opts_success", lvs_init_opts_success) == NULL || CU_add_test(suite, "lvs_unload_lvs_is_null_fail", lvs_unload_lvs_is_null_fail) == NULL || + CU_add_test(suite, "lvs_names", lvs_names) == NULL || CU_add_test(suite, "lvol_create_destroy_success", lvol_create_destroy_success) == NULL || CU_add_test(suite, "lvol_create_fail", lvol_create_fail) == NULL || CU_add_test(suite, "lvol_destroy_fail", lvol_destroy_fail) == NULL ||