bdev: handling duplicated bdev name

Change the return type of spdk_bdev_register related
functions and try to handle the duplicated name
issue.

Change-Id: I23af11583cf2050579d1624508306a35394bffde
Signed-off-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/388178
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

include/spdk_internal/bdev.h

@@ -382,11 +382,11 @@ struct spdk_bdev_io {
 	/* No members may be added after driver_ctx! */
 };
 
-void spdk_bdev_register(struct spdk_bdev *bdev);
+int spdk_bdev_register(struct spdk_bdev *bdev);
 void spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg);
 void spdk_bdev_unregister_done(struct spdk_bdev *bdev, int bdeverrno);
-void spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs,
+int spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs,
-			 int base_bdev_count);
+			int base_bdev_count);
 void spdk_vbdev_unregister(struct spdk_bdev *vbdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg);
 
 void spdk_bdev_module_examine_done(struct spdk_bdev_module_if *module);

lib/bdev/aio/bdev_aio.c

@@ -373,6 +373,7 @@ create_aio_disk(const char *name, const char *filename, uint32_t block_size)
 	struct file_disk *fdisk;
 	uint32_t detected_block_size;
 	uint64_t disk_size;
+	int rc;
 
 	fdisk = calloc(sizeof(*fdisk), 1);
 	if (!fdisk) {
@@ -450,7 +451,11 @@ create_aio_disk(const char *name, const char *filename, uint32_t block_size)
 
 	spdk_io_device_register(&fdisk->fd, bdev_aio_create_cb, bdev_aio_destroy_cb,
 				sizeof(struct bdev_aio_io_channel));
-	spdk_bdev_register(&fdisk->disk);
+	rc = spdk_bdev_register(&fdisk->disk);
+	if (rc) {
+		spdk_io_device_unregister(&fdisk->fd, NULL);
+		goto error_return;
+	}
 
 	TAILQ_INSERT_TAIL(&g_aio_disk_head, fdisk, link);
 	return &fdisk->disk;

lib/bdev/bdev.c

@@ -1773,13 +1773,23 @@ spdk_bdev_io_get_thread(struct spdk_bdev_io *bdev_io)
 	return spdk_io_channel_get_thread(bdev_io->ch->channel);
 }
 
-static void
+static int
 _spdk_bdev_register(struct spdk_bdev *bdev)
 {
 	struct spdk_bdev_module_if *module;
 
 	assert(bdev->module != NULL);
 
+	if (!bdev->name) {
+		SPDK_ERRLOG("Bdev name is NULL\n");
+		return -EINVAL;
+	}
+
+	if (spdk_bdev_get_by_name(bdev->name)) {
+		SPDK_ERRLOG("Bdev name:%s already exists\n", bdev->name);
+		return -EEXIST;
+	}
+
 	bdev->status = SPDK_BDEV_STATUS_READY;
 
 	TAILQ_INIT(&bdev->open_descs);
@@ -1802,25 +1812,33 @@ _spdk_bdev_register(struct spdk_bdev *bdev)
 			module->examine(bdev);
 		}
 	}
+
+	return 0;
 }
 
-void
+int
 spdk_bdev_register(struct spdk_bdev *bdev)
 {
-	_spdk_bdev_register(bdev);
+	return _spdk_bdev_register(bdev);
 }
 
-void
+int
 spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, int base_bdev_count)
 {
-	int i;
+	int i, rc;
+
+	rc = _spdk_bdev_register(vbdev);
+	if (rc) {
+		return rc;
+	}
 
-	_spdk_bdev_register(vbdev);
 	for (i = 0; i < base_bdev_count; i++) {
 		assert(base_bdevs[i] != NULL);
 		TAILQ_INSERT_TAIL(&vbdev->base_bdevs, base_bdevs[i], base_bdev_link);
 		TAILQ_INSERT_TAIL(&base_bdevs[i]->vbdevs, vbdev, vbdev_link);
 	}
+
+	return 0;
 }
 
 void

lib/bdev/lvol/vbdev_lvol.c

@@ -634,6 +634,7 @@ _create_lvol_disk(struct spdk_lvol *lvol)
 	struct spdk_bdev *bdev;
 	struct lvol_store_bdev *lvs_bdev;
 	uint64_t total_size;
