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

lib/iscsi: Factor out non-read task completion from spdk_iscsi_task_cpl() 



Read task completion has been factored out into
process_read_task_completion(). Factoring out non-read task completion
into process_non_read_task_completion() makes the code a little
clearer and makes us possible to add unit tests.

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


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

lib/iscsi/conn.c

+45 −37
Original line number Diff line number Diff line
@@ -1086,22 +1086,11 @@ process_read_task_completion(struct spdk_iscsi_conn *conn, 
	process_completed_read_subtask_list(conn, primary);

}



void

spdk_iscsi_task_cpl(struct spdk_scsi_task *scsi_task)

static void

process_non_read_task_completion(struct spdk_iscsi_conn *conn,

				 struct spdk_iscsi_task *task,

				 struct spdk_iscsi_task *primary)

{

	struct spdk_iscsi_task *primary;

	struct spdk_iscsi_task *task = spdk_iscsi_task_from_scsi_task(scsi_task);

	struct spdk_iscsi_conn *conn = task->conn;

	struct spdk_iscsi_pdu *pdu = task->pdu;



	spdk_trace_record(TRACE_ISCSI_TASK_DONE, conn->id, 0, (uintptr_t)task, 0);



	task->is_queued = false;

	primary = spdk_iscsi_task_get_primary(task);



	if (spdk_iscsi_task_is_read(primary)) {

		process_read_task_completion(conn, task, primary);

	} else {

	primary->bytes_completed += task->scsi.length;



	/* If the status of the subtask is the first failure, remember it as
@@ -1140,6 +1129,25 @@ spdk_iscsi_task_cpl(struct spdk_scsi_task *scsi_task) 
	}

	spdk_iscsi_task_put(task);

}



void

spdk_iscsi_task_cpl(struct spdk_scsi_task *scsi_task)

{

	struct spdk_iscsi_task *primary;

	struct spdk_iscsi_task *task = spdk_iscsi_task_from_scsi_task(scsi_task);

	struct spdk_iscsi_conn *conn = task->conn;

	struct spdk_iscsi_pdu *pdu = task->pdu;



	spdk_trace_record(TRACE_ISCSI_TASK_DONE, conn->id, 0, (uintptr_t)task, 0);



	task->is_queued = false;

	primary = spdk_iscsi_task_get_primary(task);



	if (spdk_iscsi_task_is_read(primary)) {

		process_read_task_completion(conn, task, primary);

	} else {

		process_non_read_task_completion(conn, task, primary);

	}

	if (!task->parent) {

		spdk_trace_record(TRACE_ISCSI_PDU_COMPLETED, 0, 0, (uintptr_t)pdu, 0);

	}

test/unit/lib/iscsi/conn.c/conn_ut.c

+65 −0
Original line number Diff line number Diff line
@@ -358,6 +358,69 @@ propagate_scsi_error_status_for_split_read_tasks(void) 
	CU_ASSERT(TAILQ_EMPTY(&primary.subtask_list));

}



static void

process_non_read_task_completion_test(void)

{

	struct spdk_iscsi_conn conn = {};

	struct spdk_iscsi_task primary = {};

	struct spdk_iscsi_task task = {};



	TAILQ_INIT(&conn.active_r2t_tasks);



	primary.bytes_completed = 0;

	primary.scsi.transfer_len = 4096 * 3;

	primary.rsp_scsi_status = SPDK_SCSI_STATUS_GOOD;

	TAILQ_INSERT_TAIL(&conn.active_r2t_tasks, &primary, link);



	/* First subtask which failed. */

	task.scsi.length = 4096;

	task.scsi.data_transferred = 4096;

	task.scsi.status = SPDK_SCSI_STATUS_CHECK_CONDITION;



	process_non_read_task_completion(&conn, &task, &primary);

	CU_ASSERT(!TAILQ_EMPTY(&conn.active_r2t_tasks));

	CU_ASSERT(primary.bytes_completed == 4096);

	CU_ASSERT(primary.scsi.data_transferred == 0);

	CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_CHECK_CONDITION);



	/* Second subtask which succeeded. */

	task.scsi.length = 4096;

	task.scsi.data_transferred = 4096;

	task.scsi.status = SPDK_SCSI_STATUS_GOOD;



	process_non_read_task_completion(&conn, &task, &primary);

	CU_ASSERT(!TAILQ_EMPTY(&conn.active_r2t_tasks));

	CU_ASSERT(primary.bytes_completed == 4096 * 2);

	CU_ASSERT(primary.scsi.data_transferred == 4096);

	CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_CHECK_CONDITION);



	/* Third and final subtask which succeeded. */

	task.scsi.length = 4096;

	task.scsi.data_transferred = 4096;

	task.scsi.status = SPDK_SCSI_STATUS_GOOD;



	process_non_read_task_completion(&conn, &task, &primary);

	CU_ASSERT(TAILQ_EMPTY(&conn.active_r2t_tasks));

	CU_ASSERT(primary.bytes_completed == 4096 * 3);

	CU_ASSERT(primary.scsi.data_transferred == 4096 * 2);

	CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_CHECK_CONDITION);



	/* Tricky case when the last task completed was the initial task. */

	primary.scsi.length = 4096;

	primary.bytes_completed = 4096 * 2;

	primary.scsi.data_transferred = 4096 * 2;

	primary.scsi.transfer_len = 4096 * 3;

	primary.scsi.status = SPDK_SCSI_STATUS_GOOD;

	primary.rsp_scsi_status = SPDK_SCSI_STATUS_GOOD;

	TAILQ_INSERT_TAIL(&conn.active_r2t_tasks, &primary, link);



	process_non_read_task_completion(&conn, &primary, &primary);

	CU_ASSERT(TAILQ_EMPTY(&conn.active_r2t_tasks));

	CU_ASSERT(primary.bytes_completed == 4096 * 3);

	CU_ASSERT(primary.scsi.data_transferred == 4096 * 2);

	CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_GOOD);

}



static void

recursive_flush_pdus_calls(void)

{
@@ -584,6 +647,8 @@ main(int argc, char **argv) 
			    read_task_split_reverse_order_case) == NULL ||

		CU_add_test(suite, "propagate_scsi_error_status_for_split_read_tasks",

			    propagate_scsi_error_status_for_split_read_tasks) == NULL ||

		CU_add_test(suite, "process_non_read_task_completion_test",

			    process_non_read_task_completion_test) == NULL ||

		CU_add_test(suite, "recursive_flush_pdus_calls", recursive_flush_pdus_calls) == NULL ||

		CU_add_test(suite, "free_tasks_on_connection", free_tasks_on_connection) == NULL

	) {