From 6802fe99f4e9e7aea987b38c529648564a5a81c5 Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Mon, 20 Nov 2017 17:31:39 +0800 Subject: [PATCH] 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 Reviewed-on: https://review.gerrithub.io/388178 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- include/spdk_internal/bdev.h | 6 ++-- lib/bdev/aio/bdev_aio.c | 7 ++++- lib/bdev/bdev.c | 30 +++++++++++++++---- lib/bdev/lvol/vbdev_lvol.c | 8 ++++- lib/bdev/malloc/bdev_malloc.c | 7 ++++- lib/bdev/null/bdev_null.c | 8 ++++- lib/bdev/nvme/bdev_nvme.c | 9 ++++-- lib/bdev/pmem/bdev_pmem.c | 9 +++++- lib/bdev/rbd/bdev_rbd.c | 10 +++++-- lib/bdev/virtio/bdev_virtio.c | 9 +++++- test/unit/lib/bdev/bdev.c/bdev_ut.c | 1 + test/unit/lib/bdev/pmem/bdev_pmem_ut.c | 4 ++- .../lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c | 3 +- 13 files changed, 90 insertions(+), 21 deletions(-) diff --git a/include/spdk_internal/bdev.h b/include/spdk_internal/bdev.h index 908f84469..514683292 100644 --- a/include/spdk_internal/bdev.h +++ b/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 base_bdev_count); +int spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, + 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); diff --git a/lib/bdev/aio/bdev_aio.c b/lib/bdev/aio/bdev_aio.c index 35ee2a4df..509ed8dd8 100644 --- a/lib/bdev/aio/bdev_aio.c +++ b/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; diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 675ea2892..ebf7fc60d 100644 --- a/lib/bdev/bdev.c +++ b/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 diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index f8aa7f2e5..1392e7b4d 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/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; } diff --git a/lib/bdev/malloc/bdev_malloc.c b/lib/bdev/malloc/bdev_malloc.c index e9a95c969..b102fa10a 100644 --- a/lib/bdev/malloc/bdev_malloc.c +++ b/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; diff --git a/lib/bdev/null/bdev_null.c b/lib/bdev/null/bdev_null.c index 5aecfba84..a9d45f3ad 100644 --- a/lib/bdev/null/bdev_null.c +++ b/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); diff --git a/lib/bdev/nvme/bdev_nvme.c b/lib/bdev/nvme/bdev_nvme.c index 27409d37d..14a9ca8c0 100644 --- a/lib/bdev/nvme/bdev_nvme.c +++ b/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); diff --git a/lib/bdev/pmem/bdev_pmem.c b/lib/bdev/pmem/bdev_pmem.c index de212eb00..2ceb7ee79 100644 --- a/lib/bdev/pmem/bdev_pmem.c +++ b/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); diff --git a/lib/bdev/rbd/bdev_rbd.c b/lib/bdev/rbd/bdev_rbd.c index e9a0b4428..6bc4f1143 100644 --- a/lib/bdev/rbd/bdev_rbd.c +++ b/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; } diff --git a/lib/bdev/virtio/bdev_virtio.c b/lib/bdev/virtio/bdev_virtio.c index 2cf2d7324..08596ead4 100644 --- a/lib/bdev/virtio/bdev_virtio.c +++ b/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++; } diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index 3fabfce3f..9416ae076 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/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); diff --git a/test/unit/lib/bdev/pmem/bdev_pmem_ut.c b/test/unit/lib/bdev/pmem/bdev_pmem_ut.c index 80e315cce..4b8173db5 100644 --- a/test/unit/lib/bdev/pmem/bdev_pmem_ut.c +++ b/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 diff --git a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c b/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c index 6153bbd16..55634dce9 100644 --- a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c +++ b/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