Commit f9583f2c authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

lib/scsi: Check pending tasks for the LUN only from the specific initiator 



Refine helper functions spdk_scsi_lun_has_pending_mgmt_tasks() and
spdk_scsi_lun_has_pending_tasks() to be able to check tasks only from the
specific initiator.

SCSI port is used by passing the pointer and so simple pointer
comparison is appropriate in the functions.

Add UT code to test the updated functions.

The next patch will change spdk_scsi_dev_has_pending_tasks() to
get not only SCSI device but also initiator port and make iSCSI
target use the function to fix the issue.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I89c33e05bc6ab21baa6cbebf60950039a3dcecd0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471336


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent 47873952

lib/scsi/dev.c

+2 −2
Original line number Diff line number Diff line
@@ -425,8 +425,8 @@ spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev) 


	for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; ++i) {

		if (dev->lun[i] &&

		    (spdk_scsi_lun_has_pending_tasks(dev->lun[i]) ||

		     spdk_scsi_lun_has_pending_mgmt_tasks(dev->lun[i]))) {

		    (spdk_scsi_lun_has_pending_tasks(dev->lun[i], NULL) ||

		     spdk_scsi_lun_has_pending_mgmt_tasks(dev->lun[i], NULL))) {

			return true;

		}

	}

lib/scsi/lun.c

+42 −4
Original line number Diff line number Diff line
@@ -504,17 +504,55 @@ spdk_scsi_lun_get_dev(const struct spdk_scsi_lun *lun) 
}



bool

spdk_scsi_lun_has_pending_mgmt_tasks(const struct spdk_scsi_lun *lun)

spdk_scsi_lun_has_pending_mgmt_tasks(const struct spdk_scsi_lun *lun,

				     const struct spdk_scsi_port *initiator_port)

{

	struct spdk_scsi_task *task;



	if (initiator_port == NULL) {

		return scsi_lun_has_pending_mgmt_tasks(lun);

	}



	TAILQ_FOREACH(task, &lun->pending_mgmt_tasks, scsi_link) {

		if (task->initiator_port == initiator_port) {

			return true;

		}

	}



	TAILQ_FOREACH(task, &lun->mgmt_tasks, scsi_link) {

		if (task->initiator_port == initiator_port) {

			return true;

		}

	}



	return false;

}



bool

spdk_scsi_lun_has_pending_tasks(const struct spdk_scsi_lun *lun)

spdk_scsi_lun_has_pending_tasks(const struct spdk_scsi_lun *lun,

				const struct spdk_scsi_port *initiator_port)

{

	struct spdk_scsi_task *task;



	if (initiator_port == NULL) {

		return scsi_lun_has_pending_tasks(lun);

	}



	TAILQ_FOREACH(task, &lun->pending_tasks, scsi_link) {

		if (task->initiator_port == initiator_port) {

			return true;

		}

	}



	TAILQ_FOREACH(task, &lun->tasks, scsi_link) {

		if (task->initiator_port == initiator_port) {

			return true;

		}

	}



	return false;

}



bool

spdk_scsi_lun_is_removing(const struct spdk_scsi_lun *lun)

{

lib/scsi/scsi_internal.h

+4 −2
Original line number Diff line number Diff line
@@ -182,10 +182,12 @@ void spdk_scsi_lun_append_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task 
void spdk_scsi_lun_execute_tasks(struct spdk_scsi_lun *lun);

void spdk_scsi_lun_append_mgmt_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task);

void spdk_scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun);

bool spdk_scsi_lun_has_pending_mgmt_tasks(const struct spdk_scsi_lun *lun);

bool spdk_scsi_lun_has_pending_mgmt_tasks(const struct spdk_scsi_lun *lun,

		const struct spdk_scsi_port *initiator_port);

void spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task);

void spdk_scsi_lun_complete_reset_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task);

bool spdk_scsi_lun_has_pending_tasks(const struct spdk_scsi_lun *lun);

bool spdk_scsi_lun_has_pending_tasks(const struct spdk_scsi_lun *lun,

				     const struct spdk_scsi_port *initiator_port);

int _spdk_scsi_lun_allocate_io_channel(struct spdk_scsi_lun *lun);

void _spdk_scsi_lun_free_io_channel(struct spdk_scsi_lun *lun);

test/unit/lib/scsi/dev.c/dev_ut.c

+4 −2
Original line number Diff line number Diff line
@@ -119,7 +119,8 @@ DEFINE_STUB_V(spdk_scsi_lun_append_mgmt_task, 
DEFINE_STUB_V(spdk_scsi_lun_execute_mgmt_task, (struct spdk_scsi_lun *lun));



DEFINE_STUB(spdk_scsi_lun_has_pending_mgmt_tasks, bool,

	    (const struct spdk_scsi_lun *lun), false);

	    (const struct spdk_scsi_lun *lun, const struct spdk_scsi_port *initiator_port),

	    false);



