From bac4c02bc44062ec49211eb4e0c180508ce8c690 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 2 Jun 2017 10:25:43 -0700 Subject: [PATCH] bdev: change name and product_name to char * Dynamically allocate bdev names to remove the arbitrary 16-character name length limit. All of the existing product_names are constant strings, so those can just use string literals instead of a copy per bdev. Change-Id: I3280da67a4fcf2e4ec8ee8193362ca1b96a9c0cb Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/363601 Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker --- include/spdk_internal/bdev.h | 7 ++----- lib/bdev/aio/blockdev_aio.c | 8 ++++++-- lib/bdev/aio/blockdev_aio.h | 1 - lib/bdev/bdev.c | 2 +- lib/bdev/error/vbdev_error.c | 24 ++++++++++++++++++++---- lib/bdev/malloc/blockdev_malloc.c | 26 +++++++++++++++++++++----- lib/bdev/null/blockdev_null.c | 9 +++++++-- lib/bdev/nvme/blockdev_nvme.c | 11 +++++++---- lib/bdev/rbd/blockdev_rbd.c | 11 ++++++++--- lib/bdev/split/vbdev_split.c | 11 +++++++++-- 10 files changed, 81 insertions(+), 29 deletions(-) diff --git a/include/spdk_internal/bdev.h b/include/spdk_internal/bdev.h index 395b750e1..307da8504 100644 --- a/include/spdk_internal/bdev.h +++ b/include/spdk_internal/bdev.h @@ -79,9 +79,6 @@ * must be passed to the bdev database via spdk_bdev_register(). */ -#define SPDK_BDEV_MAX_NAME_LENGTH 16 -#define SPDK_BDEV_MAX_PRODUCT_NAME_LENGTH 50 - /** Block device module */ struct spdk_bdev_module_if { /** @@ -170,10 +167,10 @@ struct spdk_bdev { void *ctxt; /** Unique name for this block device. */ - char name[SPDK_BDEV_MAX_NAME_LENGTH]; + char *name; /** Unique product name for this kind of block device. */ - char product_name[SPDK_BDEV_MAX_PRODUCT_NAME_LENGTH]; + char *product_name; /** Size in bytes of a logical block for the backend */ uint32_t blocklen; diff --git a/lib/bdev/aio/blockdev_aio.c b/lib/bdev/aio/blockdev_aio.c index 6d3d57123..b5fc1690d 100644 --- a/lib/bdev/aio/blockdev_aio.c +++ b/lib/bdev/aio/blockdev_aio.c @@ -332,6 +332,7 @@ static void aio_free_disk(struct file_disk *fdisk) { if (fdisk == NULL) return; + free(fdisk->disk.name); free(fdisk); } @@ -355,8 +356,11 @@ create_aio_disk(const char *name, const char *fname) fdisk->size = spdk_fd_get_size(fdisk->fd); TAILQ_INIT(&fdisk->sync_completion_list); - snprintf(fdisk->disk.name, SPDK_BDEV_MAX_NAME_LENGTH, "%s", name); - snprintf(fdisk->disk.product_name, SPDK_BDEV_MAX_PRODUCT_NAME_LENGTH, "AIO disk"); + fdisk->disk.name = strdup(name); + if (!fdisk->disk.name) { + goto error_return; + } + fdisk->disk.product_name = "AIO disk"; fdisk->disk.need_aligned_buffer = 1; fdisk->disk.write_cache = 1; diff --git a/lib/bdev/aio/blockdev_aio.h b/lib/bdev/aio/blockdev_aio.h index e855ee028..b71346525 100644 --- a/lib/bdev/aio/blockdev_aio.h +++ b/lib/bdev/aio/blockdev_aio.h @@ -60,7 +60,6 @@ struct file_disk { struct spdk_bdev disk; const char *file; int fd; - char disk_name[SPDK_BDEV_MAX_NAME_LENGTH]; uint64_t size; /** diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 291059fd9..eb3c635c5 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -123,7 +123,7 @@ spdk_bdev_get_by_name(const char *bdev_name) struct spdk_bdev *bdev = spdk_bdev_first(); while (bdev != NULL) { - if (strncmp(bdev_name, bdev->name, sizeof(bdev->name)) == 0) { + if (strcmp(bdev_name, bdev->name) == 0) { return bdev; } bdev = spdk_bdev_next(bdev); diff --git a/lib/bdev/error/vbdev_error.c b/lib/bdev/error/vbdev_error.c index 5909e0552..41486031a 100644 --- a/lib/bdev/error/vbdev_error.c +++ b/lib/bdev/error/vbdev_error.c @@ -40,6 +40,7 @@ #include "spdk/conf.h" #include "spdk/endian.h" #include "spdk/nvme_spec.h" +#include "spdk/string.h" #include "spdk_internal/bdev.h" #include "spdk_internal/log.h" @@ -120,6 +121,17 @@ vbdev_error_submit_request(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev pthread_mutex_unlock(&g_vbdev_error_mutex); } +static void +vbdev_error_disk_free(struct vbdev_error_disk *disk) +{ + if (!disk) { + return; + } + + free(disk->disk.name); + free(disk); +} + static void vbdev_error_free(struct vbdev_error_disk *error_disk) { @@ -130,7 +142,7 @@ vbdev_error_free(struct vbdev_error_disk *error_disk) TAILQ_REMOVE(&g_vbdev_error_disks, error_disk, tailq); spdk_bdev_unclaim(error_disk->base_bdev); - free(error_disk); + vbdev_error_disk_free(error_disk); } static int @@ -203,8 +215,12 @@ spdk_vbdev_error_create(struct spdk_bdev *base_bdev) disk->base_bdev = base_bdev; memcpy(&disk->disk, base_bdev, sizeof(*base_bdev)); - snprintf(disk->disk.name, sizeof(disk->disk.name), "EE_%s", base_bdev->name); - snprintf(disk->disk.product_name, sizeof(disk->disk.product_name), "Error Injection Disk"); + disk->disk.name = spdk_sprintf_alloc("EE_%s", base_bdev->name); + if (!disk->disk.name) { + rc = -ENOMEM; + goto cleanup; + } + disk->disk.product_name = "Error Injection Disk"; disk->disk.ctxt = disk; disk->disk.fn_table = &vbdev_error_fn_table; @@ -215,7 +231,7 @@ spdk_vbdev_error_create(struct spdk_bdev *base_bdev) rc = 0; return rc; cleanup: - free(disk); + vbdev_error_disk_free(disk); return rc; } diff --git a/lib/bdev/malloc/blockdev_malloc.c b/lib/bdev/malloc/blockdev_malloc.c index e8d12220b..c520b3d56 100644 --- a/lib/bdev/malloc/blockdev_malloc.c +++ b/lib/bdev/malloc/blockdev_malloc.c @@ -41,6 +41,7 @@ #include "spdk/env.h" #include "spdk/copy_engine.h" #include "spdk/io_channel.h" +#include "spdk/string.h" #include "spdk_internal/bdev.h" #include "spdk_internal/log.h" @@ -124,13 +125,24 @@ blockdev_malloc_delete_from_list(struct malloc_disk *malloc_disk) } } +static void +malloc_disk_free(struct malloc_disk *malloc_disk) +{ + if (!malloc_disk) { + return; + } + + free(malloc_disk->disk.name); + spdk_dma_free(malloc_disk->malloc_buf); + spdk_dma_free(malloc_disk); +} + static int blockdev_malloc_destruct(void *ctx) { struct malloc_disk *malloc_disk = ctx; blockdev_malloc_delete_from_list(malloc_disk); - spdk_dma_free(malloc_disk->malloc_buf); - spdk_dma_free(malloc_disk); + malloc_disk_free(malloc_disk); return 0; } @@ -391,12 +403,16 @@ struct spdk_bdev *create_malloc_disk(uint64_t num_blocks, uint32_t block_size) mdisk->malloc_buf = spdk_dma_zmalloc(num_blocks * block_size, 2 * 1024 * 1024, NULL); if (!mdisk->malloc_buf) { SPDK_ERRLOG("spdk_dma_zmalloc failed\n"); - spdk_dma_free(mdisk); + malloc_disk_free(mdisk); return NULL; } - snprintf(mdisk->disk.name, SPDK_BDEV_MAX_NAME_LENGTH, "Malloc%d", malloc_disk_count); - snprintf(mdisk->disk.product_name, SPDK_BDEV_MAX_PRODUCT_NAME_LENGTH, "Malloc disk"); + mdisk->disk.name = spdk_sprintf_alloc("Malloc%d", malloc_disk_count); + if (!mdisk->disk.name) { + malloc_disk_free(mdisk); + return NULL; + } + mdisk->disk.product_name = "Malloc disk"; malloc_disk_count++; mdisk->disk.write_cache = 1; diff --git a/lib/bdev/null/blockdev_null.c b/lib/bdev/null/blockdev_null.c index fbd5e3b8f..4dc55ddf4 100644 --- a/lib/bdev/null/blockdev_null.c +++ b/lib/bdev/null/blockdev_null.c @@ -63,6 +63,7 @@ blockdev_null_destruct(void *ctx) struct null_bdev *bdev = ctx; TAILQ_REMOVE(&g_null_bdev_head, bdev, tailq); + free(bdev->bdev.name); spdk_dma_free(bdev); return 0; @@ -141,8 +142,12 @@ create_null_bdev(const char *name, uint64_t num_blocks, uint32_t block_size) return NULL; } - snprintf(bdev->bdev.name, SPDK_BDEV_MAX_NAME_LENGTH, "%s", name); - snprintf(bdev->bdev.product_name, SPDK_BDEV_MAX_PRODUCT_NAME_LENGTH, "Null disk"); + bdev->bdev.name = strdup(name); + if (!bdev->bdev.name) { + spdk_dma_free(bdev); + return NULL; + } + bdev->bdev.product_name = "Null disk"; bdev->bdev.write_cache = 0; bdev->bdev.blocklen = block_size; diff --git a/lib/bdev/nvme/blockdev_nvme.c b/lib/bdev/nvme/blockdev_nvme.c index 739955ea1..ccf37f6f8 100644 --- a/lib/bdev/nvme/blockdev_nvme.c +++ b/lib/bdev/nvme/blockdev_nvme.c @@ -204,6 +204,7 @@ bdev_nvme_destruct(void *ctx) nvme_ctrlr->ref--; TAILQ_REMOVE(&g_nvme_bdevs, nvme_disk, link); + free(nvme_disk->disk.name); free(nvme_disk); if (nvme_ctrlr->ref == 0) { @@ -942,10 +943,12 @@ nvme_ctrlr_create_bdevs(struct nvme_ctrlr *nvme_ctrlr) bdev->ns = ns; nvme_ctrlr->ref++; - snprintf(bdev->disk.name, SPDK_BDEV_MAX_NAME_LENGTH, - "%sn%d", nvme_ctrlr->name, spdk_nvme_ns_get_id(ns)); - snprintf(bdev->disk.product_name, SPDK_BDEV_MAX_PRODUCT_NAME_LENGTH, - "NVMe disk"); + bdev->disk.name = spdk_sprintf_alloc("%sn%d", nvme_ctrlr->name, spdk_nvme_ns_get_id(ns)); + if (!bdev->disk.name) { + free(bdev); + return; + } + bdev->disk.product_name = "NVMe disk"; if (cdata->oncs.dsm) { /* diff --git a/lib/bdev/rbd/blockdev_rbd.c b/lib/bdev/rbd/blockdev_rbd.c index 2b357092e..a89e71127 100644 --- a/lib/bdev/rbd/blockdev_rbd.c +++ b/lib/bdev/rbd/blockdev_rbd.c @@ -44,6 +44,7 @@ #include "spdk/log.h" #include "spdk/bdev.h" #include "spdk/io_channel.h" +#include "spdk/string.h" #include "spdk_internal/bdev.h" @@ -80,6 +81,7 @@ blockdev_rbd_free(struct blockdev_rbd *rbd) return; } + free(rbd->disk.name); free(rbd->rbd_name); free(rbd->pool_name); free(rbd); @@ -522,9 +524,12 @@ spdk_bdev_rbd_create(const char *pool_name, const char *rbd_name, uint32_t block return NULL; } - snprintf(rbd->disk.name, SPDK_BDEV_MAX_NAME_LENGTH, "Ceph%d", - blockdev_rbd_count); - snprintf(rbd->disk.product_name, SPDK_BDEV_MAX_PRODUCT_NAME_LENGTH, "Ceph Rbd Disk"); + rbd->disk.name = spdk_sprintf_alloc("Ceph%d", blockdev_rbd_count); + if (!rbd->disk.name) { + blockdev_rbd_free(rbd); + return NULL; + } + rbd->disk.product_name = "Ceph Rbd Disk"; blockdev_rbd_count++; rbd->disk.write_cache = 0; diff --git a/lib/bdev/split/vbdev_split.c b/lib/bdev/split/vbdev_split.c index d0d9d6009..873bd8175 100644 --- a/lib/bdev/split/vbdev_split.c +++ b/lib/bdev/split/vbdev_split.c @@ -41,6 +41,7 @@ #include "spdk/rpc.h" #include "spdk/conf.h" #include "spdk/endian.h" +#include "spdk/string.h" #include "spdk_internal/bdev.h" #include "spdk_internal/log.h" @@ -167,6 +168,7 @@ vbdev_split_free(struct split_disk *split_disk) split_base = split_disk->split_base; TAILQ_REMOVE(&g_split_disks, split_disk, tailq); + free(split_disk->disk.name); free(split_disk); if (split_base) { @@ -295,8 +297,13 @@ vbdev_split_create(struct spdk_bdev *base_bdev, uint64_t split_count, uint64_t s d->disk.max_unmap_bdesc_count = base_bdev->max_unmap_bdesc_count; /* Append partition number to the base bdev's name, e.g. Malloc0 -> Malloc0p0 */ - snprintf(d->disk.name, sizeof(d->disk.name), "%sp%" PRIu64, spdk_bdev_get_name(base_bdev), i); - snprintf(d->disk.product_name, sizeof(d->disk.product_name), "Split Disk"); + d->disk.name = spdk_sprintf_alloc("%sp%" PRIu64, spdk_bdev_get_name(base_bdev), i); + if (!d->disk.name) { + free(d); + rc = -ENOMEM; + goto cleanup; + } + d->disk.product_name = "Split Disk"; d->base_bdev = base_bdev; d->offset_bytes = offset_bytes; d->offset_blocks = offset_blocks;