From f9583f2c0d911a19b5bee89241259d9f18ff5fd6 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 15 Oct 2019 16:22:38 +0900 Subject: [PATCH] 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: Shuhei Matsumoto Change-Id: I89c33e05bc6ab21baa6cbebf60950039a3dcecd0 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471336 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker Reviewed-by: Changpeng Liu --- lib/scsi/dev.c | 4 +- lib/scsi/lun.c | 46 ++++++++++++++++++-- lib/scsi/scsi_internal.h | 6 ++- test/unit/lib/scsi/dev.c/dev_ut.c | 6 ++- test/unit/lib/scsi/lun.c/lun_ut.c | 71 +++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 10 deletions(-) diff --git a/lib/scsi/dev.c b/lib/scsi/dev.c index e567b4033..1ea722553 100644 --- a/lib/scsi/dev.c +++ b/lib/scsi/dev.c @@ -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; } } diff --git a/lib/scsi/lun.c b/lib/scsi/lun.c index 382db49a1..c9fd05de8 100644 --- a/lib/scsi/lun.c +++ b/lib/scsi/lun.c @@ -504,15 +504,53 @@ 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) { - return scsi_lun_has_pending_mgmt_tasks(lun); + 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) { - return scsi_lun_has_pending_tasks(lun); + 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 diff --git a/lib/scsi/scsi_internal.h b/lib/scsi/scsi_internal.h index 8ad47f62e..299f96228 100644 --- a/lib/scsi/scsi_internal.h +++ b/lib/scsi/scsi_internal.h @@ -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); diff --git a/test/unit/lib/scsi/dev.c/dev_ut.c b/test/unit/lib/scsi/dev.c/dev_ut.c index 108959311..16e4b5434 100644 --- a/test/unit/lib/scsi/dev.c/dev_ut.c +++ b/test/unit/lib/scsi/dev.c/dev_ut.c @@ -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) diff --git a/test/unit/lib/scsi/lun.c/lun_ut.c b/test/unit/lib/scsi/lun.c/lun_ut.c index 87879854b..cd1c049e7 100644 --- a/test/unit/lib/scsi/lun.c/lun_ut.c +++ b/test/unit/lib/scsi/lun.c/lun_ut.c @@ -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();