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

Refine the public helper function spdk_scsi_dev_has_pending_tasks to
be able to check tasks only from the specific initiator.

Then use the function in iSCSI target to fix the issue.

Besides add UT code to test the updated spdk_scsi_dev_has_pending_tasks().
Automated multi hosts test is much better but some UT code will be of any
help to mitigate the risk of degradation.

Fixes #985

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I50afb940de7174360c8a30479450850002a3e525
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471337
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
This commit is contained in:
Shuhei Matsumoto 2019-10-15 16:46:15 +09:00 committed by Tomasz Zawadzki
parent f9583f2c0d
commit 1dc9a7627f
6 changed files with 61 additions and 15 deletions

View File

@ -233,10 +233,13 @@ struct spdk_scsi_lun *spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_i
* Check whether the SCSI device has any pending task.
*
* \param dev SCSI device.
* \param initiator_port Check tasks only from the initiator if specified, or
* all all tasks otherwise.
*
* \return true if the SCSI device has any pending task, or false otherwise.
*/
bool spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev);
bool spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev,
const struct spdk_scsi_port *initiator_port);
/**
* Destruct the SCSI decice.

View File

@ -660,7 +660,8 @@ _iscsi_conn_check_pending_tasks(void *arg)
{
struct spdk_iscsi_conn *conn = arg;
if (conn->dev != NULL && spdk_scsi_dev_has_pending_tasks(conn->dev)) {
if (conn->dev != NULL &&
spdk_scsi_dev_has_pending_tasks(conn->dev, conn->initiator_port)) {
return 1;
}
@ -685,7 +686,8 @@ spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn)
iscsi_conn_cleanup_backend(conn);
}
if (conn->dev != NULL && spdk_scsi_dev_has_pending_tasks(conn->dev)) {
if (conn->dev != NULL &&
spdk_scsi_dev_has_pending_tasks(conn->dev, conn->initiator_port)) {
conn->shutdown_timer = spdk_poller_register(_iscsi_conn_check_pending_tasks, conn, 1000);
} else {
_iscsi_conn_destruct(conn);

View File

@ -419,14 +419,15 @@ spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_id)
}
bool
spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev)
spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev,
const struct spdk_scsi_port *initiator_port)
{
int i;
for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; ++i) {
if (dev->lun[i] &&
(spdk_scsi_lun_has_pending_tasks(dev->lun[i], NULL) ||
spdk_scsi_lun_has_pending_mgmt_tasks(dev->lun[i], NULL))) {
(spdk_scsi_lun_has_pending_tasks(dev->lun[i], initiator_port) ||
spdk_scsi_lun_has_pending_mgmt_tasks(dev->lun[i], initiator_port))) {
return true;
}
}

View File

@ -250,7 +250,8 @@ process_removed_devs(struct spdk_vhost_scsi_session *svsession)
state = &svsession->scsi_dev_state[i];
dev = state->dev;
if (dev && state->status == VHOST_SCSI_DEV_REMOVING && !spdk_scsi_dev_has_pending_tasks(dev)) {
if (dev && state->status == VHOST_SCSI_DEV_REMOVING &&
!spdk_scsi_dev_has_pending_tasks(dev, NULL)) {
/* detach the device from this session */
spdk_scsi_dev_free_io_channels(dev);
state->dev = NULL;

View File

@ -90,7 +90,8 @@ DEFINE_STUB(spdk_scsi_dev_get_lun, struct spdk_scsi_lun *,
(struct spdk_scsi_dev *dev, int lun_id), NULL);
DEFINE_STUB(spdk_scsi_dev_has_pending_tasks, bool,
(const struct spdk_scsi_dev *dev), true);
(const struct spdk_scsi_dev *dev, const struct spdk_scsi_port *initiator_port),
true);
DEFINE_STUB(spdk_scsi_lun_open, int,
(struct spdk_scsi_lun *lun, spdk_scsi_lun_remove_cb_t hotremove_cb,

View File

@ -53,6 +53,9 @@ static struct spdk_bdev g_bdevs[] = {
{"malloc1"},
};
static struct spdk_scsi_port *g_initiator_port_with_pending_tasks = NULL;
static struct spdk_scsi_port *g_initiator_port_with_pending_mgmt_tasks = NULL;
const char *
spdk_bdev_get_name(const struct spdk_bdev *bdev)
{
@ -118,10 +121,6 @@ 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, 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,9 +131,19 @@ 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, const struct spdk_scsi_port *initiator_port),
false);
bool
spdk_scsi_lun_has_pending_mgmt_tasks(const struct spdk_scsi_lun *lun,
const struct spdk_scsi_port *initiator_port)
{
return (g_initiator_port_with_pending_mgmt_tasks == initiator_port);
}
bool
spdk_scsi_lun_has_pending_tasks(const struct spdk_scsi_lun *lun,
const struct spdk_scsi_port *initiator_port)
{
return (g_initiator_port_with_pending_tasks == initiator_port);
}
static void
dev_destruct_null_dev(void)
@ -603,6 +612,33 @@ dev_add_lun_success2(void)
spdk_scsi_dev_destruct(&dev, NULL, NULL);
}
static void
dev_check_pending_tasks(void)
{
struct spdk_scsi_dev dev = {};
struct spdk_scsi_lun lun = {};
struct spdk_scsi_port initiator_port = {};
g_initiator_port_with_pending_tasks = NULL;
g_initiator_port_with_pending_mgmt_tasks = NULL;
CU_ASSERT(spdk_scsi_dev_has_pending_tasks(&dev, NULL) == false);
dev.lun[SPDK_SCSI_DEV_MAX_LUN - 1] = &lun;
CU_ASSERT(spdk_scsi_dev_has_pending_tasks(&dev, NULL) == true);
CU_ASSERT(spdk_scsi_dev_has_pending_tasks(&dev, &initiator_port) == false);
g_initiator_port_with_pending_tasks = &initiator_port;
CU_ASSERT(spdk_scsi_dev_has_pending_tasks(&dev, NULL) == true);
CU_ASSERT(spdk_scsi_dev_has_pending_tasks(&dev, &initiator_port) == true);
g_initiator_port_with_pending_tasks = NULL;
g_initiator_port_with_pending_mgmt_tasks = &initiator_port;
CU_ASSERT(spdk_scsi_dev_has_pending_tasks(&dev, NULL) == true);
CU_ASSERT(spdk_scsi_dev_has_pending_tasks(&dev, &initiator_port) == true);
}
int
main(int argc, char **argv)
{
@ -666,6 +702,8 @@ main(int argc, char **argv)
dev_add_lun_success1) == NULL
|| CU_add_test(suite, "dev add lun - success 2",
dev_add_lun_success2) == NULL
|| CU_add_test(suite, "dev check pending tasks",
dev_check_pending_tasks) == NULL
) {
CU_cleanup_registry();
return CU_get_error();