bdev: register bdev's UUID as its alias

In many cases, addressing bdevs by their UUIDs is often easier than using their names, which can be somewhat arbitrary. For instance, the NVMe bdev builds a name by addng the n{NSID} suffix to the controller's name, while the UUID is filled with NGUID (if available). The UUID alias is stored in the form defined by RFC 4122, meaning five groups of lower-case hexadecimal characters. It's important to note that bdev layer uses case-sensitive name comparison, so the user needs to use the same textual UUID representation. Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com> Change-Id: I8b112fb81f29e952459d5f81d97fdc7a591730f8 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11395 Tested-by: SPDK CI Jenkins <sys_sgci@intel.com> Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com> Community-CI: Mellanox Build Bot Reviewed-by: Jim Harris <james.r.harris@intel.com> Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2022-02-03 09:03:15 +01:00 · 2022-02-03 09:03:15 +01:00 · 79415753ea
commit 79415753ea
parent a5895656ed
8 changed files with 120 additions and 27 deletions
--- a/lib/bdev/bdev.c
+++ b/lib/bdev/bdev.c
@ -5717,6 +5717,7 @@ static int
 bdev_register(struct spdk_bdev *bdev)
 {
 	char *bdev_name;
+	char uuid[SPDK_UUID_STRING_LEN];
 	int ret;

 	assert(bdev->module != NULL);
@ -5761,6 +5762,18 @@ bdev_register(struct spdk_bdev *bdev)
 		spdk_uuid_generate(&bdev->uuid);
 	}

