Commit 7920c642 authored by Tomasz Zawadzki's avatar Tomasz Zawadzki
Browse files

lib/lvol: fix action_in_progress 



action_in_progress right now determines if lvol is
in process of being closed or deleted.

It should be set only once blobstore functions are called,
and unset if they do encounter an error afterwards.
Failure to close can only occur during close, then the lvol
remains. For destroy it is freed anyway.

This patch moves setting it to true bit later on close/destroy,
and joins success/error paths by setting in to false in both.

While here remove lvol_free() that should not be freed
in the close path, if blob close failed.
This allows to add relevant UT.

Part of work on #3006.

Change-Id: I9dc8b8defb6090b9e40b3520fbdccbd071622ba6
Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/18684


Community-CI: Mellanox Build Bot
Reviewed-by: default avatarBen Walker <ben@nvidia.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 13d41fb7

lib/lvol/lvol.c

+5 −6
Original line number Diff line number Diff line
@@ -993,16 +993,15 @@ lvol_close_blob_cb(void *cb_arg, int lvolerrno) 


	if (lvolerrno < 0) {

		SPDK_ERRLOG("Could not close blob on lvol\n");

		lvol_free(lvol);

		goto end;

	}



	lvol->ref_count--;

	lvol->action_in_progress = false;

	lvol->blob = NULL;

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



end:

	lvol->action_in_progress = false;

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

	free(req);

}
@@ -1575,8 +1574,6 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ 
		return;

	}



	lvol->action_in_progress = true;



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

	if (!req) {

		SPDK_ERRLOG("Cannot alloc memory for lvol request pointer\n");
@@ -1601,6 +1598,8 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ 
		return;

	}



	lvol->action_in_progress = true;



	spdk_bs_delete_blob(bs, lvol->blob_id, lvol_delete_blob_cb, req);

}
@@ -1626,8 +1625,6 @@ spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ar 
		return;

	}



	lvol->action_in_progress = true;



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

	if (!req) {

		SPDK_ERRLOG("Cannot alloc memory for lvol request pointer\n");
@@ -1639,6 +1636,8 @@ spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ar 
	req->cb_arg = cb_arg;

	req->lvol = lvol;



	lvol->action_in_progress = true;



	spdk_blob_close(lvol->blob, lvol_close_blob_cb, req);

}

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

+10 −0
Original line number Diff line number Diff line
@@ -1006,17 +1006,27 @@ lvol_close(void) 
	CU_ASSERT(cb_res.err == 0);

	SPDK_CU_ASSERT_FATAL(cb_res.data != NULL);

	lvol = cb_res.data;

	CU_ASSERT(lvol->action_in_progress == false);



	/* Fail - lvol does not exist */

	spdk_lvol_close(NULL, op_complete, ut_cb_res_clear(&cb_res));

	CU_ASSERT(cb_res.err == -ENODEV);

	CU_ASSERT(lvol->action_in_progress == false);



	/* Fail - lvol not open */

	lvol->ref_count = 0;

	spdk_lvol_close(lvol, op_complete, ut_cb_res_clear(&cb_res));

	CU_ASSERT(cb_res.err == -EINVAL);

	CU_ASSERT(lvol->action_in_progress == false);

	lvol->ref_count = 1;



	/* Fail - blob close fails */

	lvol->blob->close_status = -1;

	spdk_lvol_close(lvol, op_complete, ut_cb_res_clear(&cb_res));

	CU_ASSERT(cb_res.err == -1);

	CU_ASSERT(lvol->action_in_progress == false);

	lvol->blob->close_status = 0;



	/* Success */

	spdk_lvol_close(lvol, op_complete, ut_cb_res_clear(&cb_res));

	CU_ASSERT(cb_res.err == 0);