Commit 63fce559 authored by Maciej Szwed's avatar Maciej Szwed Committed by Jim Harris
Browse files

lvol: add reference counter for open lvols 



Signed-off-by: default avatarMaciej Szwed <maciej.szwed@intel.com>
Change-Id: I3a86f28056e67b3c237441fb1048ca6ccd081ae7

Reviewed-on: https://review.gerrithub.io/383252


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

include/spdk_internal/lvolstore.h

+1 −0
Original line number Diff line number Diff line
@@ -87,6 +87,7 @@ struct spdk_lvol { 
	char				*name;

	bool				close_only;

	struct spdk_bdev		*bdev;

	int				ref_count;

	TAILQ_ENTRY(spdk_lvol) link;

};

lib/bdev/lvol/vbdev_lvol.c

+18 −2
Original line number Diff line number Diff line
@@ -166,6 +166,7 @@ vbdev_lvs_destruct(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, 
	struct spdk_lvs_req *req;

	struct lvol_store_bdev *lvs_bdev;

	struct spdk_lvol *lvol, *tmp;

	bool all_lvols_closed = true;



	lvs_bdev = vbdev_get_lvs_bdev_by_lvs(lvs);

	TAILQ_REMOVE(&g_spdk_lvol_pairs, lvs_bdev, lvol_stores);
@@ -181,7 +182,13 @@ vbdev_lvs_destruct(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, 
	req->cb_fn = cb_fn;

	req->cb_arg = cb_arg;



	if (TAILQ_EMPTY(&lvs->lvols)) {

	TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) {

		if (lvol->ref_count != 0) {

			all_lvols_closed = false;

		}

	}



	if (all_lvols_closed == true) {

		spdk_lvs_unload(lvs, _vbdev_lvs_destruct_cb, req);

	} else {

		lvs->destruct_req = calloc(1, sizeof(*lvs->destruct_req));
@@ -288,6 +295,15 @@ _vbdev_lvol_destroy_cb(void *cb_arg, int lvserrno) 
	SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol destroyed\n");

}



static void

_vbdev_lvol_destroy_after_close_cb(void *cb_arg, int lvserrno)

{

	struct spdk_lvol *lvol = cb_arg;



	SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol %s closed, begin destroying\n", lvol->name);

	spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, NULL);

}



static int

vbdev_lvol_destruct(void *ctx)

{
@@ -298,7 +314,7 @@ vbdev_lvol_destruct(void *ctx) 
	if (lvol->close_only) {

		spdk_lvol_close(lvol, _vbdev_lvol_close_cb, NULL);

	} else {

		spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, NULL);

		spdk_lvol_close(lvol, _vbdev_lvol_destroy_after_close_cb, lvol);

	}



	return 0;

lib/lvol/lvol.c

+51 −21
Original line number Diff line number Diff line
@@ -249,16 +249,25 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, 
		void *cb_arg)

{

	struct spdk_lvs_req *lvs_req;

	struct spdk_lvol *lvol, *tmp;



	if (lvs == NULL) {

		SPDK_ERRLOG("Lvol store is NULL\n");

		return -ENODEV;

	}



	if (!TAILQ_EMPTY(&lvs->lvols)) {

	TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) {

		if (lvol->ref_count != 0) {

			SPDK_ERRLOG("Lvols still open on lvol store\n");

			return -EBUSY;

		}

	}



	TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) {

		TAILQ_REMOVE(&lvs->lvols, lvol, link);

		free(lvol->name);

		free(lvol);

	}



	lvs_req = calloc(1, sizeof(*lvs_req));

	if (!lvs_req) {
@@ -276,16 +285,6 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, 
	return 0;

}



static void

_spdk_lvol_return_to_caller(void *cb_arg, int lvolerrno)

{

	struct spdk_lvol_with_handle_req *req = cb_arg;



	assert(req->cb_fn != NULL);

	req->cb_fn(req->cb_arg, req->lvol, lvolerrno);

	free(req);

}



static void

_spdk_lvs_destruct_cb(void *cb_arg, int lvserrno)