+	/* Add the UUID alias only if it's different than the name */
+	spdk_uuid_fmt_lower(uuid, sizeof(uuid), &bdev->uuid);
+	if (strcmp(bdev->name, uuid) != 0) {
+		ret = spdk_bdev_alias_add(bdev, uuid);
+		if (ret != 0) {
+			SPDK_ERRLOG("Unable to add uuid:%s alias for bdev %s\n", uuid, bdev->name);
+			bdev_name_del(&bdev->internal.bdev_name);
+			free(bdev_name);
+			return ret;
+		}
+	}
+
 	if (spdk_bdev_get_buf_align(bdev) > 1) {
 		if (bdev->split_on_optimal_io_boundary) {
 			bdev->optimal_io_boundary = spdk_min(bdev->optimal_io_boundary,
@ -5889,6 +5902,7 @@ bdev_unregister_unsafe(struct spdk_bdev *bdev)
 {
 	struct spdk_bdev_desc	*desc, *tmp;
 	int			rc = 0;
+	char			uuid[SPDK_UUID_STRING_LEN];

 	/* Notify each descriptor about hotremoval */
 	TAILQ_FOREACH_SAFE(desc, &bdev->internal.open_descs, link, tmp) {
@ -5909,7 +5923,12 @@ bdev_unregister_unsafe(struct spdk_bdev *bdev)
 	if (rc == 0) {
 		TAILQ_REMOVE(&g_bdev_mgr.bdevs, bdev, internal.link);
 		SPDK_DEBUGLOG(bdev, "Removing bdev %s from list done\n", bdev->name);
+
+		/* Delete the name and the UUID alias */
+		spdk_uuid_fmt_lower(uuid, sizeof(uuid), &bdev->uuid);
 		bdev_name_del_unsafe(&bdev->internal.bdev_name);
+		bdev_alias_del(bdev, uuid, bdev_name_del_unsafe);
+
 		spdk_notify_send("bdev_unregister", spdk_bdev_get_name(bdev));
 	}

--- a/test/nvmf/host/async_init.sh
+++ b/test/nvmf/host/async_init.sh
@ -12,6 +12,10 @@ null_block_size=512
 null_bdev=null0
 nvme_bdev=nvme0

+# Since we're connecting the same bdev, we need to use a different NGUID to avoid errors when
+# registering the bdev during bdev_nvme_attach_controller
+nguid=$(uuidgen | tr -d '-')
+
 if [ "$TEST_TRANSPORT" != "tcp" ]; then
 	echo "This test can only be executed with TCP for now"
 	exit 0
@ -25,7 +29,7 @@ $rpc_py nvmf_create_transport $NVMF_TRANSPORT_OPTS
 $rpc_py bdev_null_create $null_bdev $null_bdev_size $null_block_size
 $rpc_py bdev_wait_for_examine
 $rpc_py nvmf_create_subsystem nqn.2016-06.io.spdk:cnode0 -a
-$rpc_py nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode0 $null_bdev
+$rpc_py nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode0 $null_bdev -g $nguid
 $rpc_py nvmf_subsystem_add_listener nqn.2016-06.io.spdk:cnode0 -t $TEST_TRANSPORT \
 	-a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT

--- a/test/spdkcli/match_files/spdkcli_details_vhost.test.match
+++ b/test/spdkcli/match_files/spdkcli_details_vhost.test.match
@ -1,5 +1,7 @@
 {
-  "aliases": [],
+  "aliases": [
+    "$(UUID)"
+  ],
  "assigned_rate_limits": {
    "r_mbytes_per_sec": $(N),
    "rw_ios_per_sec": $(N),
--- a/test/spdkcli/match_files/spdkcli_pmem.test.match
+++ b/test/spdkcli/match_files/spdkcli_pmem.test.match
@ -1,3 +1,3 @@
 o- pmemblk .............................................................................................................. [Bdevs: 2]
-  o- pmem_bdev0 .......................................................................................... [Size=31.6M, Not claimed]
-  o- pmem_bdev1 .......................................................................................... [Size=31.6M, Not claimed]
+  o- pmem_bdev0 $(S) [$(UUID), Size=31.6M, Not claimed]
+  o- pmem_bdev1 $(S) [$(UUID), Size=31.6M, Not claimed]
--- a/test/spdkcli/match_files/spdkcli_raid.test.match
+++ b/test/spdkcli/match_files/spdkcli_raid.test.match
@ -4,13 +4,13 @@ o- bdevs .......................................................................
  o- iscsi .............................................................................................................. [Bdevs: 0]
  o- logical_volume ..................................................................................................... [Bdevs: 0]
  o- malloc ............................................................................................................. [Bdevs: 2]
-  | o- Malloc1 ................................................................................................ [Size=8.0M, Claimed]
-  | o- Malloc2 ................................................................................................ [Size=8.0M, Claimed]
+  | o- Malloc1 $(S) [$(UUID), Size=8.0M, Claimed]
+  | o- Malloc2 $(S) [$(UUID), Size=8.0M, Claimed]
  o- null ............................................................................................................... [Bdevs: 0]
  o- nvme ............................................................................................................... [Bdevs: 0]
  o- pmemblk ............................................................................................................ [Bdevs: 0]
  o- raid_volume ........................................................................................................ [Bdevs: 1]
-  | o- testraid .......................................................................................... [Size=16.0M, Not claimed]
+  | o- testraid $(S) [$(UUID), Size=16.0M, Not claimed]
  o- rbd ................................................................................................................ [Bdevs: 0]
  o- split_disk ......................................................................................................... [Bdevs: 0]
  o- virtioblk_disk ..................................................................................................... [Bdevs: 0]
--- a/test/spdkcli/match_files/spdkcli_rbd.test.match
+++ b/test/spdkcli/match_files/spdkcli_rbd.test.match
@ -1,3 +1,3 @@
 o- rbd .................................................................................................................. [Bdevs: 2]
-  o- Ceph0 ............................................................................................. [Size=1000.0M, Not claimed]
-  o- Ceph1 ............................................................................................. [Size=1000.0M, Not claimed]
+  o- Ceph0 $(S) [$(UUID), Size=1000.0M, Not claimed]
+  o- Ceph1 $(S) [$(UUID), Size=1000.0M, Not claimed]
--- a/test/spdkcli/match_files/spdkcli_vhost.test.match
+++ b/test/spdkcli/match_files/spdkcli_vhost.test.match
@ -1,35 +1,35 @@
 o- / ......................................................................................................................... [...]
  o- bdevs ................................................................................................................... [...]
  | o- aio .............................................................................................................. [Bdevs: 2]
-  | | o- sample0 .......................................................................................... [Size=$(FP)M, Not claimed]
-  | | o- sample1 .......................................................................................... [Size=$(FP)M, Not claimed]
+  | | o- sample0 $(S) [$(UUID), Size=$(FP)M, Not claimed]
+  | | o- sample1 $(S) [$(UUID), Size=$(FP)M, Not claimed]
  | o- error ............................................................................................................ [Bdevs: 2]
-  | | o- EE_Malloc1 ...................................................................................... [Size=$(FP)M, Not claimed]
-  | | o- EE_Malloc4 ...................................................................................... [Size=$(FP)M, Not claimed]
+  | | o- EE_Malloc1 $(S) [$(UUID), Size=$(FP)M, Not claimed]
+  | | o- EE_Malloc4 $(S) [$(UUID), Size=$(FP)M, Not claimed]
  | o- iscsi ............................................................................................................ [Bdevs: 0]
  | o- logical_volume ................................................................................................... [Bdevs: 2]
  | | o- $(UUID) ................................................ [lvs0/lvol$(FP), Size=$(FP)M, Not claimed]
  | | o- $(UUID) ................................................ [lvs0/lvol$(FP), Size=$(FP)M, Not claimed]
  | o- malloc ........................................................................................................... [Bdevs: 6]
-  | | o- Malloc0 ............................................................................................. [Size=$(FP)M, Claimed]
-  | | o- Malloc1 ............................................................................................. [Size=$(FP)M, Claimed]
-  | | o- Malloc2 ......................................................................................... [Size=$(FP)M, Not claimed]
-  | | o- Malloc3 ......................................................................................... [Size=$(FP)M, Not claimed]
-  | | o- Malloc4 ............................................................................................. [Size=$(FP)M, Claimed]
-  | | o- Malloc5 ............................................................................................. [Size=$(FP)M, Claimed]
+  | | o- Malloc0 $(S) [$(UUID), Size=$(FP)M, Claimed]
+  | | o- Malloc1 $(S) [$(UUID), Size=$(FP)M, Claimed]
+  | | o- Malloc2 $(S) [$(UUID), Size=$(FP)M, Not claimed]
+  | | o- Malloc3 $(S) [$(UUID), Size=$(FP)M, Not claimed]
+  | | o- Malloc4 $(S) [$(UUID), Size=$(FP)M, Claimed]
+  | | o- Malloc5 $(S) [$(UUID), Size=$(FP)M, Claimed]
  | o- null ............................................................................................................. [Bdevs: 2]
-  | | o- null_bdev0 ...................................................................................... [Size=$(FP)M, Not claimed]
-  | | o- null_bdev1 ...................................................................................... [Size=$(FP)M, Not claimed]
+  | | o- null_bdev0 $(S) [$(UUID), Size=$(FP)M, Not claimed]
+  | | o- null_bdev1 $(S) [$(UUID), Size=$(FP)M, Not claimed]
  | o- nvme ............................................................................................................. [Bdevs: 1]
-  | | o- Nvme0n1 $(S) [Size=$(S), Claimed]
+  | | o- Nvme0n1 $(S) [$(UUID), Size=$(S), Claimed]
  | o- pmemblk .......................................................................................................... [Bdevs: 0]
  | o- raid_volume ...................................................................................................... [Bdevs: 0]
  | o- rbd .............................................................................................................. [Bdevs: 0]
  | o- split_disk ....................................................................................................... [Bdevs: 4]
-  | | o- Nvme0n1p0 $(S) [Size=$(FP)G, Not claimed]
-  | | o- Nvme0n1p1 $(S) [Size=$(FP)G, Not claimed]
-  | | o- Nvme0n1p2 $(S) [Size=$(FP)G, Not claimed]
-  | | o- Nvme0n1p3 $(S) [Size=$(FP)G, Not claimed]
+  | | o- Nvme0n1p0 $(S) [$(UUID), Size=$(FP)G, Not claimed]
+  | | o- Nvme0n1p1 $(S) [$(UUID), Size=$(FP)G, Not claimed]
+  | | o- Nvme0n1p2 $(S) [$(UUID), Size=$(FP)G, Not claimed]
+  | | o- Nvme0n1p3 $(S) [$(UUID), Size=$(FP)G, Not claimed]
  | o- virtioblk_disk ................................................................................................... [Bdevs: 0]
  | o- virtioscsi_disk .................................................................................................. [Bdevs: 0]
  o- lvol_stores .................................................................................................. [Lvol stores: 2]
--- a/test/unit/lib/bdev/bdev.c/bdev_ut.c
+++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c
@ -4823,7 +4823,7 @@ bdev_multi_allocation(void)
 		for (j = 0; j < bdev_num; j++) {
 			bdev[j] = allocate_bdev(name[j]);
 			height = rb_tree_get_height(&bdev[j]->internal.bdev_name);
-			CU_ASSERT(height <= (int)(spdk_u32log2(j + 1)));
+			CU_ASSERT(height <= (int)(spdk_u32log2(2 * j + 2)));
 		}
 		SPDK_NOTICELOG("alloc bdev num %d takes %" PRIu64 " ms\n", bdev_num,
 			       (get_ns_time() - last_time) / 1000 / 1000);
@ -4950,6 +4950,73 @@ bdev_writev_readv_ext(void)
 	poll_threads();
 }

+static void
+bdev_register_uuid_alias(void)
+{
+	struct spdk_bdev *bdev, *second;
+	char uuid[SPDK_UUID_STRING_LEN];
+	int rc;
+
+	spdk_bdev_initialize(bdev_init_cb, NULL);
+	bdev = allocate_bdev("bdev0");
+
+	/* Make sure an UUID was generated  */
+	CU_ASSERT_FALSE(spdk_mem_all_zero(&bdev->uuid, sizeof(bdev->uuid)));
+
+	/* Check that an UUID alias was registered */
+	spdk_uuid_fmt_lower(uuid, sizeof(uuid), &bdev->uuid);
+	CU_ASSERT_EQUAL(spdk_bdev_get_by_name(uuid), bdev);
+
+	/* Unregister the bdev */
+	spdk_bdev_unregister(bdev, NULL, NULL);
+	poll_threads();
+	CU_ASSERT_PTR_NULL(spdk_bdev_get_by_name(uuid));
+
+	/* Check the same, but this time register the bdev with non-zero UUID */
+	rc = spdk_bdev_register(bdev);
+	CU_ASSERT_EQUAL(rc, 0);
+	CU_ASSERT_EQUAL(spdk_bdev_get_by_name(uuid), bdev);
+
+	/* Unregister the bdev */
+	spdk_bdev_unregister(bdev, NULL, NULL);
+	poll_threads();
+	CU_ASSERT_PTR_NULL(spdk_bdev_get_by_name(uuid));
+
+	/* Regiser the bdev using UUID as the name */
+	bdev->name = uuid;
+	rc = spdk_bdev_register(bdev);
+	CU_ASSERT_EQUAL(rc, 0);
+	CU_ASSERT_EQUAL(spdk_bdev_get_by_name(uuid), bdev);
+
+	/* Unregister the bdev */
+	spdk_bdev_unregister(bdev, NULL, NULL);
+	poll_threads();
+	CU_ASSERT_PTR_NULL(spdk_bdev_get_by_name(uuid));
+
+	/* Check that it's not possible to register two bdevs with the same UUIDs */
+	bdev->name = "bdev0";
+	second = allocate_bdev("bdev1");
+	spdk_uuid_copy(&bdev->uuid, &second->uuid);
+	rc = spdk_bdev_register(bdev);
+	CU_ASSERT_EQUAL(rc, -EEXIST);
+
+	/* Regenerate the UUID and re-check */
+	spdk_uuid_generate(&bdev->uuid);
+	rc = spdk_bdev_register(bdev);
+	CU_ASSERT_EQUAL(rc, 0);
+
+	/* And check that both bdevs can be retrieved through their UUIDs */
+	spdk_uuid_fmt_lower(uuid, sizeof(uuid), &bdev->uuid);
+	CU_ASSERT_EQUAL(spdk_bdev_get_by_name(uuid), bdev);
+	spdk_uuid_fmt_lower(uuid, sizeof(uuid), &second->uuid);
+	CU_ASSERT_EQUAL(spdk_bdev_get_by_name(uuid), second);
+
+	free_bdev(second);
+	free_bdev(bdev);
+	spdk_bdev_finish(bdev_fini_cb, NULL);
+	poll_threads();
+}
+
 int
 main(int argc, char **argv)
 {
@ -4998,6 +5065,7 @@ main(int argc, char **argv)
 	CU_ADD_TEST(suite, bdev_multi_allocation);
 	CU_ADD_TEST(suite, bdev_get_memory_domains);
 	CU_ADD_TEST(suite, bdev_writev_readv_ext);
+	CU_ADD_TEST(suite, bdev_register_uuid_alias);

 	allocate_cores(1);
 	allocate_threads(1);