Commit 8ed35e41 authored by Tomasz Zawadzki's avatar Tomasz Zawadzki Committed by Ben Walker
Browse files

bdev/lvol: fix examine error path 



Added proper paths for examine:
- if lvs loaded, do spdk_lvs_unload
- if lvol created, call spdk_lvol_close instead of spdk_blob_close

Change-Id: I77a2905417d1c53f36b96da3fbea2679d9a2f56d
Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-on: https://review.gerrithub.io/419558


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarMaciej Szwed <maciej.szwed@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
parent 4c0f590b

lib/bdev/lvol/vbdev_lvol.c

+11 −6
Original line number Diff line number Diff line
@@ -1118,6 +1118,12 @@ vbdev_lvs_get_ctx_size(void) 
	return sizeof(struct lvol_task);

}



static void

_vbdev_lvs_examine_failed(void *cb_arg, int lvserrno)

{

	spdk_bdev_module_examine_done(&g_lvol_if);

}



static void

_vbdev_lvs_examine_finish(void *cb_arg, struct spdk_lvol *lvol, int lvolerrno)

{
@@ -1138,10 +1144,8 @@ _vbdev_lvs_examine_finish(void *cb_arg, struct spdk_lvol *lvol, int lvolerrno) 
		SPDK_ERRLOG("Cannot create bdev for lvol %s\n", lvol->unique_id);

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

		lvs->lvol_count--;

		spdk_blob_close(lvol->blob, _vbdev_lvol_close_cb, lvs);

		spdk_lvol_close(lvol, _vbdev_lvol_close_cb, lvs);

		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Opening lvol %s failed\n", lvol->unique_id);

		free(lvol->unique_id);

		free(lvol);

		return;

	}
@@ -1168,10 +1172,12 @@ _vbdev_lvs_examine_cb(void *arg, struct spdk_lvol_store *lvol_store, int lvserrn 
		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL,

			     "Name for lvolstore on device %s conflicts with name for already loaded lvs\n",

			     req->base_bdev->name);

		/* On error blobstore destroys bs_dev itself */

		spdk_bdev_module_examine_done(&g_lvol_if);

		goto end;

	} else if (lvserrno != 0) {

		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Lvol store not found on %s\n", req->base_bdev->name);

		/* On error blobstore destroys bs_dev itself */

		spdk_bdev_module_examine_done(&g_lvol_if);

		goto end;

	}
@@ -1179,15 +1185,14 @@ _vbdev_lvs_examine_cb(void *arg, struct spdk_lvol_store *lvol_store, int lvserrn 
	lvserrno = spdk_bs_bdev_claim(lvol_store->bs_dev, &g_lvol_if);

	if (lvserrno != 0) {

		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Lvol store base bdev already claimed by another bdev\n");

		lvol_store->bs_dev->destroy(lvol_store->bs_dev);

		spdk_bdev_module_examine_done(&g_lvol_if);

		spdk_lvs_unload(lvol_store, _vbdev_lvs_examine_failed, NULL);

		goto end;

	}



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

	if (!lvs_bdev) {

		SPDK_ERRLOG("Cannot alloc memory for lvs_bdev\n");

		spdk_bdev_module_examine_done(&g_lvol_if);

		spdk_lvs_unload(lvol_store, _vbdev_lvs_examine_failed, NULL);

		goto end;

	}

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

+26 −19
Original line number Diff line number Diff line
@@ -152,11 +152,6 @@ spdk_lvol_open(struct spdk_lvol *lvol, spdk_lvol_op_with_handle_complete cb_fn, 
	cb_fn(cb_arg, lvol, g_lvolerrno);

}



void

spdk_blob_close(struct spdk_blob *b, spdk_blob_op_complete cb_fn, void *cb_arg)

{

}



uint64_t

spdk_blob_get_num_clusters(struct spdk_blob *b)

{
@@ -211,8 +206,18 @@ spdk_lvs_load(struct spdk_bs_dev *dev, 
{

	struct spdk_lvol_store *lvs;

	int i;

	int lvserrno = g_lvserrno;



	if (lvserrno != 0) {

		/* On error blobstore destroys bs_dev itself,

		 * by puttin back io channels.

		 * This operation is asynchronous, and completed

		 * after calling the callback for lvol. */

		cb_fn(cb_arg, g_lvol_store, lvserrno);

		dev->destroy(dev);

		return;

	}



	if (g_lvserrno == 0) {

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

	SPDK_CU_ASSERT_FATAL(lvs != NULL);

	TAILQ_INIT(&lvs->lvols);
@@ -221,9 +226,8 @@ spdk_lvs_load(struct spdk_bs_dev *dev, 
	for (i = 0; i < g_num_lvols; i++) {

		_lvol_create(lvs);

	}

	}



	cb_fn(cb_arg, g_lvol_store, g_lvserrno);

	cb_fn(cb_arg, g_lvol_store, lvserrno);

}



int
@@ -598,6 +602,7 @@ spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, int 
void

spdk_bdev_module_examine_done(struct spdk_bdev_module *module)

{

	SPDK_CU_ASSERT_FATAL(g_examine_done != true);

	g_examine_done = true;

}
@@ -959,13 +964,14 @@ ut_lvol_examine(void) 
	g_lvserrno = -1;

	lvol_already_opened = false;

	vbdev_lvs_examine(&g_bdev);

	CU_ASSERT(g_bs_dev != NULL);

	CU_ASSERT(g_bs_dev == NULL);

	CU_ASSERT(g_lvol_store == NULL);

	CU_ASSERT(g_examine_done == true);

	CU_ASSERT(TAILQ_EMPTY(&g_spdk_lvol_pairs));

	free(g_bs_dev);



	/* Examine unsuccesfully - fail on lvol load */

	/* Examine succesfully

	 * - one lvol fails to load

	 * - lvs is loaded with no lvols present */

	g_bs_dev = NULL;

	g_lvserrno = 0;

	g_lvolerrno = -1;
@@ -981,7 +987,8 @@ ut_lvol_examine(void) 
	CU_ASSERT(!TAILQ_EMPTY(&g_spdk_lvol_pairs));

	CU_ASSERT(TAILQ_EMPTY(&g_lvol_store->lvols));

	vbdev_lvs_destruct(g_lvol_store, lvol_store_op_complete, NULL);

	free(g_bs_dev);

	CU_ASSERT(g_lvserrno == 0);

	CU_ASSERT(g_lvol_store == NULL);



	/* Examine succesfully */

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