DEFINE_STUB_V(spdk_scsi_lun_append_task,

	      (struct spdk_scsi_lun *lun, struct spdk_scsi_task *task));
@@ -132,7 +133,8 @@ DEFINE_STUB(_spdk_scsi_lun_allocate_io_channel, int, 
DEFINE_STUB_V(_spdk_scsi_lun_free_io_channel, (struct spdk_scsi_lun *lun));



DEFINE_STUB(spdk_scsi_lun_has_pending_tasks, bool,

	    (const struct spdk_scsi_lun *lun), false);

	    (const struct spdk_scsi_lun *lun, const struct spdk_scsi_port *initiator_port),

	    false);



static void

dev_destruct_null_dev(void)

test/unit/lib/scsi/lun.c/lun_ut.c

+71 −0
Original line number Diff line number Diff line
@@ -597,6 +597,75 @@ lun_reset_task_suspend_scsi_task(void) 
	CU_ASSERT_EQUAL(g_task_count, 0);

}



static void

lun_check_pending_tasks_only_for_specific_initiator(void)

{

	struct spdk_bdev bdev = {};

	struct spdk_scsi_lun *lun;

	struct spdk_scsi_task task1 = {};

	struct spdk_scsi_task task2 = {};

	struct spdk_scsi_port initiator_port1 = {};

	struct spdk_scsi_port initiator_port2 = {};

	struct spdk_scsi_port initiator_port3 = {};



	lun = spdk_scsi_lun_construct(&bdev, NULL, NULL);



	task1.initiator_port = &initiator_port1;

	task2.initiator_port = &initiator_port2;



	TAILQ_INSERT_TAIL(&lun->tasks, &task1, scsi_link);

	TAILQ_INSERT_TAIL(&lun->tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_outstanding_tasks(lun) == true);

	CU_ASSERT(scsi_lun_has_pending_tasks(lun) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, NULL) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, &initiator_port1) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, &initiator_port2) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, &initiator_port3) == false);

	TAILQ_REMOVE(&lun->tasks, &task1, scsi_link);

	TAILQ_REMOVE(&lun->tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_pending_tasks(lun) == false);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, NULL) == false);



	TAILQ_INSERT_TAIL(&lun->pending_tasks, &task1, scsi_link);

	TAILQ_INSERT_TAIL(&lun->pending_tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_outstanding_tasks(lun) == false);

	CU_ASSERT(scsi_lun_has_pending_tasks(lun) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, NULL) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, &initiator_port1) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, &initiator_port2) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, &initiator_port3) == false);

	TAILQ_REMOVE(&lun->pending_tasks, &task1, scsi_link);

	TAILQ_REMOVE(&lun->pending_tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_pending_tasks(lun) == false);

	CU_ASSERT(spdk_scsi_lun_has_pending_tasks(lun, NULL) == false);



	TAILQ_INSERT_TAIL(&lun->mgmt_tasks, &task1, scsi_link);

	TAILQ_INSERT_TAIL(&lun->mgmt_tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_pending_mgmt_tasks(lun) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, NULL) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, &initiator_port1) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, &initiator_port2) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, &initiator_port3) == false);

	TAILQ_REMOVE(&lun->mgmt_tasks, &task1, scsi_link);

	TAILQ_REMOVE(&lun->mgmt_tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_pending_mgmt_tasks(lun) == false);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, NULL) == false);



	TAILQ_INSERT_TAIL(&lun->pending_mgmt_tasks, &task1, scsi_link);

	TAILQ_INSERT_TAIL(&lun->pending_mgmt_tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_pending_mgmt_tasks(lun) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, NULL) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, &initiator_port1) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, &initiator_port2) == true);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, &initiator_port3) == false);

	TAILQ_REMOVE(&lun->pending_mgmt_tasks, &task1, scsi_link);

	TAILQ_REMOVE(&lun->pending_mgmt_tasks, &task2, scsi_link);

	CU_ASSERT(scsi_lun_has_pending_mgmt_tasks(lun) == false);

	CU_ASSERT(spdk_scsi_lun_has_pending_mgmt_tasks(lun, NULL) == false);



	scsi_lun_remove(lun);

}



int

main(int argc, char **argv)

{
@@ -639,6 +708,8 @@ main(int argc, char **argv) 
			       lun_reset_task_wait_scsi_task_complete) == NULL

		|| CU_add_test(suite, "reset task suspend subsequent scsi task",

			       lun_reset_task_suspend_scsi_task) == NULL

		|| CU_add_test(suite, "check pending tasks only for specific initiator",

			       lun_check_pending_tasks_only_for_specific_initiator) == NULL

	) {

		CU_cleanup_registry();

		return CU_get_error();