diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index d361a8767..1550452d1 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -914,6 +914,25 @@ spdk_iscsi_task_mgmt_cpl(struct spdk_scsi_task *scsi_task) spdk_iscsi_task_put(task); } +static void +spdk_iscsi_task_copy_to_rsp_scsi_status(struct spdk_iscsi_task *primary, + struct spdk_scsi_task *task) +{ + memcpy(primary->rsp_sense_data, task->sense_data, task->sense_data_len); + primary->rsp_sense_data_len = task->sense_data_len; + primary->rsp_scsi_status = task->status; +} + +static void +spdk_iscsi_task_copy_from_rsp_scsi_status(struct spdk_scsi_task *task, + struct spdk_iscsi_task *primary) +{ + memcpy(task->sense_data, primary->rsp_sense_data, + primary->rsp_sense_data_len); + task->sense_data_len = primary->rsp_sense_data_len; + task->status = primary->rsp_scsi_status; +} + static void process_completed_read_subtask_list(struct spdk_iscsi_conn *conn, struct spdk_iscsi_task *primary) @@ -939,10 +958,23 @@ process_read_task_completion(struct spdk_iscsi_conn *conn, { struct spdk_iscsi_task *tmp; + /* If the status of the completed subtask is the first failure, + * copy it to out-of-order subtasks and remember it as the status + * of the command, + * + * Even if the status of the completed task is success, + * there are any failed subtask ever, copy the first failed status + * to it. + */ if (task->scsi.status != SPDK_SCSI_STATUS_GOOD) { - TAILQ_FOREACH(tmp, &primary->subtask_list, subtask_link) { - spdk_scsi_task_copy_status(&tmp->scsi, &task->scsi); + if (primary->rsp_scsi_status == SPDK_SCSI_STATUS_GOOD) { + TAILQ_FOREACH(tmp, &primary->subtask_list, subtask_link) { + spdk_scsi_task_copy_status(&tmp->scsi, &task->scsi); + } + spdk_iscsi_task_copy_to_rsp_scsi_status(primary, &task->scsi); } + } else if (primary->rsp_scsi_status != SPDK_SCSI_STATUS_GOOD) { + spdk_iscsi_task_copy_from_rsp_scsi_status(&task->scsi, primary); } if ((task != primary) && @@ -987,16 +1019,27 @@ spdk_iscsi_task_cpl(struct spdk_scsi_task *scsi_task) process_read_task_completion(conn, task, primary); } else { primary->bytes_completed += task->scsi.length; - if (task != primary) { - if (task->scsi.status == SPDK_SCSI_STATUS_GOOD) { + + /* If the status of the subtask is the first failure, remember it as + * the status of the command and set it to the status of the primary + * task later. + * + * If the first failed task is the primary, two copies can be avoided + * but code simplicity is prioritized. + */ + if (task->scsi.status == SPDK_SCSI_STATUS_GOOD) { + if (task != primary) { primary->scsi.data_transferred += task->scsi.data_transferred; - } else { - spdk_scsi_task_copy_status(&primary->scsi, &task->scsi); } + } else if (primary->rsp_scsi_status == SPDK_SCSI_STATUS_GOOD) { + spdk_iscsi_task_copy_to_rsp_scsi_status(primary, &task->scsi); } if (primary->bytes_completed == primary->scsi.transfer_len) { spdk_del_transfer_task(conn, primary->tag); + if (primary->rsp_scsi_status != SPDK_SCSI_STATUS_GOOD) { + spdk_iscsi_task_copy_from_rsp_scsi_status(&primary->scsi, primary); + } spdk_iscsi_task_response(conn, primary); /* * Check if this is the last task completed for an iSCSI write diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index 4d4e4e483..34329c72b 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -2881,6 +2881,7 @@ spdk_iscsi_op_scsi(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) task->scsi.target_port = conn->target_port; task->scsi.initiator_port = conn->initiator_port; task->parent = NULL; + task->rsp_scsi_status = SPDK_SCSI_STATUS_GOOD; if (task->scsi.lun == NULL) { spdk_scsi_task_process_null_lun(&task->scsi); diff --git a/lib/iscsi/task.h b/lib/iscsi/task.h index 993e65cb8..453824aae 100644 --- a/lib/iscsi/task.h +++ b/lib/iscsi/task.h @@ -44,6 +44,10 @@ struct spdk_iscsi_task { struct spdk_iscsi_task *parent; + uint8_t rsp_scsi_status; + uint8_t rsp_sense_data[32]; + size_t rsp_sense_data_len; + struct spdk_iscsi_conn *conn; struct spdk_iscsi_pdu *pdu; uint32_t outstanding_r2t; diff --git a/test/unit/lib/iscsi/conn.c/conn_ut.c b/test/unit/lib/iscsi/conn.c/conn_ut.c index 1b7e05185..b99ff9ac4 100644 --- a/test/unit/lib/iscsi/conn.c/conn_ut.c +++ b/test/unit/lib/iscsi/conn.c/conn_ut.c @@ -120,8 +120,12 @@ DEFINE_STUB(spdk_scsi_lun_get_id, int, (const struct spdk_scsi_lun *lun), 0); DEFINE_STUB(spdk_scsi_port_get_name, const char *, (const struct spdk_scsi_port *port), NULL); -DEFINE_STUB_V(spdk_scsi_task_copy_status, - (struct spdk_scsi_task *dst, struct spdk_scsi_task *src)); +void +spdk_scsi_task_copy_status(struct spdk_scsi_task *dst, + struct spdk_scsi_task *src) +{ + dst->status = src->status; +} DEFINE_STUB_V(spdk_put_pdu, (struct spdk_iscsi_pdu *pdu)); @@ -264,6 +268,69 @@ read_task_split_in_order_case(void) CU_ASSERT(TAILQ_EMPTY(&g_ut_read_tasks)); } +static void +propagate_scsi_error_status_for_split_read_tasks(void) +{ + struct spdk_iscsi_task primary, task1, task2, task3, task4, task5, task6; + + memset(&primary, 0, sizeof(struct spdk_iscsi_task)); + primary.scsi.length = 512; + primary.scsi.status = SPDK_SCSI_STATUS_GOOD; + primary.rsp_scsi_status = SPDK_SCSI_STATUS_GOOD; + TAILQ_INIT(&primary.subtask_list); + + memset(&task1, 0, sizeof(struct spdk_iscsi_task)); + task1.scsi.offset = 512; + task1.scsi.length = 512; + task1.scsi.status = SPDK_SCSI_STATUS_GOOD; + + memset(&task2, 0, sizeof(struct spdk_iscsi_task)); + task2.scsi.offset = 512 * 2; + task2.scsi.length = 512; + task2.scsi.status = SPDK_SCSI_STATUS_CHECK_CONDITION; + + memset(&task3, 0, sizeof(struct spdk_iscsi_task)); + task3.scsi.offset = 512 * 3; + task3.scsi.length = 512; + task3.scsi.status = SPDK_SCSI_STATUS_GOOD; + + memset(&task4, 0, sizeof(struct spdk_iscsi_task)); + task4.scsi.offset = 512 * 4; + task4.scsi.length = 512; + task4.scsi.status = SPDK_SCSI_STATUS_GOOD; + + memset(&task5, 0, sizeof(struct spdk_iscsi_task)); + task5.scsi.offset = 512 * 5; + task5.scsi.length = 512; + task5.scsi.status = SPDK_SCSI_STATUS_GOOD; + + memset(&task6, 0, sizeof(struct spdk_iscsi_task)); + task6.scsi.offset = 512 * 6; + task6.scsi.length = 512; + task6.scsi.status = SPDK_SCSI_STATUS_GOOD; + + /* task2 has check condition status, and verify if the check condition + * status is propagated to remaining tasks correctly when these tasks complete + * by the following order, task4, task3, task2, task1, primary, task5, and task6. + */ + process_read_task_completion(NULL, &task4, &primary); + process_read_task_completion(NULL, &task3, &primary); + process_read_task_completion(NULL, &task2, &primary); + process_read_task_completion(NULL, &task1, &primary); + process_read_task_completion(NULL, &primary, &primary); + process_read_task_completion(NULL, &task5, &primary); + process_read_task_completion(NULL, &task6, &primary); + + CU_ASSERT(primary.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(task1.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(task2.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(task3.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(task4.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(task5.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(task6.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(TAILQ_EMPTY(&primary.subtask_list)); +} + int main(int argc, char **argv) { @@ -281,7 +348,9 @@ main(int argc, char **argv) } if ( - CU_add_test(suite, "read task split in order", read_task_split_in_order_case) == NULL + CU_add_test(suite, "read task split in order", read_task_split_in_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_cleanup_registry(); return CU_get_error();