From a15c2a5c61c587ded738d71c1bcf81470ea4babe Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 22 Oct 2018 16:08:49 -0700 Subject: [PATCH 1/4] test-tool/test_prin_read_keys_truncate: Do not crash if PERSISTENT RESERVE IN fails Do not try to dereference the 'rk' pointer if it is NULL. Signed-off-by: Bart Van Assche --- test-tool/test_prin_read_keys_truncate.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/test-tool/test_prin_read_keys_truncate.c b/test-tool/test_prin_read_keys_truncate.c index 1590b33..976269c 100644 --- a/test-tool/test_prin_read_keys_truncate.c +++ b/test-tool/test_prin_read_keys_truncate.c @@ -58,15 +58,17 @@ test_prin_read_keys_truncate(void) } CU_ASSERT_EQUAL(ret, 0); - /* - * SPC5r17: 6.16.2 READ KEYS service action - * The ADDITIONAL LENGTH field indicates the number of bytes in - * the Reservation key list. The contents of the ADDITIONAL - * LENGTH field are not altered based on the allocation length. - */ - CU_ASSERT_NOT_EQUAL(rk->additional_length, 0); - /* key array should have been truncated in the response */ - CU_ASSERT_EQUAL(rk->num_keys, 0); + if (rk) { + /* + * SPC5r17: 6.16.2 READ KEYS service action + * The ADDITIONAL LENGTH field indicates the number of bytes in + * the Reservation key list. The contents of the ADDITIONAL + * LENGTH field are not altered based on the allocation length. + */ + CU_ASSERT_NOT_EQUAL(rk->additional_length, 0); + /* key array should have been truncated in the response */ + CU_ASSERT_EQUAL(rk->num_keys, 0); + } /* remove our key from the target */ ret = prout_register_key(sd, 0, key); From 663aad13bab114038a90d72f54d3d92dd16d49df Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 22 Oct 2018 16:31:55 -0700 Subject: [PATCH 2/4] Avoid that iscsi_reconnect() crashes In the else branch, set the tmp_iscsi->old_iscsi pointer instead of the iscsi->old_iscsi pointer. Signed-off-by: Bart Van Assche --- lib/connect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connect.c b/lib/connect.c index f11d845..54cb8c8 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -457,7 +457,7 @@ int iscsi_reconnect(struct iscsi_context *iscsi) } tmp_iscsi->old_iscsi = iscsi->old_iscsi; } else { - iscsi->old_iscsi = malloc(sizeof(struct iscsi_context)); + tmp_iscsi->old_iscsi = malloc(sizeof(struct iscsi_context)); memcpy(tmp_iscsi->old_iscsi, iscsi, sizeof(struct iscsi_context)); } memcpy(iscsi, tmp_iscsi, sizeof(struct iscsi_context)); From 1bd1232a2ad5339ffa5549e3ded6d5fef9f35465 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 13 Jan 2019 11:50:43 -0800 Subject: [PATCH 3/4] Move the clear_pr() function definition This patch does not change any functionality. Signed-off-by: Bart Van Assche --- test-tool/iscsi-test-cu.c | 54 +++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/test-tool/iscsi-test-cu.c b/test-tool/iscsi-test-cu.c index 76a16e0..6faddf9 100644 --- a/test-tool/iscsi-test-cu.c +++ b/test-tool/iscsi-test-cu.c @@ -770,6 +770,33 @@ print_usage(void) fprintf(stderr, "\n"); } +/* Clear persistent reservations and reservation keys left by a previous run */ +static int clear_pr(struct scsi_device *sdev) +{ + int i, res; + struct scsi_task *pr_task; + struct scsi_persistent_reserve_in_read_keys *rk; + + res = 0; + if (prin_read_keys(sdev, &pr_task, &rk, 16384) != 0) + goto out; + + res = -1; + if (rk->num_keys && data_loss == 0) + goto out; + + res = 0; + for (i = 0; i < rk->num_keys; i++) { + prout_register_and_ignore(sdev, rk->keys[i]); + prout_register_key(sdev, 0, rk->keys[i]); + } + + scsi_free_scsi_task(pr_task); + +out: + return res; +} + void test_setup(void) { @@ -1062,33 +1089,6 @@ static void free_scsi_device(struct scsi_device *sdev) free(sdev); } -/* Clear persistent reservations and reservation keys left by a previous run */ -static int clear_pr(struct scsi_device *sdev) -{ - int i, res; - struct scsi_task *pr_task; - struct scsi_persistent_reserve_in_read_keys *rk; - - res = 0; - if (prin_read_keys(sdev, &pr_task, &rk, 16384) != 0) - goto out; - - res = -1; - if (rk->num_keys && data_loss == 0) - goto out; - - res = 0; - for (i = 0; i < rk->num_keys; i++) { - prout_register_and_ignore(sdev, rk->keys[i]); - prout_register_key(sdev, 0, rk->keys[i]); - } - - scsi_free_scsi_task(pr_task); - -out: - return res; -} - int main(int argc, char *argv[]) { From 16435a917d671c3fca4ef4a3372eaf8db4492e2b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 13 Jan 2019 11:50:46 -0800 Subject: [PATCH 4/4] iscsi-test-cu: Improve persistent reservation clearing Clear persistent reservations before sending the first SCSI command to the target to avoid that that command fails due to persistent reservations left behind by a previous run of the test tool. Avoid that a subsequent test fails if a test did not remove the persistent reservations it obtained. Make sure that clear_pr() returns -1 if clearing persistent reservations fails. Signed-off-by: Bart Van Assche --- test-tool/iscsi-test-cu.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test-tool/iscsi-test-cu.c b/test-tool/iscsi-test-cu.c index 6faddf9..bb74990 100644 --- a/test-tool/iscsi-test-cu.c +++ b/test-tool/iscsi-test-cu.c @@ -787,8 +787,12 @@ static int clear_pr(struct scsi_device *sdev) res = 0; for (i = 0; i < rk->num_keys; i++) { - prout_register_and_ignore(sdev, rk->keys[i]); - prout_register_key(sdev, 0, rk->keys[i]); + res = prout_register_and_ignore(sdev, rk->keys[i]); + if (res) + break; + res = prout_register_key(sdev, 0, rk->keys[i]); + if (res) + break; } scsi_free_scsi_task(pr_task); @@ -857,6 +861,7 @@ suite_cleanup(void) for (i = 0; i < mp_num_sds; i++) { if (mp_sds[i]->iscsi_url) { if (mp_sds[i]->iscsi_ctx) { + clear_pr(mp_sds[i]); iscsi_logout_sync(mp_sds[i]->iscsi_ctx); iscsi_destroy_context(mp_sds[i]->iscsi_ctx); mp_sds[i]->iscsi_ctx = NULL; @@ -1241,6 +1246,10 @@ main(int argc, char *argv[]) "Failed to connect to SCSI device %d\n", i); goto err_sds_free; } + if (clear_pr(mp_sds[i]) < 0) { + printf("One or more persistent reservations keys have been registered\n"); + return -1; + } } if (mp_num_sds > 1) { @@ -1439,11 +1448,6 @@ main(int argc, char *argv[]) } scsi_free_scsi_task(task); - if (clear_pr(sd) < 0) { - printf("One or more persistent reservations keys have been registered\n"); - return -1; - } - /* BLKSECTGET for /dev/sg* is a shitshow under linux. * Even 4.2 kernels return number of bytes instead of number * of sectors here. Just force it to 120k and let us get on with