{
@@ -303,6 +302,8 @@ _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno) 
{

	struct spdk_lvol_req *req = cb_arg;

	struct spdk_lvol *lvol = req->lvol;

	struct spdk_lvol *iter_lvol, *tmp;

	bool all_lvols_closed = true;



	if (lvolerrno < 0) {

		SPDK_ERRLOG("Could not close blob on lvol\n");
@@ -311,14 +312,19 @@ _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno) 
		goto end;

	}



	if (lvol->lvol_store->destruct_req && TAILQ_EMPTY(&lvol->lvol_store->lvols)) {

		spdk_lvs_unload(lvol->lvol_store, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req);

	lvol->ref_count--;



	TAILQ_FOREACH_SAFE(iter_lvol, &lvol->lvol_store->lvols, link, tmp) {

		if (iter_lvol->ref_count != 0) {

			all_lvols_closed = false;

		}

	}



	SPDK_INFOLOG(SPDK_TRACE_LVOL, "Lvol %s closed\n", lvol->name) ;



	free(lvol->name);

	free(lvol);

	if (lvol->lvol_store->destruct_req && all_lvols_closed == true) {

		spdk_lvs_unload(lvol->lvol_store, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req);

	}



end:

	req->cb_fn(req->cb_arg, lvolerrno);
@@ -377,6 +383,23 @@ _spdk_lvol_destroy_cb(void *cb_arg, int lvolerrno) 
}





static void

_spdk_lvol_sync_cb(void *cb_arg, int lvolerrno)

{

	struct spdk_lvol_with_handle_req *req = cb_arg;

	struct spdk_lvol *lvol = req->lvol;



	if (lvolerrno != 0) {

		spdk_bs_md_close_blob(&lvol->blob, _spdk_lvol_delete_blob_cb, lvol);

	} else {

		lvol->ref_count++;

	}



	assert(req->cb_fn != NULL);

	req->cb_fn(req->cb_arg, req->lvol, lvolerrno);

	free(req);

}



static void

_spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno)

{
@@ -399,18 +422,19 @@ _spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno) 
		spdk_bs_md_close_blob(&blob, _spdk_lvol_delete_blob_cb, lvol);

		SPDK_ERRLOG("Cannot alloc memory for lvol name\n");

		lvolerrno = -ENOMEM;

		free(lvol);

		goto invalid;

	}



	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);

		spdk_bs_md_close_blob(&blob, _spdk_lvol_destroy_cb, lvol);

		goto invalid;

	}



	TAILQ_INSERT_TAIL(&lvol->lvol_store->lvols, lvol, link);



	spdk_bs_md_sync_blob(blob, _spdk_lvol_return_to_caller, req);

	spdk_bs_md_sync_blob(blob, _spdk_lvol_sync_cb, req);



	return;
@@ -549,6 +573,7 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ 
{

	struct spdk_lvol_with_handle_req *req;

	struct spdk_lvol_req *lvol_req;

	struct spdk_blob_store *bs = lvol->lvol_store->blobstore;



	assert(cb_fn != NULL);
@@ -558,6 +583,12 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ 
		return;

	}



	if (lvol->ref_count != 0) {

		SPDK_ERRLOG("Cannot destroy lvol %s because it is still open\n", lvol->name);

		cb_fn(cb_arg, -EBUSY);

		return;

	}



	req = calloc(1, sizeof(*req));

	if (!req) {

		SPDK_ERRLOG("Cannot alloc memory for lvol request pointer\n");
@@ -579,7 +610,7 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ 
	req->lvol = lvol;



	TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link);

	spdk_bs_md_close_blob(&(lvol->blob), _spdk_lvol_destroy_cb, req);

	spdk_bs_md_delete_blob(bs, lvol->blob_id, _spdk_lvol_delete_blob_cb, req);

}



void
@@ -606,7 +637,6 @@ spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ar 
	req->cb_arg = cb_arg;

	req->lvol = lvol;



	TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link);

	spdk_bs_md_close_blob(&(lvol->blob), _spdk_lvol_close_blob_cb, req);

}

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