+	int rc;
 
 	if (!lvol->old_name) {
 		return NULL;
@@ -668,7 +669,12 @@ _create_lvol_disk(struct spdk_lvol *lvol)
 	bdev->fn_table = &vbdev_lvol_fn_table;
 	bdev->module = SPDK_GET_BDEV_MODULE(lvol);
 
-	spdk_vbdev_register(bdev, &lvs_bdev->bdev, 1);
+	rc = spdk_vbdev_register(bdev, &lvs_bdev->bdev, 1);
+	if (rc) {
+		free(bdev->name);
+		free(bdev);
+		return NULL;
+	}
 
 	return bdev;
 }

lib/bdev/malloc/bdev_malloc.c

@@ -370,6 +370,7 @@ static const struct spdk_bdev_fn_table malloc_fn_table = {
 struct spdk_bdev *create_malloc_disk(const char *name, uint64_t num_blocks, uint32_t block_size)
 {
 	struct malloc_disk	*mdisk;
+	int			rc;
 
 	if (block_size % 512 != 0) {
 		SPDK_ERRLOG("Block size %u is not a multiple of 512.\n", block_size);
@@ -421,7 +422,11 @@ struct spdk_bdev *create_malloc_disk(const char *name, uint64_t num_blocks, uint
 	mdisk->disk.fn_table = &malloc_fn_table;
 	mdisk->disk.module = SPDK_GET_BDEV_MODULE(malloc);
 
-	spdk_bdev_register(&mdisk->disk);
+	rc = spdk_bdev_register(&mdisk->disk);
+	if (rc) {
+		malloc_disk_free(mdisk);
+		return NULL;
+	}
 
 	mdisk->next = g_malloc_disk_head;
 	g_malloc_disk_head = mdisk;

lib/bdev/null/bdev_null.c

@@ -129,6 +129,7 @@ struct spdk_bdev *
 create_null_bdev(const char *name, uint64_t num_blocks, uint32_t block_size)
 {
 	struct null_bdev *bdev;
+	int rc;
 
 	if (block_size % 512 != 0) {
 		SPDK_ERRLOG("Block size %u is not a multiple of 512.\n", block_size);
@@ -161,7 +162,12 @@ create_null_bdev(const char *name, uint64_t num_blocks, uint32_t block_size)
 	bdev->bdev.fn_table = &null_fn_table;
 	bdev->bdev.module = SPDK_GET_BDEV_MODULE(null);
 
-	spdk_bdev_register(&bdev->bdev);
+	rc = spdk_bdev_register(&bdev->bdev);
+	if (rc) {
+		free(bdev->bdev.name);
+		spdk_dma_free(bdev);
+		return NULL;
+	}
 
 	TAILQ_INSERT_TAIL(&g_null_bdev_head, bdev, tailq);
 

lib/bdev/nvme/bdev_nvme.c

@@ -1131,7 +1131,7 @@ nvme_ctrlr_create_bdevs(struct nvme_ctrlr *nvme_ctrlr)
 	struct spdk_nvme_ctrlr	*ctrlr = nvme_ctrlr->ctrlr;
 	struct spdk_nvme_ns	*ns;
 	const struct spdk_nvme_ctrlr_data *cdata;
-	int			ns_id, num_ns;
+	int			ns_id, num_ns, rc;
 	int			bdev_created = 0;
 
 	num_ns = spdk_nvme_ctrlr_get_num_ns(ctrlr);
@@ -1176,7 +1176,12 @@ nvme_ctrlr_create_bdevs(struct nvme_ctrlr *nvme_ctrlr)
 		bdev->disk.ctxt = bdev;
 		bdev->disk.fn_table = &nvmelib_fn_table;
 		bdev->disk.module = SPDK_GET_BDEV_MODULE(nvme);
-		spdk_bdev_register(&bdev->disk);
+		rc = spdk_bdev_register(&bdev->disk);
+		if (rc) {
+			free(bdev->disk.name);
+			free(bdev);
+			break;
+		}
 
 		TAILQ_INSERT_TAIL(&g_nvme_bdevs, bdev, link);
 

lib/bdev/pmem/bdev_pmem.c

@@ -295,6 +295,7 @@ spdk_create_pmem_disk(const char *pmem_file, char *name, struct spdk_bdev **bdev
 	uint64_t num_blocks;
 	uint32_t block_size;
 	struct pmem_disk *pdisk;
+	int rc;
 
 	if (pmemblk_check(pmem_file, 0) != 1) {
 		SPDK_ERRLOG("Pool '%s' check failed: %s\n", pmem_file, pmemblk_errormsg());
@@ -353,7 +354,13 @@ spdk_create_pmem_disk(const char *pmem_file, char *name, struct spdk_bdev **bdev
 	pdisk->disk.fn_table = &pmem_fn_table;
 	pdisk->disk.module = SPDK_GET_BDEV_MODULE(pmem);
 
-	spdk_bdev_register(&pdisk->disk);
+	rc = spdk_bdev_register(&pdisk->disk);
+	if (rc) {
+		pmemblk_close(pdisk->pool);
+		free(pdisk->disk.name);
+		free(pdisk);
+		return rc;
+	}
 
 	TAILQ_INSERT_TAIL(&g_pmem_disks, pdisk, tailq);
 

lib/bdev/rbd/bdev_rbd.c

@@ -534,12 +534,18 @@ spdk_bdev_rbd_create(const char *pool_name, const char *rbd_name, uint32_t block
 	rbd->disk.module = SPDK_GET_BDEV_MODULE(rbd);
 
 	SPDK_NOTICELOG("Add %s rbd disk to lun\n", rbd->disk.name);
-	TAILQ_INSERT_TAIL(&g_rbds, rbd, tailq);
 
 	spdk_io_device_register(&rbd->info, bdev_rbd_create_cb,
 				bdev_rbd_destroy_cb,
 				sizeof(struct bdev_rbd_io_channel));
-	spdk_bdev_register(&rbd->disk);
+	ret = spdk_bdev_register(&rbd->disk);
+	if (ret) {
+		spdk_io_device_unregister(&rbd->info, NULL);
+		bdev_rbd_free(rbd);
+		return NULL;
+	}
+
+	TAILQ_INSERT_TAIL(&g_rbds, rbd, tailq);
 	return &rbd->disk;
 }
 

lib/bdev/virtio/bdev_virtio.c

@@ -634,7 +634,14 @@ scan_target_finish(struct virtio_scsi_scan_base *base)
 
 	while ((disk = TAILQ_FIRST(&base->found_disks))) {
 		TAILQ_REMOVE(&base->found_disks, disk, link);
-		spdk_bdev_register(&disk->bdev);
+		rc = spdk_bdev_register(&disk->bdev);
+		if (rc) {
+			spdk_io_device_unregister(base->vdev, NULL);
+			SPDK_ERRLOG("Failed to register bdev name=%s\n", disk->bdev.name);
+			spdk_ring_free(ctrlq_ring);
+			scan_target_abort(base, rc);
+			return;
+		}
 		bdevs[bdevs_cnt] = &disk->bdev;
 		bdevs_cnt++;
 	}

test/unit/lib/bdev/bdev.c/bdev_ut.c

@@ -337,6 +337,7 @@ part_test(void)
 	base = calloc(1, sizeof(*base));
 	SPDK_CU_ASSERT_FATAL(base != NULL);
 
+	bdev_base.name = "base";
 	bdev_base.fn_table = &base_fn_table;
 	bdev_base.module = SPDK_GET_BDEV_MODULE(bdev_ut);
 	spdk_bdev_register(&bdev_base);

test/unit/lib/bdev/pmem/bdev_pmem_ut.c

@@ -259,11 +259,13 @@ spdk_bdev_io_complete(struct spdk_bdev_io *bdev_io, enum spdk_bdev_io_status sta
 	bdev_io->status = status;
 }
 
-void
+int
 spdk_bdev_register(struct spdk_bdev *bdev)
 {
 	CU_ASSERT_PTR_NULL(g_bdev);
 	g_bdev = bdev;
+
+	return 0;
 }
 
 void

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

@@ -421,10 +421,11 @@ spdk_bdev_get_name(const struct spdk_bdev *bdev)
 	return "test";
 }
 
-void
+int
 spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, int base_bdev_count)
 {
 	g_registered_bdevs++;
+	return 0;
 }
 
 void