Commit be633ff1 authored by Oded Mashbach's avatar Oded Mashbach Committed by Tomasz Zawadzki
Browse files

scsi pr: passig all registrant reservation to next registrant inline 



When a registrant releases an "all registrant" reservation (mode 7/8),
we should pass the reservation holder to the first registrant in the
lun's
registrant list that is not the same registrant that is releasing the
reservation.
other wise we might face the following bugs:
1. when A is the first registrant and B is the second:
   Once A will release his mode 7/8 reservation - it will stay under A
   and not move to B.
2. when A is the only registrant that exist, Once A will try to release
   the reservation of mode 7/8 - it will not work.

Change-Id: Ib4f1e82283725638c5d2ea57b6ec7c19fff543aa
Signed-off-by: default avatarOded Mashbach <oded.mashbach@silk.us>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/23614


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
parent 8ee698f8

lib/scsi/scsi_pr.c

+9 −3
Original line number Diff line number Diff line
@@ -111,16 +111,22 @@ static void 
scsi_pr_release_reservation(struct spdk_scsi_lun *lun, struct spdk_scsi_pr_registrant *reg)

{

	bool all_regs = false;

	struct spdk_scsi_pr_registrant *curr_reg, *tmp;



	SPDK_DEBUGLOG(scsi, "REGISTER: release reservation "

		      "with type %u\n", lun->reservation.rtype);



	/* TODO: Unit Attention */

	all_regs = scsi_pr_is_all_registrants_type(lun);

	if (all_regs && !TAILQ_EMPTY(&lun->reg_head)) {

		lun->reservation.holder = TAILQ_FIRST(&lun->reg_head);

	if (all_regs) {

		TAILQ_FOREACH_SAFE(curr_reg, &lun->reg_head, link, tmp) {

			if (curr_reg != reg) {

				lun->reservation.holder = curr_reg;

				lun->reservation.crkey = curr_reg->rkey;

				return;

			}

		}

	}



	memset(&lun->reservation, 0, sizeof(struct spdk_scsi_pr_reservation));

}

test/unit/lib/scsi/scsi_pr.c/scsi_pr_ut.c

+73 −0
Original line number Diff line number Diff line
@@ -205,6 +205,78 @@ test_reservation_register(void) 
	ut_deinit_reservation_test();

}



static void

test_all_registrant_reservation_reserve(void)

{

	struct spdk_scsi_pr_registrant *reg;

	struct spdk_scsi_task task = {0};

	uint32_t gen;

	int rc;



	task.lun = &g_lun;

	task.target_port = &g_t_port_0;



	ut_init_reservation_test();



	test_build_registrants();

	gen = g_lun.pr_generation;

	/* Test Case: Host A takes all registrant reservation */

	task.initiator_port = &g_i_port_a;

	task.status = 0;

	rc = scsi_pr_out_reserve(&task, SPDK_SCSI_PR_WRITE_EXCLUSIVE_ALL_REGS,

				 0xa, 0, 0, 0);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	SPDK_CU_ASSERT_FATAL(task.status == 0);

	SPDK_CU_ASSERT_FATAL(g_lun.reservation.rtype == SPDK_SCSI_PR_WRITE_EXCLUSIVE_ALL_REGS);

	SPDK_CU_ASSERT_FATAL(g_lun.reservation.crkey == 0xa);

	SPDK_CU_ASSERT_FATAL(g_lun.pr_generation == gen);



	/* Test case: Host A release reservation - which should pass to next inline -> Host B */

	task.initiator_port = &g_i_port_a;

	task.status = 0;

	rc = scsi_pr_out_release(&task, SPDK_SCSI_PR_WRITE_EXCLUSIVE_ALL_REGS, 0xa);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	SPDK_CU_ASSERT_FATAL(task.status == 0);

	SPDK_CU_ASSERT_FATAL(g_lun.reservation.rtype == SPDK_SCSI_PR_WRITE_EXCLUSIVE_ALL_REGS);

	SPDK_CU_ASSERT_FATAL(g_lun.reservation.crkey == 0xb);

	SPDK_CU_ASSERT_FATAL(g_lun.pr_generation == gen);



	/* Test case: Host A unregister + Host C unregister: Host B left alone.

	 * Host B than releases reservation - lun should not have any reservation holder *

	 */

	task.initiator_port = &g_i_port_a;

	task.status = 0;

	rc = scsi_pr_out_register(&task, SPDK_SCSI_PR_OUT_REGISTER,

				  0xa, 0, 0, 0, 0);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	SPDK_CU_ASSERT_FATAL(task.status == 0);

	reg = scsi_pr_get_registrant(&g_lun, &g_i_port_a, &g_t_port_0);

	SPDK_CU_ASSERT_FATAL(reg == NULL);

	SPDK_CU_ASSERT_FATAL(g_lun.pr_generation == ++gen);



	task.initiator_port = &g_i_port_c;

	task.status = 0;

	rc = scsi_pr_out_register(&task, SPDK_SCSI_PR_OUT_REGISTER,

				  0xc, 0, 0, 0, 0);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	SPDK_CU_ASSERT_FATAL(task.status == 0);

	reg = scsi_pr_get_registrant(&g_lun, &g_i_port_c, &g_t_port_0);

	SPDK_CU_ASSERT_FATAL(reg == NULL);

	SPDK_CU_ASSERT_FATAL(g_lun.pr_generation == ++gen);



	task.initiator_port = &g_i_port_b;

	task.status = 0;

	rc = scsi_pr_out_release(&task, SPDK_SCSI_PR_WRITE_EXCLUSIVE_ALL_REGS, 0xb);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	SPDK_CU_ASSERT_FATAL(task.status == 0);

	SPDK_CU_ASSERT_FATAL(g_lun.reservation.rtype == 0x0);

	SPDK_CU_ASSERT_FATAL(g_lun.reservation.crkey == 0x0);

	SPDK_CU_ASSERT_FATAL(g_lun.reservation.holder == NULL);

	SPDK_CU_ASSERT_FATAL(g_lun.pr_generation == gen);



	ut_deinit_reservation_test();

}



static void

test_reservation_reserve(void)

{
@@ -634,6 +706,7 @@ main(int argc, char **argv) 
	suite = CU_add_suite("reservation_suite", NULL, NULL);

	CU_ADD_TEST(suite, test_reservation_register);

	CU_ADD_TEST(suite, test_reservation_reserve);

	CU_ADD_TEST(suite, test_all_registrant_reservation_reserve);

	CU_ADD_TEST(suite, test_reservation_preempt_non_all_regs);

	CU_ADD_TEST(suite, test_reservation_preempt_all_regs);

	CU_ADD_TEST(suite, test_reservation_cmds_conflict);