+20 −16
Original line number Diff line number Diff line
@@ -175,37 +175,39 @@ spdk_bdev_get_by_name(const char *bdev_name) 
	return NULL;

}



static void

close_cb(void *cb_arg, int lvolerrno)

{

}



void

spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg)

{

	struct spdk_lvs_req *destruct_req;

	struct spdk_lvol *iter_lvol, *tmp;

	bool all_lvols_closed = true;



	SPDK_CU_ASSERT_FATAL(lvol == g_lvol);



	TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link);

	lvol->ref_count--;



	TAILQ_FOREACH_SAFE(iter_lvol, &lvol->lvol_store->lvols, link, tmp) {

		if (iter_lvol->ref_count != 0) {

			all_lvols_closed = false;

		}

	}



	destruct_req = lvol->lvol_store->destruct_req;

	if (destruct_req && TAILQ_EMPTY(&lvol->lvol_store->lvols)) {

	if (destruct_req && all_lvols_closed == true) {

		spdk_lvs_unload(lvol->lvol_store, destruct_req->cb_fn, destruct_req->cb_arg);

		free(destruct_req);

	}



	free(lvol->name);

	free(lvol);

	g_lvol = NULL;

	cb_fn(NULL, 0);

	cb_fn(cb_arg, 0);

}



void

spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg)

{

	/* Lvol destroy and close are effectively the same from UT perspective */

	spdk_lvol_close(lvol, close_cb, NULL);

	free(lvol->name);

	free(lvol);

	g_lvol = NULL;



	cb_fn(NULL, 0);

}
@@ -311,6 +313,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->ref_count++;

	lvol->name = spdk_sprintf_alloc("%s", "UNIT_TEST_UUID");

	SPDK_CU_ASSERT_FATAL(lvol->name != NULL);
@@ -369,18 +372,19 @@ ut_lvs_destroy(void) 


	uuid_generate_time(lvs->uuid);



	/* Suuccessfully create lvol, which should be destroyed with lvs later */

	/* Suuccessfully create lvol, which should be unloaded with lvs later */

	g_lvolerrno = -1;

	rc = vbdev_lvol_create(lvs->uuid, sz, vbdev_lvol_create_complete, NULL);

	CU_ASSERT(rc == 0);

	CU_ASSERT(g_lvolerrno == 0);

	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);



	/* Destroy lvol store */

	/* Unload lvol store */

	vbdev_lvs_destruct(lvs, lvol_store_op_complete, NULL);

	CU_ASSERT(g_lvserrno == 0);

	CU_ASSERT(g_lvol_store == NULL);

	CU_ASSERT(g_lvol == NULL);

	free(g_lvol->name);

	free(g_lvol);

}



static void

test/unit/lib/lvol/lvol.c/lvol_ut.c

+6 −0
Original line number Diff line number Diff line
@@ -330,6 +330,8 @@ lvol_create_destroy_success(void) 
	CU_ASSERT(g_lvserrno == 0);

	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);



	spdk_lvol_close(g_lvol, close_cb, NULL);

	CU_ASSERT(g_lvserrno == 0);

	spdk_lvol_destroy(g_lvol, destroy_cb, NULL);

	CU_ASSERT(g_lvserrno == 0);
@@ -405,6 +407,8 @@ lvol_destroy_fail(void) 
	CU_ASSERT(g_lvserrno == 0);

	SPDK_CU_ASSERT_FATAL(g_lvol != NULL);



	spdk_lvol_close(g_lvol, close_cb, NULL);

	CU_ASSERT(g_lvserrno == 0);

	spdk_lvol_destroy(g_lvol, destroy_cb, NULL);

	CU_ASSERT(g_lvserrno == 0);
@@ -541,6 +545,8 @@ lvol_resize(void) 
	CU_ASSERT(rc != 0);

	CU_ASSERT(g_lvserrno != 0);



	spdk_lvol_close(g_lvol, close_cb, NULL);

	CU_ASSERT(g_lvserrno == 0);

	spdk_lvol_destroy(g_lvol, destroy_cb, NULL);

	CU_ASSERT(g_lvserrno == 0);