lvol: add lvol unique name

Add a name to each lvol which is persisted as a blob
xattr.  lvol names must be unique within its
lvolstore.

While here, fix a few lvol_ut issues that were caught
as part of testing the lvol unique names.  Also fix
a couple of tests that registered the wrong string
name with CUnit.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I6d24d241e8f52158d14886f928d41823bbc93fa9

Reviewed-on: https://review.gerrithub.io/383567
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>

include/spdk/lvol.h

@@ -47,6 +47,7 @@ struct spdk_lvol;
 
 /* Must include null terminator. */
 #define SPDK_LVS_NAME_MAX	64
+#define SPDK_LVOL_NAME_MAX	64
 
 struct spdk_lvs_opts {
 	uint32_t	cluster_sz;
@@ -68,7 +69,7 @@ int spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o,
 int spdk_lvs_unload(struct spdk_lvol_store *lvol_store, spdk_lvs_op_complete cb_fn, void *cb_arg);
 int spdk_lvs_destroy(struct spdk_lvol_store *lvol_store, bool unmap_device,
 		     spdk_lvs_op_complete cb_fn, void *cb_arg);
-int spdk_lvol_create(struct spdk_lvol_store *lvs, uint64_t sz,
+int spdk_lvol_create(struct spdk_lvol_store *lvs, char *name, uint64_t sz,
 		     spdk_lvol_op_with_handle_complete cb_fn, void *cb_arg);
 void spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg);
 void spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg);

include/spdk_internal/lvolstore.h

