From b51b8b4f9c0a0a234e766c1ca162b8b07e9608d7 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Wed, 27 Sep 2017 13:18:50 +0200 Subject: [PATCH] lvol: fix incorrect assumption of 1MiB cluster_size Previously incorrectly it was assumed that cluster size was always to be 1MiB. For most evident example of this please see spdk_lvol_create() sz > free_clusters comparison. This is now fixed and lvol->sz was changed to lvol->cluster_num. It was done to increase readability - only dealing with number of clusters when creating or resizing lvol. Signed-off-by: Tomasz Zawadzki Change-Id: If8cdbad18978319e57b6952dbf5a55d56785f108 Reviewed-on: https://review.gerrithub.io/380467 Reviewed-by: Jim Harris Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp --- include/spdk_internal/lvolstore.h | 2 +- lib/bdev/lvol/vbdev_lvol.c | 6 ++-- lib/lvol/lvol.c | 34 ++++++++++++------- scripts/rpc.py | 3 +- .../lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c | 2 -- test/unit/lib/lvol/lvol.c/lvol_ut.c | 9 ++--- 6 files changed, 33 insertions(+), 23 deletions(-) diff --git a/include/spdk_internal/lvolstore.h b/include/spdk_internal/lvolstore.h index 7535c6e56..32158d8e2 100644 --- a/include/spdk_internal/lvolstore.h +++ b/include/spdk_internal/lvolstore.h @@ -75,7 +75,7 @@ struct spdk_lvol_store { struct spdk_lvol { struct spdk_lvol_store *lvol_store; struct spdk_blob *blob; - uint64_t sz; + uint64_t num_clusters; spdk_blob_id blob_id; char *name; bool close_only; diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 8522eea43..b03e728fb 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -405,6 +405,7 @@ _create_lvol_disk(struct spdk_lvol *lvol) { struct spdk_bdev *bdev; struct lvol_store_bdev *lvs_bdev; + uint64_t total_size; if (!lvol->name) { return NULL; @@ -426,8 +427,9 @@ _create_lvol_disk(struct spdk_lvol *lvol) bdev->product_name = "Logical Volume"; bdev->write_cache = 1; bdev->blocklen = spdk_bs_get_page_size(lvol->lvol_store->blobstore); - assert((lvol->sz % bdev->blocklen) == 0); - bdev->blockcnt = lvol->sz / bdev->blocklen; + total_size = lvol->num_clusters * spdk_bs_get_cluster_size(lvol->lvol_store->blobstore); + assert((total_size % bdev->blocklen) == 0); + bdev->blockcnt = total_size / bdev->blocklen; bdev->ctxt = lvol; bdev->fn_table = &vbdev_lvol_fn_table; diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index d3b2de679..257f4ba2b 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -41,6 +41,12 @@ SPDK_LOG_REGISTER_TRACE_FLAG("lvol", SPDK_TRACE_LVOL) +static inline size_t +divide_round_up(size_t num, size_t divisor) +{ + return (num + divisor - 1) / divisor; +} + static void _lvs_init_cb(void *cb_arg, struct spdk_blob_store *bs, int lvserrno) { @@ -208,8 +214,6 @@ _spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno) struct spdk_lvol_with_handle_req *req = cb_arg; spdk_blob_id blob_id = spdk_blob_get_id(blob); struct spdk_lvol *lvol = req->lvol; - uint64_t cluster_size = spdk_bs_get_cluster_size(lvol->lvol_store->blobstore); - uint64_t number_of_clusters = lvol->sz / cluster_size; char uuid[UUID_STRING_LEN]; if (lvolerrno < 0) { @@ -227,7 +231,7 @@ _spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno) goto invalid; } - lvolerrno = spdk_bs_md_resize_blob(blob, number_of_clusters); + lvolerrno = spdk_bs_md_resize_blob(blob, lvol->num_clusters); if (lvolerrno < 0) { spdk_bs_md_close_blob(&blob, _spdk_lvol_delete_blob_cb, lvol); goto invalid; @@ -271,17 +275,21 @@ spdk_lvol_create(struct spdk_lvol_store *lvs, 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; - uint64_t free_clusters; + uint64_t num_clusters, free_clusters; if (lvs == NULL) { SPDK_ERRLOG("lvol store does not exist\n"); return -ENODEV; } + bs = lvs->blobstore; - free_clusters = spdk_bs_free_cluster_count(lvs->blobstore); - if (sz > free_clusters) { - SPDK_ERRLOG("Not enough free clusters left on lvol store to add lvol with %zu clusters\n", sz); + num_clusters = divide_round_up(sz, spdk_bs_get_cluster_size(bs)); + free_clusters = spdk_bs_free_cluster_count(bs); + if (num_clusters > free_clusters) { + SPDK_ERRLOG("Not enough free clusters left (%zu) on lvol store to add lvol %zu clusters\n", + free_clusters, num_clusters); return -ENOMEM; } @@ -301,7 +309,7 @@ spdk_lvol_create(struct spdk_lvol_store *lvs, uint64_t sz, } lvol->lvol_store = lvs; - lvol->sz = sz * spdk_bs_get_cluster_size(lvs->blobstore); + lvol->num_clusters = num_clusters; lvol->close_only = false; req->lvol = lvol; @@ -327,14 +335,14 @@ spdk_lvol_resize(struct spdk_lvol *lvol, uint64_t sz, struct spdk_blob *blob = lvol->blob; struct spdk_lvol_store *lvs = lvol->lvol_store; struct spdk_lvol_req *req; - uint64_t cluster_size = spdk_bs_get_cluster_size(lvs->blobstore); uint64_t free_clusters = spdk_bs_free_cluster_count(lvs->blobstore); - uint64_t used_clusters = lvol->sz / cluster_size; + uint64_t used_clusters = lvol->num_clusters; + uint64_t new_clusters = divide_round_up(sz, spdk_bs_get_cluster_size(lvs->blobstore)); /* Check if size of lvol increasing */ - if (sz > used_clusters) { + if (new_clusters > used_clusters) { /* Check if there is enough clusters left to resize */ - if (sz - used_clusters > free_clusters) { + if (new_clusters - used_clusters > free_clusters) { SPDK_ERRLOG("Not enough free clusters left on lvol store to resize lvol to %zu clusters\n", sz); return -ENOMEM; } @@ -353,7 +361,7 @@ spdk_lvol_resize(struct spdk_lvol *lvol, uint64_t sz, goto invalid; } - lvol->sz = sz * cluster_size; + lvol->num_clusters = new_clusters; spdk_blob_md_set_xattr(blob, "length", &sz, sizeof(sz)); diff --git a/scripts/rpc.py b/scripts/rpc.py index a856e25c3..bdbb65b33 100755 --- a/scripts/rpc.py +++ b/scripts/rpc.py @@ -257,9 +257,10 @@ p.set_defaults(func=construct_lvol_store) def construct_lvol_bdev(args): + num_bytes = (args.size * 1024 * 1024) params = { 'lvol_store_uuid': args.lvol_store_uuid, - 'size': args.size, + 'size': num_bytes, } print_array(jsonrpc_call('construct_lvol_bdev', params)) p = subparsers.add_parser('construct_lvol_bdev', help='Add a bdev with an logical volume backend') 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 2e1f0cf0d..1ee2783cc 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 @@ -275,9 +275,7 @@ spdk_lvol_create(struct spdk_lvol_store *lvs, size_t sz, spdk_lvol_op_with_handl SPDK_CU_ASSERT_FATAL(lvol != NULL); - lvol->lvol_store = lvs; - lvol->sz = sz * 1024 * 1024; lvol->name = spdk_sprintf_alloc("%s", "UNIT_TEST_UUID"); SPDK_CU_ASSERT_FATAL(lvol->name != NULL); diff --git a/test/unit/lib/lvol/lvol.c/lvol_ut.c b/test/unit/lib/lvol/lvol.c/lvol_ut.c index b59d5214b..b8ae29cf8 100644 --- a/test/unit/lib/lvol/lvol.c/lvol_ut.c +++ b/test/unit/lib/lvol/lvol.c/lvol_ut.c @@ -42,7 +42,8 @@ #define DEV_BUFFER_SIZE (64 * 1024 * 1024) #define DEV_BUFFER_BLOCKLEN (4096) #define DEV_BUFFER_BLOCKCNT (DEV_BUFFER_SIZE / DEV_BUFFER_BLOCKLEN) -#define DEV_FREE_CLUSTERS 0xFFFF +#define BS_CLUSTER_SIZE (1024 * 1024) +#define BS_FREE_CLUSTERS (DEV_BUFFER_SIZE / BS_CLUSTER_SIZE) int g_lvserrno; int g_resize_rc; @@ -112,7 +113,7 @@ spdk_blob_get_id(struct spdk_blob *blob) uint64_t spdk_bs_get_cluster_size(struct spdk_blob_store *bs) { - return DEV_BUFFER_BLOCKLEN; + return BS_CLUSTER_SIZE; } void spdk_bs_md_close_blob(struct spdk_blob **b, @@ -144,7 +145,7 @@ spdk_bs_md_open_blob(struct spdk_blob_store *bs, spdk_blob_id blobid, uint64_t spdk_bs_free_cluster_count(struct spdk_blob_store *bs) { - return DEV_FREE_CLUSTERS; + return BS_FREE_CLUSTERS; } void @@ -275,7 +276,7 @@ lvol_create_fail(void) CU_ASSERT(rc != 0); CU_ASSERT(g_lvol == NULL); - rc = spdk_lvol_create(g_lvol_store, DEV_FREE_CLUSTERS + 1, lvol_op_with_handle_complete, NULL); + rc = spdk_lvol_create(g_lvol_store, DEV_BUFFER_SIZE + 1, lvol_op_with_handle_complete, NULL); CU_ASSERT(rc != 0); CU_ASSERT(g_lvol == NULL);