Commit b51b8b4f authored by Tomasz Zawadzki's avatar Tomasz Zawadzki Committed by Daniel Verkamp
Browse files

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: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: If8cdbad18978319e57b6952dbf5a55d56785f108
Reviewed-on: https://review.gerrithub.io/380467


Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent e2e6fd37

include/spdk_internal/lvolstore.h

+1 −1
Original line number Diff line number Diff line
@@ -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;

lib/bdev/lvol/vbdev_lvol.c

+4 −2
Original line number Diff line number Diff line
@@ -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;

lib/lvol/lvol.c

+21 −13
Original line number Diff line number Diff line
@@ -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));

scripts/rpc.py

+2 −1
Original line number Diff line number Diff line
@@ -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')

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

+0 −2
Original line number Diff line number Diff line
@@ -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);