@@ -97,6 +97,7 @@ struct spdk_lvol {
 	uint64_t			num_clusters;
 	spdk_blob_id			blob_id;
 	char				*old_name;
+	char				name[SPDK_LVOL_NAME_MAX];
 	bool				close_only;
 	struct spdk_bdev		*bdev;
 	int				ref_count;

lib/bdev/lvol/vbdev_lvol.c

@@ -574,6 +574,8 @@ vbdev_lvol_create(uuid_t uuid, size_t sz,
 {
 	struct spdk_lvol_with_handle_req *req;
 	struct spdk_lvol_store *lvs;
+	uuid_t lvol_uuid;
+	char name[SPDK_LVOL_NAME_MAX];
 	int rc;
 
 	lvs = vbdev_get_lvol_store_by_uuid(uuid);
@@ -588,7 +590,14 @@ vbdev_lvol_create(uuid_t uuid, size_t sz,
 	req->cb_fn = cb_fn;
 	req->cb_arg = cb_arg;
 
-	rc = spdk_lvol_create(lvs, sz, _vbdev_lvol_create_cb, req);
+	/*
+	 * This is temporary until the RPCs take a name parameter for creating
+	 *  an lvol.
+	 */
+	uuid_generate(lvol_uuid);
+	uuid_unparse(lvol_uuid, name);
+
+	rc = spdk_lvol_create(lvs, name, sz, _vbdev_lvol_create_cb, req);
 	if (rc != 0) {
 		free(req);
 	}

lib/lvol/lvol.c

@@ -842,6 +842,13 @@ _spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno)
 		goto invalid;
 	}
 
+	lvolerrno = spdk_blob_md_set_xattr(blob, "name", lvol->name,
+					   strnlen(lvol->name, SPDK_LVOL_NAME_MAX) + 1);
+	if (lvolerrno < 0) {
+		spdk_bs_md_close_blob(&blob, _spdk_lvol_destroy_cb, lvol);
+		goto invalid;
+	}
+
 	TAILQ_INSERT_TAIL(&lvol->lvol_store->lvols, lvol, link);
 
 	spdk_bs_md_sync_blob(blob, _spdk_lvol_sync_cb, req);
@@ -874,18 +881,36 @@ _spdk_lvol_create_cb(void *cb_arg, spdk_blob_id blobid, int lvolerrno)
 }
 
 int
-spdk_lvol_create(struct spdk_lvol_store *lvs, uint64_t sz,
+spdk_lvol_create(struct spdk_lvol_store *lvs, char *name, uint64_t sz,
 		 spdk_lvol_op_with_handle_complete cb_fn, void *cb_arg)
 {
 	struct spdk_lvol_with_handle_req *req;
 	struct spdk_blob_store *bs;
-	struct spdk_lvol *lvol;
+	struct spdk_lvol *lvol, *tmp;
 	uint64_t num_clusters, free_clusters;
 
 	if (lvs == NULL) {
 		SPDK_ERRLOG("lvol store does not exist\n");
 		return -ENODEV;
 	}
+
+	if (name == NULL || strnlen(name, SPDK_LVS_NAME_MAX) == 0) {
+		SPDK_ERRLOG("No name specified.\n");
+		return -EINVAL;
+	}
+
+	if (strnlen(name, SPDK_LVOL_NAME_MAX) == SPDK_LVOL_NAME_MAX) {
+		SPDK_ERRLOG("Name has no null terminator.\n");
+		return -EINVAL;
+	}
+
+	TAILQ_FOREACH(tmp, &lvs->lvols, link) {
+		if (!strncmp(name, tmp->name, SPDK_LVOL_NAME_MAX)) {
+			SPDK_ERRLOG("lvol with name %s already exists\n", name);
+			return -EINVAL;
+		}
+	}
+
 	bs = lvs->blobstore;
 
 	num_clusters = divide_round_up(sz, spdk_bs_get_cluster_size(bs));
@@ -914,6 +939,7 @@ spdk_lvol_create(struct spdk_lvol_store *lvs, uint64_t sz,
 	lvol->lvol_store = lvs;
 	lvol->num_clusters = num_clusters;
 	lvol->close_only = false;
+	strncpy(lvol->name, name, SPDK_LVS_NAME_MAX);
 	req->lvol = lvol;
 
 	spdk_bs_md_create_blob(lvs->blobstore, _spdk_lvol_create_cb, req);

test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c

@@ -381,8 +381,8 @@ _lvol_create(struct spdk_lvol_store *lvs)
 }
 
 int
-spdk_lvol_create(struct spdk_lvol_store *lvs, size_t sz, spdk_lvol_op_with_handle_complete cb_fn,
+spdk_lvol_create(struct spdk_lvol_store *lvs, char *name, size_t sz,
-		 void *cb_arg)
+		 spdk_lvol_op_with_handle_complete cb_fn, void *cb_arg)
 {
 	struct spdk_lvol *lvol;
 

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

@@ -70,6 +70,7 @@ int g_close_super_status;
 int g_resize_rc;
 struct spdk_lvol_store *g_lvol_store;
 struct spdk_lvol *g_lvol;
+spdk_blob_id g_blobid = 1;
 
 struct spdk_blob_store {
 	struct spdk_bs_opts	bs_opts;
@@ -350,8 +351,10 @@ spdk_bs_md_create_blob(struct spdk_blob_store *bs,
 	b = calloc(1, sizeof(*b));
 	SPDK_CU_ASSERT_FATAL(b != NULL);
 
+	b->id = g_blobid++;
+
 	TAILQ_INSERT_TAIL(&bs->blobs, b, link);
-	cb_fn(cb_arg, 0, 0);
+	cb_fn(cb_arg, b->id, 0);
 }
 
 static void
@@ -366,6 +369,13 @@ lvol_store_op_with_handle_complete(void *cb_arg, struct spdk_lvol_store *lvol_st
 	g_lvol_store = lvol_store;
 	g_lvserrno = lvserrno;
 }
+
+static void
+lvol_op_complete(void *cb_arg, int lvolerrno)
+{
+	g_lvolerrno = lvolerrno;
+}
+
 static void
 lvol_op_with_handle_complete(void *cb_arg, struct spdk_lvol *lvol, int lvserrno)
 {
@@ -413,7 +423,7 @@ lvs_init_unload_success(void)
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 	CU_ASSERT(!TAILQ_EMPTY(&g_lvol_stores));
 
-	spdk_lvol_create(g_lvol_store, 10, lvol_op_with_handle_complete, NULL);
+	spdk_lvol_create(g_lvol_store, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
 
@@ -461,7 +471,7 @@ lvs_init_destroy_success(void)
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 
-	spdk_lvol_create(g_lvol_store, 10, lvol_op_with_handle_complete, NULL);
+	spdk_lvol_create(g_lvol_store, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
 
@@ -663,7 +673,7 @@ lvol_create_destroy_success(void)
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 
-	spdk_lvol_create(g_lvol_store, 10, lvol_op_with_handle_complete, NULL);
+	spdk_lvol_create(g_lvol_store, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
 
@@ -708,11 +718,12 @@ lvol_create_fail(void)
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 
 	g_lvol = NULL;
-	rc = spdk_lvol_create(NULL, 10, lvol_op_with_handle_complete, NULL);
+	rc = spdk_lvol_create(NULL, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(rc != 0);
 	CU_ASSERT(g_lvol == NULL);
 
-	rc = spdk_lvol_create(g_lvol_store, DEV_BUFFER_SIZE + 1, lvol_op_with_handle_complete, NULL);
+	rc = spdk_lvol_create(g_lvol_store, "lvol", DEV_BUFFER_SIZE + 1,
+			      lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(rc != 0);
 	CU_ASSERT(g_lvol == NULL);
 
@@ -746,7 +757,7 @@ lvol_destroy_fail(void)
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 
-	spdk_lvol_create(g_lvol_store, 10, lvol_op_with_handle_complete, NULL);
+	spdk_lvol_create(g_lvol_store, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
 
@@ -785,7 +796,7 @@ lvol_close_fail(void)
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 
-	spdk_lvol_create(g_lvol_store, 10, lvol_op_with_handle_complete, NULL);
+	spdk_lvol_create(g_lvol_store, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
 
@@ -823,7 +834,7 @@ lvol_close_success(void)
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 
-	spdk_lvol_create(g_lvol_store, 10, lvol_op_with_handle_complete, NULL);
+	spdk_lvol_create(g_lvol_store, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
 
@@ -862,7 +873,7 @@ lvol_resize(void)
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
 
-	spdk_lvol_create(g_lvol_store, 10, lvol_op_with_handle_complete, NULL);
+	spdk_lvol_create(g_lvol_store, "lvol", 10, lvol_op_with_handle_complete, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
 
@@ -897,6 +908,7 @@ lvol_resize(void)
 	CU_ASSERT(rc != 0);
 	CU_ASSERT(g_lvserrno != 0);
 
+	g_resize_rc = 0;
 	spdk_lvol_close(g_lvol, close_cb, NULL);
 	CU_ASSERT(g_lvserrno == 0);
 	spdk_lvol_destroy(g_lvol, destroy_cb, NULL);
@@ -1179,11 +1191,97 @@ lvol_open(void)
 		CU_ASSERT(g_lvserrno == 0);
 	}
 
+	/* Close all lvols */
+	TAILQ_FOREACH_SAFE(lvol, &g_lvol_store->lvols, link, tmp) {
+		spdk_lvol_close(lvol, lvol_op_complete, NULL);
+		CU_ASSERT(g_lvserrno == 0);
+	}
+
 	g_lvserrno = -1;
-	spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL);
+	spdk_lvs_destroy(g_lvol_store, false, lvol_store_op_complete, NULL);
 
 	free(req);
-	free_dev(&dev);
+	free(blob1);
+	free(blob2);
+	free(blob3);
+
+	spdk_free_thread();
+}
+
+static void
+lvol_names(void)
+{
+	struct lvol_ut_bs_dev dev;
+	struct spdk_lvs_opts opts;
+	struct spdk_lvol_store *lvs;
+	struct spdk_lvol *lvol, *lvol2;
+	char fullname[SPDK_LVOL_NAME_MAX];
+	int rc = 0;
+
+	init_dev(&dev);
+
+	spdk_allocate_thread(_lvol_send_msg, NULL, NULL);
+
+	spdk_lvs_opts_init(&opts);
+	strncpy(opts.name, "lvs", sizeof(opts.name));
+
+	g_lvserrno = -1;
+	g_lvol_store = NULL;
+	rc = spdk_lvs_init(&dev.bs_dev, &opts, lvol_store_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(g_lvserrno == 0);
+	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
+	lvs = g_lvol_store;
+
+	rc = spdk_lvol_create(lvs, NULL, 1, lvol_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == -EINVAL);
+
+	rc = spdk_lvol_create(lvs, "", 1, lvol_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == -EINVAL);
+
+	memset(fullname, 'x', sizeof(fullname));
+	rc = spdk_lvol_create(lvs, fullname, 1, lvol_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == -EINVAL);
+
+	g_lvserrno = -1;
+	rc = spdk_lvol_create(lvs, "lvol", 1, lvol_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(g_lvserrno == 0);
+	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
+	lvol = g_lvol;
+
+	rc = spdk_lvol_create(lvs, "lvol", 1, lvol_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == -EINVAL);
+
+	g_lvserrno = -1;
+	rc = spdk_lvol_create(lvs, "lvol2", 1, lvol_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(g_lvserrno == 0);
+	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
+	lvol2 = g_lvol;
+
+	spdk_lvol_close(lvol, close_cb, NULL);
+	spdk_lvol_destroy(lvol, lvol_op_complete, NULL);
+
+	g_lvserrno = -1;
+	g_lvol = NULL;
+	rc = spdk_lvol_create(lvs, "lvol", 1, lvol_op_with_handle_complete, NULL);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(g_lvserrno == 0);
+	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);
+	lvol = g_lvol;
+
+	spdk_lvol_close(lvol, close_cb, NULL);
+	spdk_lvol_destroy(lvol, destroy_cb, NULL);
+
+	spdk_lvol_close(lvol2, close_cb, NULL);
+	spdk_lvol_destroy(lvol2, destroy_cb, NULL);
+
+	g_lvserrno = -1;
+	rc = spdk_lvs_destroy(lvs, false, lvol_store_op_complete, NULL);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(g_lvserrno == 0);
+	g_lvol_store = NULL;
 
 	spdk_free_thread();
 }
@@ -1216,9 +1314,10 @@ int main(int argc, char **argv)
 		CU_add_test(suite, "lvol_close_fail", lvol_close_fail) == NULL ||
 		CU_add_test(suite, "lvol_close_success", lvol_close_success) == NULL ||
 		CU_add_test(suite, "lvol_resize", lvol_resize) == NULL ||
-		CU_add_test(suite, "lvol_load", lvs_load) == NULL ||
+		CU_add_test(suite, "lvs_load", lvs_load) == NULL ||
-		CU_add_test(suite, "lvs_load", lvols_load) == NULL ||
+		CU_add_test(suite, "lvols_load", lvols_load) == NULL ||
-		CU_add_test(suite, "lvol_open", lvol_open) == NULL
+		CU_add_test(suite, "lvol_open", lvol_open) == NULL ||
+		CU_add_test(suite, "lvol_names", lvol_names) == NULL
 	) {
 		CU_cleanup_registry();
 		return CU_get_error();