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

lvol: do not unload/destroy lvs when operation on lvol is pending 



There is a scenario where we can try do unload or remove
lvol store while lvol present on that lvol store is being
closed or destroyed.

Scenario:
1. send delete_bdev rpc command
2. command returns before lvol is actually closed/destroyed
   (does not wait for callback)
3. send destroy_lvol_store rpc command
4. lvs is destroyed before lvol is destroyed
5. lvol destroy callback is called on destroeyd lvol store

Aboive scenario can be reproduced using:
spdk/test/vhost/spdk_vhost.sh --integrity-lvol-scsi

Signed-off-by: default avatarMaciej Szwed <maciej.szwed@intel.com>
Change-Id: Ie715279195bd4b1145cf05d4f5a8477b4fac87f7
Reviewed-on: https://review.gerrithub.io/383595


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

include/spdk_internal/lvolstore.h

+1 −0
Original line number Diff line number Diff line
@@ -101,6 +101,7 @@ struct spdk_lvol { 
	bool				close_only;

	struct spdk_bdev		*bdev;

	int				ref_count;

	bool				action_in_progress;

	TAILQ_ENTRY(spdk_lvol) link;

};

lib/lvol/lvol.c

+25 −3
Original line number Diff line number Diff line
@@ -112,6 +112,12 @@ spdk_lvol_open(struct spdk_lvol *lvol, spdk_lvol_op_with_handle_complete cb_fn, 
		return;

	}



	if (lvol->action_in_progress == true) {

		SPDK_ERRLOG("Cannot open lvol - operations on lvol pending\n");

		cb_fn(cb_arg, lvol, -EBUSY);

		return;

	}



	if (lvol->ref_count > 0) {

		lvol->ref_count++;

		cb_fn(cb_arg, lvol, 0);
@@ -602,8 +608,13 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, 
	}



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

		if (lvol->ref_count != 0) {

		if (lvol->action_in_progress == true) {

			SPDK_ERRLOG("Cannot unload lvol store - operations on lvols pending\n");

			cb_fn(cb_arg, -EBUSY);

			return -EBUSY;

		} else if (lvol->ref_count != 0) {

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

			cb_fn(cb_arg, -EBUSY);

			return -EBUSY;

		}

	}
@@ -679,8 +690,13 @@ spdk_lvs_destroy(struct spdk_lvol_store *lvs, bool unmap_device, spdk_lvs_op_com 
	}



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

		if (iter_lvol->ref_count != 0) {

		if (iter_lvol->action_in_progress == true) {

			SPDK_ERRLOG("Cannot destroy lvol store - operations on lvols pending\n");

			cb_fn(cb_arg, -EBUSY);

			return -EBUSY;

		} else if (iter_lvol->ref_count != 0) {

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

			cb_fn(cb_arg, -EBUSY);

			return -EBUSY;

		}

	}
@@ -730,6 +746,8 @@ _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno) 
		}

	}



	lvol->action_in_progress = false;



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



	if (lvol->lvol_store->destruct_req && all_lvols_closed == true) {
@@ -1025,6 +1043,8 @@ 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");
@@ -1071,6 +1091,8 @@ 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");

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

+4 −4
Original line number Diff line number Diff line
@@ -429,9 +429,9 @@ lvs_init_unload_success(void) 


	/* Lvol store has an open lvol, this unload should fail. */

	g_lvserrno = -1;

	rc = spdk_lvs_unload(g_lvol_store, NULL, NULL);

	rc = spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL);

	CU_ASSERT(rc == -EBUSY);

	CU_ASSERT(g_lvserrno == -1);

	CU_ASSERT(g_lvserrno == -EBUSY);

	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);

	CU_ASSERT(!TAILQ_EMPTY(&g_lvol_stores));
@@ -477,9 +477,9 @@ lvs_init_destroy_success(void) 


	/* Lvol store contains one lvol, this destroy should fail. */

	g_lvserrno = -1;

	rc = spdk_lvs_destroy(g_lvol_store, true, NULL, NULL);

	rc = spdk_lvs_destroy(g_lvol_store, true, lvol_store_op_complete, NULL);

	CU_ASSERT(rc == -EBUSY);

	CU_ASSERT(g_lvserrno == -1);

	CU_ASSERT(g_lvserrno == -EBUSY);

	SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);



	spdk_lvol_close(g_lvol, close_cb, NULL);