Commit 08ac0074 authored by Jim Harris's avatar Jim Harris
Browse files

lvol: add unique lvolstore name 



Each lvolstore now has a unique name which is persisted to
metadata.  Eventually this will provide a friendlier
way to reference logical volumes, in the form
lvolstore_name/lvol_name.

This patch only covers the unique lvolstore name.  An
lvolstore may not be created nor loaded if its name
conflicts with an already loaded blobstore.

Currently there is no way to rename an lvolstore to
resolve a conflict.  This will be coming in the future.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I842f13b79776e5b8f81e56de10778c35328e8cd3
Reviewed-on: https://review.gerrithub.io/383533


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarMaciej Szwed <maciej.szwed@intel.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent 2120e4d3

include/spdk/lvol.h

+5 −1
Original line number Diff line number Diff line
@@ -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;

	char		name[SPDK_LVS_NAME_MAX];

};



/* Initialize an spdk_lvs_opts structure to the default logical volume store option values. */

include/spdk_internal/lvolstore.h

+1 −0
Original line number Diff line number Diff line
@@ -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 {

lib/bdev/lvol/vbdev_lvol.c

+8 −0
Original line number Diff line number Diff line
@@ -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");

lib/lvol/lvol.c

+50 −9
Original line number Diff line number Diff line
@@ -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);

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

test/unit/lib/lvol/lvol.c/lvol_ut.c

+139 −1
Original line number Diff line number Diff line
@@ -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 ||