From 8aa6d9de8f6de3af0d7fa14950daa2b69041dbc0 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Mon, 23 Sep 2013 21:58:55 -0700 Subject: [PATCH 01/17] TESTS: Add a test for COMPARE_AND_WRITE when the data to verify does not match --- Makefile.am | 1 + include/scsi-lowlevel.h | 1 + lib/scsi-lowlevel.c | 2 + test-tool/iscsi-support.c | 52 ++++++++ test-tool/iscsi-support.h | 1 + test-tool/iscsi-test-cu.c | 1 + test-tool/iscsi-test-cu.h | 1 + test-tool/test_compareandwrite_miscompare.c | 136 ++++++++++++++++++++ 8 files changed, 195 insertions(+) create mode 100644 test-tool/test_compareandwrite_miscompare.c diff --git a/Makefile.am b/Makefile.am index 9ee2a38..806262b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -194,6 +194,7 @@ bin_iscsi_test_cu_LDFLAGS = -ldl -lcunit bin_iscsi_test_cu_SOURCES = test-tool/iscsi-test-cu.c \ test-tool/iscsi-support.c \ test-tool/test_compareandwrite_simple.c \ + test-tool/test_compareandwrite_miscompare.c \ test-tool/test_get_lba_status_simple.c \ test-tool/test_get_lba_status_beyond_eol.c \ test-tool/test_inquiry_alloc_length.c \ diff --git a/include/scsi-lowlevel.h b/include/scsi-lowlevel.h index b9a0deb..44bf80a 100644 --- a/include/scsi-lowlevel.h +++ b/include/scsi-lowlevel.h @@ -152,6 +152,7 @@ EXTERN const char *scsi_sense_key_str(int key); #define SCSI_SENSE_ASCQ_WRITE_AFTER_SANITIZE_REQUIRED 0x1115 #define SCSI_SENSE_ASCQ_PARAMETER_LIST_LENGTH_ERROR 0x1a00 #define SCSI_SENSE_ASCQ_MISCOMPARE_DURING_VERIFY 0x1d00 +#define SCSI_SENSE_ASCQ_MISCOMPARE_VERIFY_OF_UNMAPPED_LBA 0x1d01 #define SCSI_SENSE_ASCQ_INVALID_OPERATION_CODE 0x2000 #define SCSI_SENSE_ASCQ_LBA_OUT_OF_RANGE 0x2100 #define SCSI_SENSE_ASCQ_INVALID_FIELD_IN_CDB 0x2400 diff --git a/lib/scsi-lowlevel.c b/lib/scsi-lowlevel.c index fd96157..7257d9d 100644 --- a/lib/scsi-lowlevel.c +++ b/lib/scsi-lowlevel.c @@ -215,6 +215,8 @@ scsi_sense_ascq_str(int ascq) "INTERNAL_TARGET_FAILURE"}, {SCSI_SENSE_ASCQ_MISCOMPARE_DURING_VERIFY, "MISCOMPARE_DURING_VERIFY"}, + {SCSI_SENSE_ASCQ_MISCOMPARE_VERIFY_OF_UNMAPPED_LBA, + "MISCOMPARE_VERIFY_OF_UNMAPPED_LBA"}, { SCSI_SENSE_ASCQ_MEDIUM_LOAD_OR_EJECT_FAILED, "MEDIUM_LOAD_OR_EJECT_FAILED" }, {SCSI_SENSE_ASCQ_MEDIUM_REMOVAL_PREVENTED, diff --git a/test-tool/iscsi-support.c b/test-tool/iscsi-support.c index bcb1a9e..3cccbe8 100644 --- a/test-tool/iscsi-support.c +++ b/test-tool/iscsi-support.c @@ -1757,6 +1757,58 @@ int compareandwrite(struct iscsi_context *iscsi, int lun, uint64_t lba, return 0; } +int compareandwrite_miscompare(struct iscsi_context *iscsi, int lun, + uint64_t lba, unsigned char *data, + uint32_t len, int blocksize, + int wrprotect, int dpo, + int fua, int group_number) +{ + struct scsi_task *task; + + logging(LOG_VERBOSE, "Send COMPARE_AND_WRITE LBA:%" PRIu64 + " LEN:%d WRPROTECT:%d (expecting MISCOMPARE)", + lba, len, wrprotect); + + task = iscsi_compareandwrite_sync(iscsi, lun, lba, + data, len, blocksize, + wrprotect, dpo, fua, 0, group_number); + if (task == NULL) { + logging(LOG_NORMAL, "[FAILED] Failed to send COMPARE_AND_WRITE " + "command: %s", + iscsi_get_error(iscsi)); + return -1; + } + if (task->status == SCSI_STATUS_CHECK_CONDITION + && task->sense.key == SCSI_SENSE_ILLEGAL_REQUEST + && task->sense.ascq == SCSI_SENSE_ASCQ_INVALID_OPERATION_CODE) { + logging(LOG_NORMAL, "[SKIPPED] COMPARE_AND_WRITE is not " + "implemented on target"); + scsi_free_scsi_task(task); + return -2; + } + if (task->status == SCSI_STATUS_GOOD) { + logging(LOG_NORMAL, "[FAILED] COMPARE_AND_WRITE successful " + "but should have failed with MISCOMPARE."); + scsi_free_scsi_task(task); + return -1; + } + + if (task->status != SCSI_STATUS_CHECK_CONDITION + || task->sense.key != SCSI_SENSE_MISCOMPARE + || task->sense.ascq != SCSI_SENSE_ASCQ_MISCOMPARE_DURING_VERIFY) { + logging(LOG_NORMAL, "[FAILED] COMPARE_AND_WRITE failed with " + "the wrong sense code. Should have failed with " + "MISCOMPARE/MISCOMPARE_DURING_VERIFY but failed with " + "sense:%s", iscsi_get_error(iscsi)); + scsi_free_scsi_task(task); + return -1; + } + + scsi_free_scsi_task(task); + logging(LOG_VERBOSE, "[OK] COMPARE_AND_WRITE returned MISCOMPARE."); + return 0; +} + struct scsi_task *get_lba_status_task(struct iscsi_context *iscsi, int lun, uint64_t lba, uint32_t len) { struct scsi_task *task; diff --git a/test-tool/iscsi-support.h b/test-tool/iscsi-support.h index f64f89b..ec88220 100644 --- a/test-tool/iscsi-support.h +++ b/test-tool/iscsi-support.h @@ -227,6 +227,7 @@ int inquiry(struct iscsi_context *iscsi, int lun, int evpd, int page_code, int m int inquiry_invalidfieldincdb(struct iscsi_context *iscsi, int lun, int evpd, int page_code, int maxsize); struct scsi_task *get_lba_status_task(struct iscsi_context *iscsi, int lun, uint64_t lba, uint32_t len); int compareandwrite(struct iscsi_context *iscsi, int lun, uint64_t lba, unsigned char *data, uint32_t len, int blocksize, int wrprotect, int dpo, int fua, int group_number); +int compareandwrite_miscompare(struct iscsi_context *iscsi, int lun, uint64_t lba, unsigned char *data, uint32_t len, int blocksize, int wrprotect, int dpo, int fua, int group_number); int get_lba_status(struct iscsi_context *iscsi, int lun, uint64_t lba, uint32_t len); int get_lba_status_lbaoutofrange(struct iscsi_context *iscsi, int lun, uint64_t lba, uint32_t len); int get_lba_status_nomedium(struct iscsi_context *iscsi, int lun, uint64_t lba, uint32_t len); diff --git a/test-tool/iscsi-test-cu.c b/test-tool/iscsi-test-cu.c index c94d7b5..84298ad 100644 --- a/test-tool/iscsi-test-cu.c +++ b/test-tool/iscsi-test-cu.c @@ -62,6 +62,7 @@ int (*real_iscsi_queue_pdu)(struct iscsi_context *iscsi, struct iscsi_pdu *pdu); *****************************************************************/ static CU_TestInfo tests_compareandwrite[] = { { (char *)"Simple", test_compareandwrite_simple }, + { (char *)"Miscompare", test_compareandwrite_miscompare }, CU_TEST_INFO_NULL }; diff --git a/test-tool/iscsi-test-cu.h b/test-tool/iscsi-test-cu.h index 1745b9b..8d93c76 100644 --- a/test-tool/iscsi-test-cu.h +++ b/test-tool/iscsi-test-cu.h @@ -41,6 +41,7 @@ int test_setup_pgr(void); int test_teardown_pgr(void); void test_compareandwrite_simple(void); +void test_compareandwrite_miscompare(void); void test_get_lba_status_simple(void); void test_get_lba_status_beyond_eol(void); diff --git a/test-tool/test_compareandwrite_miscompare.c b/test-tool/test_compareandwrite_miscompare.c new file mode 100644 index 0000000..f73ee56 --- /dev/null +++ b/test-tool/test_compareandwrite_miscompare.c @@ -0,0 +1,136 @@ +/* + Copyright (C) 2013 Ronnie Sahlberg + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, see . +*/ + +#include +#include +#include + +#include + +#include "iscsi.h" +#include "scsi-lowlevel.h" +#include "iscsi-support.h" +#include "iscsi-test-cu.h" + + +void +test_compareandwrite_miscompare(void) +{ + int i, ret; + unsigned j; + unsigned char *buf = alloca(2 * 256 * block_size); + + CHECK_FOR_DATALOSS; + CHECK_FOR_SBC; + + logging(LOG_VERBOSE, LOG_BLANK_LINE); + logging(LOG_VERBOSE, "Test COMPARE_AND_WRITE of 1-256 blocks at the " + "start of the LUN. One Byte miscompare in the final block."); + for (i = 1; i < 256; i++) { + logging(LOG_VERBOSE, "Write %d blocks of 'A' at LBA:0", i); + memset(buf, 'A', 2 * i * block_size); + if (maximum_transfer_length && maximum_transfer_length < i) { + break; + } + ret = write16(iscsic, tgt_lun, 0, i * block_size, + block_size, 0, 0, 0, 0, 0, buf); + if (ret == -2) { + logging(LOG_NORMAL, "[SKIPPED] WRITE16 is not implemented."); + CU_PASS("WRITE16 is not implemented."); + return; + } + CU_ASSERT_EQUAL(ret, 0); + + + logging(LOG_VERBOSE, "Change byte 27 from the end to 'C' so that it does not match."); + buf[i * block_size - 27] = 'C'; + + + memset(buf + i * block_size, 'B', i * block_size); + + logging(LOG_VERBOSE, "Overwrite %d blocks with 'B' " + "at LBA:0 (if they all contain 'A')", i); + ret = compareandwrite_miscompare(iscsic, tgt_lun, 0, + buf, 2 * i * block_size, block_size, 0, 0, 0, 0); + if (ret == -2) { + CU_PASS("[SKIPPED] Target does not support " + "COMPARE_AND_WRITE. Skipping test"); + return; + } + CU_ASSERT_EQUAL(ret, 0); + + logging(LOG_VERBOSE, "Read %d blocks at LBA:0 and verify " + "they are still unchanged as 'A'", i); + ret = read16(iscsic, tgt_lun, 0, i * block_size, + block_size, 0, 0, 0, 0, 0, buf); + CU_ASSERT_EQUAL(ret, 0); + + for (j = 0; j < i * block_size; j++) { + if (buf[j] != 'A') { + logging(LOG_VERBOSE, "[FAILED] Data changed " + "eventhough there was a miscompare"); + CU_FAIL("Block was written to"); + return; + } + } + } + + + logging(LOG_VERBOSE, "Test COMPARE_AND_WRITE of 1-256 blocks at the " + "end of the LUN"); + for (i = 1; i < 256; i++) { + logging(LOG_VERBOSE, "Write %d blocks of 'A' at LBA:%" PRIu64, + i, num_blocks - i); + memset(buf, 'A', 2 * i * block_size); + if (maximum_transfer_length && maximum_transfer_length < i) { + break; + } + ret = write16(iscsic, tgt_lun, num_blocks - i, i * block_size, + block_size, 0, 0, 0, 0, 0, buf); + CU_ASSERT_EQUAL(ret, 0); + + logging(LOG_VERBOSE, "Change byte 27 from the end to 'C' so that it does not match."); + buf[i * block_size - 27] = 'C'; + + + memset(buf + i * block_size, 'B', i * block_size); + + logging(LOG_VERBOSE, "Overwrite %d blocks with 'B' " + "at LBA:%" PRIu64 " (if they all contain 'A')", + i, num_blocks - i); + ret = compareandwrite_miscompare(iscsic, tgt_lun, + num_blocks - i, + buf, 2 * i * block_size, block_size, 0, 0, 0, 0); + CU_ASSERT_EQUAL(ret, 0); + + logging(LOG_VERBOSE, "Read %d blocks at LBA:%" PRIu64 + "they are still unchanged as 'A'", + i, num_blocks - i); + ret = read16(iscsic, tgt_lun, num_blocks - i, i * block_size, + block_size, 0, 0, 0, 0, 0, buf); + CU_ASSERT_EQUAL(ret, 0); + + for (j = 0; j < i * block_size; j++) { + if (buf[j] != 'A') { + logging(LOG_VERBOSE, "[FAILED] Data changed " + "eventhough there was a miscompare"); + CU_FAIL("Block was written to"); + return; + } + } + } +} From 69867d23a8183b33022368121cb00c90e81f8544 Mon Sep 17 00:00:00 2001 From: Sitsofe Wheeler Date: Tue, 3 Sep 2013 12:44:28 +0100 Subject: [PATCH 02/17] Fix incorrect second initiator Use the second initiator name in all files found by ack "iscsic2 = " --- test-tool/test_preventallow_2_itnexuses.c | 2 +- test-tool/test_reserve6_itnexus_loss.c | 2 +- test-tool/test_reserve6_logout.c | 2 +- test-tool/test_reserve6_lun_reset.c | 2 +- test-tool/test_reserve6_target_cold_reset.c | 2 +- test-tool/test_reserve6_target_warm_reset.c | 2 +- test-tool/test_sanitize_readonly.c | 2 +- test-tool/test_sanitize_reservations.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test-tool/test_preventallow_2_itnexuses.c b/test-tool/test_preventallow_2_itnexuses.c index b1511d5..49af325 100644 --- a/test-tool/test_preventallow_2_itnexuses.c +++ b/test-tool/test_preventallow_2_itnexuses.c @@ -48,7 +48,7 @@ test_preventallow_2_itnexuses(void) CU_ASSERT_EQUAL(ret, 0); logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; diff --git a/test-tool/test_reserve6_itnexus_loss.c b/test-tool/test_reserve6_itnexus_loss.c index 2c8e10b..8894c39 100644 --- a/test-tool/test_reserve6_itnexus_loss.c +++ b/test-tool/test_reserve6_itnexus_loss.c @@ -46,7 +46,7 @@ test_reserve6_itnexus_loss(void) logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; diff --git a/test-tool/test_reserve6_logout.c b/test-tool/test_reserve6_logout.c index eb7dd99..3b0ccf6 100644 --- a/test-tool/test_reserve6_logout.c +++ b/test-tool/test_reserve6_logout.c @@ -46,7 +46,7 @@ test_reserve6_logout(void) logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; diff --git a/test-tool/test_reserve6_lun_reset.c b/test-tool/test_reserve6_lun_reset.c index 980464c..9853e49 100644 --- a/test-tool/test_reserve6_lun_reset.c +++ b/test-tool/test_reserve6_lun_reset.c @@ -57,7 +57,7 @@ test_reserve6_lun_reset(void) logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; diff --git a/test-tool/test_reserve6_target_cold_reset.c b/test-tool/test_reserve6_target_cold_reset.c index 4e53cf7..00a847e 100644 --- a/test-tool/test_reserve6_target_cold_reset.c +++ b/test-tool/test_reserve6_target_cold_reset.c @@ -56,7 +56,7 @@ test_reserve6_target_cold_reset(void) sleep(3); logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; diff --git a/test-tool/test_reserve6_target_warm_reset.c b/test-tool/test_reserve6_target_warm_reset.c index 283ae39..41ed515 100644 --- a/test-tool/test_reserve6_target_warm_reset.c +++ b/test-tool/test_reserve6_target_warm_reset.c @@ -57,7 +57,7 @@ test_reserve6_target_warm_reset(void) logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; diff --git a/test-tool/test_sanitize_readonly.c b/test-tool/test_sanitize_readonly.c index b34bec7..b0e7a4b 100644 --- a/test-tool/test_sanitize_readonly.c +++ b/test-tool/test_sanitize_readonly.c @@ -40,7 +40,7 @@ test_sanitize_readonly(void) CHECK_FOR_DATALOSS; logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; diff --git a/test-tool/test_sanitize_reservations.c b/test-tool/test_sanitize_reservations.c index 409374f..05e1919 100644 --- a/test-tool/test_sanitize_reservations.c +++ b/test-tool/test_sanitize_reservations.c @@ -40,7 +40,7 @@ test_sanitize_reservations(void) CHECK_FOR_DATALOSS; logging(LOG_VERBOSE, "Create a second connection to the target"); - iscsic2 = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); + iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun); if (iscsic2 == NULL) { logging(LOG_VERBOSE, "Failed to login to target"); return; From 286df502446f45fd2415eb3bb2cb0904b2644680 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Wed, 25 Sep 2013 20:26:00 -0700 Subject: [PATCH 03/17] Add more status codes : CONDITIONS_MET/TASK_SET_FULL/ACA_ACTIVE/TASK_ABORTED --- include/iscsi.h | 4 ++++ lib/iscsi-command.c | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/include/iscsi.h b/include/iscsi.h index 0fa7bea..0f0f07a 100644 --- a/include/iscsi.h +++ b/include/iscsi.h @@ -275,8 +275,12 @@ EXTERN int iscsi_is_logged_in(struct iscsi_context *iscsi); enum scsi_status { SCSI_STATUS_GOOD = 0, SCSI_STATUS_CHECK_CONDITION = 2, + SCSI_STATUS_CONDITION_MET = 4, SCSI_STATUS_BUSY = 8, SCSI_STATUS_RESERVATION_CONFLICT = 0x18, + SCSI_STATUS_TASK_SET_FULL = 0x28, + SCSI_STATUS_ACA_ACTIVE = 0x30, + SCSI_STATUS_TASK_ABORTED = 0x40, SCSI_STATUS_REDIRECT = 0x101, SCSI_STATUS_CANCELLED = 0x0f000000, SCSI_STATUS_ERROR = 0x0f000001, diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index f4d12d5..90d3fdb 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -50,6 +50,10 @@ iscsi_scsi_response_cb(struct iscsi_context *iscsi, int status, case SCSI_STATUS_CHECK_CONDITION: case SCSI_STATUS_GOOD: case SCSI_STATUS_BUSY: + case SCSI_STATUS_CONDITION_MET: + case SCSI_STATUS_TASK_SET_FULL: + case SCSI_STATUS_ACA_ACTIVE: + case SCSI_STATUS_TASK_ABORTED: case SCSI_STATUS_ERROR: case SCSI_STATUS_CANCELLED: scsi_cbdata->callback(iscsi, status, scsi_cbdata->task, @@ -381,6 +385,7 @@ iscsi_process_scsi_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, switch (status) { case SCSI_STATUS_GOOD: + case SCSI_STATUS_CONDITION_MET: task->datain.data = pdu->indata.data; task->datain.size = pdu->indata.size; @@ -448,6 +453,21 @@ iscsi_process_scsi_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, pdu->callback(iscsi, SCSI_STATUS_RESERVATION_CONFLICT, task, pdu->private_data); break; + case SCSI_STATUS_TASK_SET_FULL: + iscsi_set_error(iscsi, "TASK_SET_FULL"); + pdu->callback(iscsi, SCSI_STATUS_TASK_SET_FULL, + task, pdu->private_data); + break; + case SCSI_STATUS_ACA_ACTIVE: + iscsi_set_error(iscsi, "ACA_ACTIVE"); + pdu->callback(iscsi, SCSI_STATUS_ACA_ACTIVE, + task, pdu->private_data); + break; + case SCSI_STATUS_TASK_ABORTED: + iscsi_set_error(iscsi, "TASK_ABORTED"); + pdu->callback(iscsi, SCSI_STATUS_TASK_ABORTED, + task, pdu->private_data); + break; case SCSI_STATUS_BUSY: iscsi_set_error(iscsi, "BUSY"); pdu->callback(iscsi, SCSI_STATUS_BUSY, From 766d92221cd9d09be94824c1c5e3bd4e17ea0d47 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Thu, 26 Sep 2013 18:26:29 -0700 Subject: [PATCH 04/17] Reconnect: recalculate the header digest when we re-queue after a reconnect --- lib/connect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connect.c b/lib/connect.c index 09a0f9e..ee3a0ef 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -345,7 +345,7 @@ try_again: pdu->outdata_written = 0; pdu->payload_written = 0; - iscsi_add_to_outqueue(iscsi, pdu); + iscsi_queue_pdu(iscsi, pdu); } if (dup2(iscsi->fd, old_iscsi->fd) == -1) { From bd948c959e5fd5d62d774abcd7d414b799662453 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 28 Sep 2013 21:04:51 +0200 Subject: [PATCH 05/17] Port to CUnit version 2 Linux distributions like openSUSE 12.3 include CUnit version 2. Apparently libiscsi uses some CUnit version 1 data structures that have been modified in CUnit version 2. This causes the libiscsi build to fail against CUnit version 2. Fix this by detecting the CUnit version during the configure step. Signed-off-by: Bart Van Assche --- configure.ac | 9 + test-tool/iscsi-test-cu.c | 341 +++++++++++++++++++------------------- test-tool/iscsi-test-cu.h | 18 +- 3 files changed, 197 insertions(+), 171 deletions(-) diff --git a/configure.ac b/configure.ac index 74396bc..71b835d 100644 --- a/configure.ac +++ b/configure.ac @@ -111,5 +111,14 @@ fi AM_CONDITIONAL(ISCSITEST, [test "$ac_cv_have_cunit" = yes]) +AC_CHECK_MEMBER([struct CU_SuiteInfo.pSetUpFunc], + [AC_DEFINE([HAVE_CU_SUITEINFO_PSETUPFUNC], 1, + [Define to 1 if struct CU_SuiteInfo has a member called pSetUpFunc])], + [], [[ +#include +#include +]]) + + AC_CONFIG_FILES(Makefile libiscsi.pc) AC_OUTPUT diff --git a/test-tool/iscsi-test-cu.c b/test-tool/iscsi-test-cu.c index 84298ad..75d2189 100644 --- a/test-tool/iscsi-test-cu.c +++ b/test-tool/iscsi-test-cu.c @@ -396,86 +396,92 @@ static CU_TestInfo tests_writeverify16[] = { CU_TEST_INFO_NULL }; +typedef struct libiscsi_suite_info { + const char *pName; /**< Suite name. */ + CU_InitializeFunc pInitFunc; /**< Suite initialization function. */ + CU_CleanupFunc pCleanupFunc; /**< Suite cleanup function */ + CU_TestInfo *pTests; /**< Test case array - must be NULL terminated. */ +} libiscsi_suite_info; /* SCSI protocol tests */ -static CU_SuiteInfo scsi_suites[] = { - { (char *)"CompareAndWrite", test_setup, test_teardown, +static libiscsi_suite_info scsi_suites[] = { + { "CompareAndWrite", test_setup, test_teardown, tests_compareandwrite }, - { (char *)"GetLBAStatus", test_setup, test_teardown, + { "GetLBAStatus", test_setup, test_teardown, tests_get_lba_status }, - { (char *)"Inquiry", test_setup, test_teardown, + { "Inquiry", test_setup, test_teardown, tests_inquiry }, - { (char *)"Mandatory", test_setup, test_teardown, + { "Mandatory", test_setup, test_teardown, tests_mandatory }, - { (char *)"ModeSense6", test_setup, test_teardown, + { "ModeSense6", test_setup, test_teardown, tests_modesense6 }, - { (char *)"NoMedia", test_setup, test_teardown, + { "NoMedia", test_setup, test_teardown, tests_nomedia }, - { (char *)"OrWrite", test_setup, test_teardown, + { "OrWrite", test_setup, test_teardown, tests_orwrite }, - { (char *)"Prefetch10", test_setup, test_teardown, + { "Prefetch10", test_setup, test_teardown, tests_prefetch10 }, - { (char *)"Prefetch16", test_setup, test_teardown, + { "Prefetch16", test_setup, test_teardown, tests_prefetch16 }, - { (char *)"PreventAllow", test_setup, test_teardown, + { "PreventAllow", test_setup, test_teardown, tests_preventallow }, - { (char *)"PrinReadKeys", test_setup, test_teardown, + { "PrinReadKeys", test_setup, test_teardown, tests_prin_read_keys }, - { (char *)"PrinServiceactionRange", test_setup, test_teardown, + { "PrinServiceactionRange", test_setup, test_teardown, tests_prin_serviceaction_range }, - { (char *)"ProutRegister", test_setup, test_teardown, + { "ProutRegister", test_setup, test_teardown, tests_prout_register }, - { (char *)"ProutReserve", test_setup_pgr, test_teardown_pgr, + { "ProutReserve", test_setup_pgr, test_teardown_pgr, tests_prout_reserve }, - { (char *)"Read6", test_setup, test_teardown, + { "Read6", test_setup, test_teardown, tests_read6 }, - { (char *)"Read10", test_setup, test_teardown, + { "Read10", test_setup, test_teardown, tests_read10 }, - { (char *)"Read12", test_setup, test_teardown, + { "Read12", test_setup, test_teardown, tests_read12 }, - { (char *)"Read16", test_setup, test_teardown, + { "Read16", test_setup, test_teardown, tests_read16 }, - { (char *)"ReadCapacity10", test_setup, test_teardown, + { "ReadCapacity10", test_setup, test_teardown, tests_readcapacity10 }, - { (char *)"ReadCapacity16", test_setup, test_teardown, + { "ReadCapacity16", test_setup, test_teardown, tests_readcapacity16 }, - { (char *)"ReadOnly", test_setup, test_teardown, + { "ReadOnly", test_setup, test_teardown, tests_readonly }, - { (char *)"ReportSupportedOpcodes", test_setup, test_teardown, + { "ReportSupportedOpcodes", test_setup, test_teardown, tests_report_supported_opcodes }, - { (char *)"Reserve6", test_setup, test_teardown, + { "Reserve6", test_setup, test_teardown, tests_reserve6 }, - { (char *)"Sanitize", test_setup, test_teardown, + { "Sanitize", test_setup, test_teardown, tests_sanitize }, - { (char *)"StartStopUnit", test_setup, test_teardown, + { "StartStopUnit", test_setup, test_teardown, tests_startstopunit }, - { (char *)"UnitReady", test_setup, test_teardown, + { "UnitReady", test_setup, test_teardown, tests_testunitready }, - { (char *)"Unmap", test_setup, test_teardown, + { "Unmap", test_setup, test_teardown, tests_unmap }, - { (char *)"Verify10", test_setup, test_teardown, + { "Verify10", test_setup, test_teardown, tests_verify10 }, - { (char *)"Verify12", test_setup, test_teardown, + { "Verify12", test_setup, test_teardown, tests_verify12 }, - { (char *)"Verify16", test_setup, test_teardown, + { "Verify16", test_setup, test_teardown, tests_verify16 }, - { (char *)"Write10", test_setup, test_teardown, + { "Write10", test_setup, test_teardown, tests_write10 }, - { (char *)"Write12", test_setup, test_teardown, + { "Write12", test_setup, test_teardown, tests_write12 }, - { (char *)"Write16", test_setup, test_teardown, + { "Write16", test_setup, test_teardown, tests_write16 }, - { (char *)"WriteSame10", test_setup, test_teardown, + { "WriteSame10", test_setup, test_teardown, tests_writesame10 }, - { (char *)"WriteSame16", test_setup, test_teardown, + { "WriteSame16", test_setup, test_teardown, tests_writesame16 }, - { (char *)"WriteVerify10", test_setup, test_teardown, + { "WriteVerify10", test_setup, test_teardown, tests_writeverify10 }, - { (char *)"WriteVerify12", test_setup, test_teardown, + { "WriteVerify12", test_setup, test_teardown, tests_writeverify12 }, - { (char *)"WriteVerify16", test_setup, test_teardown, + { "WriteVerify16", test_setup, test_teardown, tests_writeverify16 }, - CU_SUITE_INFO_NULL + { NULL, NULL, NULL, NULL } }; static CU_TestInfo tests_iscsi_cmdsn[] = { @@ -499,174 +505,174 @@ static CU_TestInfo tests_iscsi_residuals[] = { }; /* iSCSI protocol tests */ -static CU_SuiteInfo iscsi_suites[] = { - { (char *)"iSCSIcmdsn", test_setup, test_teardown, +static libiscsi_suite_info iscsi_suites[] = { + { "iSCSIcmdsn", test_setup, test_teardown, tests_iscsi_cmdsn }, - { (char *)"iSCSIResiduals", test_setup, test_teardown, + { "iSCSIResiduals", test_setup, test_teardown, tests_iscsi_residuals }, - CU_SUITE_INFO_NULL + { NULL, NULL, NULL, NULL } }; /* All tests */ -static CU_SuiteInfo all_suites[] = { - { (char *)"CompareAndWrite", test_setup, test_teardown, +static libiscsi_suite_info all_suites[] = { + { "CompareAndWrite", test_setup, test_teardown, tests_compareandwrite }, - { (char *)"GetLBAStatus", test_setup, test_teardown, + { "GetLBAStatus", test_setup, test_teardown, tests_get_lba_status }, - { (char *)"Inquiry", test_setup, test_teardown, + { "Inquiry", test_setup, test_teardown, tests_inquiry }, - { (char *)"Mandatory", test_setup, test_teardown, + { "Mandatory", test_setup, test_teardown, tests_mandatory }, - { (char *)"ModeSense6", test_setup, test_teardown, + { "ModeSense6", test_setup, test_teardown, tests_modesense6 }, - { (char *)"NoMedia", test_setup, test_teardown, + { "NoMedia", test_setup, test_teardown, tests_nomedia }, - { (char *)"OrWrite", test_setup, test_teardown, + { "OrWrite", test_setup, test_teardown, tests_orwrite }, - { (char *)"Prefetch10", test_setup, test_teardown, + { "Prefetch10", test_setup, test_teardown, tests_prefetch10 }, - { (char *)"Prefetch16", test_setup, test_teardown, + { "Prefetch16", test_setup, test_teardown, tests_prefetch16 }, - { (char *)"PreventAllow", test_setup, test_teardown, + { "PreventAllow", test_setup, test_teardown, tests_preventallow }, - { (char *)"PrinReadKeys", test_setup, test_teardown, + { "PrinReadKeys", test_setup, test_teardown, tests_prin_read_keys }, - { (char *)"PrinServiceactionRange", test_setup, test_teardown, + { "PrinServiceactionRange", test_setup, test_teardown, tests_prin_serviceaction_range }, - { (char *)"ProutRegister", test_setup, test_teardown, + { "ProutRegister", test_setup, test_teardown, tests_prout_register }, - { (char *)"ProutReserve", test_setup_pgr, test_teardown_pgr, + { "ProutReserve", test_setup_pgr, test_teardown_pgr, tests_prout_reserve }, - { (char *)"Read6", test_setup, test_teardown, + { "Read6", test_setup, test_teardown, tests_read6 }, - { (char *)"Read10", test_setup, test_teardown, + { "Read10", test_setup, test_teardown, tests_read10 }, - { (char *)"Read12", test_setup, test_teardown, + { "Read12", test_setup, test_teardown, tests_read12 }, - { (char *)"Read16", test_setup, test_teardown, + { "Read16", test_setup, test_teardown, tests_read16 }, - { (char *)"ReadCapacity10", test_setup, test_teardown, + { "ReadCapacity10", test_setup, test_teardown, tests_readcapacity10 }, - { (char *)"ReadCapacity16", test_setup, test_teardown, + { "ReadCapacity16", test_setup, test_teardown, tests_readcapacity16 }, - { (char *)"ReadOnly", test_setup, test_teardown, + { "ReadOnly", test_setup, test_teardown, tests_readonly }, - { (char *)"ReportSupportedOpcodes", test_setup, test_teardown, + { "ReportSupportedOpcodes", test_setup, test_teardown, tests_report_supported_opcodes }, - { (char *)"Reserve6", test_setup, test_teardown, + { "Reserve6", test_setup, test_teardown, tests_reserve6 }, - { (char *)"Sanitize", test_setup, test_teardown, + { "Sanitize", test_setup, test_teardown, tests_sanitize }, - { (char *)"StartStopUnit", test_setup, test_teardown, + { "StartStopUnit", test_setup, test_teardown, tests_startstopunit }, - { (char *)"TestUnitReady", test_setup, test_teardown, + { "TestUnitReady", test_setup, test_teardown, tests_testunitready }, - { (char *)"Unmap", test_setup, test_teardown, + { "Unmap", test_setup, test_teardown, tests_unmap }, - { (char *)"Verify10", test_setup, test_teardown, + { "Verify10", test_setup, test_teardown, tests_verify10 }, - { (char *)"Verify12", test_setup, test_teardown, + { "Verify12", test_setup, test_teardown, tests_verify12 }, - { (char *)"Verify16", test_setup, test_teardown, + { "Verify16", test_setup, test_teardown, tests_verify16 }, - { (char *)"Write10", test_setup, test_teardown, + { "Write10", test_setup, test_teardown, tests_write10 }, - { (char *)"Write12", test_setup, test_teardown, + { "Write12", test_setup, test_teardown, tests_write12 }, - { (char *)"Write16", test_setup, test_teardown, + { "Write16", test_setup, test_teardown, tests_write16 }, - { (char *)"WriteSame10", test_setup, test_teardown, + { "WriteSame10", test_setup, test_teardown, tests_writesame10 }, - { (char *)"WriteSame16", test_setup, test_teardown, + { "WriteSame16", test_setup, test_teardown, tests_writesame16 }, - { (char *)"WriteVerify10", test_setup, test_teardown, + { "WriteVerify10", test_setup, test_teardown, tests_writeverify10 }, - { (char *)"WriteVerify12", test_setup, test_teardown, + { "WriteVerify12", test_setup, test_teardown, tests_writeverify12 }, - { (char *)"WriteVerify16", test_setup, test_teardown, + { "WriteVerify16", test_setup, test_teardown, tests_writeverify16 }, - { (char *)"iSCSIcmdsn", test_setup, test_teardown, + { "iSCSIcmdsn", test_setup, test_teardown, tests_iscsi_cmdsn }, - { (char *)"iSCSIResiduals", test_setup, test_teardown, + { "iSCSIResiduals", test_setup, test_teardown, tests_iscsi_residuals }, - CU_SUITE_INFO_NULL + { NULL, NULL, NULL, NULL }, }; -static CU_SuiteInfo scsi_usb_sbc_suites[] = { - { (char *)"CompareAndWrite", test_setup, test_teardown, +static libiscsi_suite_info scsi_usb_sbc_suites[] = { + { "CompareAndWrite", test_setup, test_teardown, tests_compareandwrite }, - { (char *)"GetLBAStatus", test_setup, test_teardown, + { "GetLBAStatus", test_setup, test_teardown, tests_get_lba_status }, - { (char *)"Inquiry", test_setup, test_teardown, + { "Inquiry", test_setup, test_teardown, tests_inquiry }, - { (char *)"Mandatory", test_setup, test_teardown, + { "Mandatory", test_setup, test_teardown, tests_mandatory }, - { (char *)"ModeSense6", test_setup, test_teardown, + { "ModeSense6", test_setup, test_teardown, tests_modesense6 }, - { (char *)"OrWrite", test_setup, test_teardown, + { "OrWrite", test_setup, test_teardown, tests_orwrite }, - { (char *)"Prefetch10", test_setup, test_teardown, + { "Prefetch10", test_setup, test_teardown, tests_prefetch10 }, - { (char *)"Prefetch16", test_setup, test_teardown, + { "Prefetch16", test_setup, test_teardown, tests_prefetch16 }, - { (char *)"PrinReadKeys", test_setup, test_teardown, + { "PrinReadKeys", test_setup, test_teardown, tests_prin_read_keys }, - { (char *)"PrinServiceactionRange", test_setup, test_teardown, + { "PrinServiceactionRange", test_setup, test_teardown, tests_prin_serviceaction_range }, - { (char *)"ProutRegister", test_setup, test_teardown, + { "ProutRegister", test_setup, test_teardown, tests_prout_register }, - { (char *)"ProutReserve", test_setup_pgr, test_teardown_pgr, + { "ProutReserve", test_setup_pgr, test_teardown_pgr, tests_prout_reserve }, - { (char *)"Read6", test_setup, test_teardown, + { "Read6", test_setup, test_teardown, tests_read6 }, - { (char *)"Read10", test_setup, test_teardown, + { "Read10", test_setup, test_teardown, tests_read10 }, - { (char *)"Read12", test_setup, test_teardown, + { "Read12", test_setup, test_teardown, tests_read12 }, - { (char *)"Read16", test_setup, test_teardown, + { "Read16", test_setup, test_teardown, tests_read16 }, - { (char *)"ReadCapacity10", test_setup, test_teardown, + { "ReadCapacity10", test_setup, test_teardown, tests_readcapacity10 }, - { (char *)"ReadCapacity16", test_setup, test_teardown, + { "ReadCapacity16", test_setup, test_teardown, tests_readcapacity16 }, - { (char *)"ReadOnly", test_setup, test_teardown, + { "ReadOnly", test_setup, test_teardown, tests_readonly }, - { (char *)"ReportSupportedOpcodes", test_setup, test_teardown, + { "ReportSupportedOpcodes", test_setup, test_teardown, tests_report_supported_opcodes }, - { (char *)"Reserve6", test_setup, test_teardown, + { "Reserve6", test_setup, test_teardown, tests_reserve6 }, - { (char *)"TestUnitReady", test_setup, test_teardown, + { "TestUnitReady", test_setup, test_teardown, tests_testunitready }, - { (char *)"Unmap", test_setup, test_teardown, + { "Unmap", test_setup, test_teardown, tests_unmap }, - { (char *)"Verify10", test_setup, test_teardown, + { "Verify10", test_setup, test_teardown, tests_verify10 }, - { (char *)"Verify12", test_setup, test_teardown, + { "Verify12", test_setup, test_teardown, tests_verify12 }, - { (char *)"Verify16", test_setup, test_teardown, + { "Verify16", test_setup, test_teardown, tests_verify16 }, - { (char *)"Write10", test_setup, test_teardown, + { "Write10", test_setup, test_teardown, tests_write10 }, - { (char *)"Write12", test_setup, test_teardown, + { "Write12", test_setup, test_teardown, tests_write12 }, - { (char *)"Write16", test_setup, test_teardown, + { "Write16", test_setup, test_teardown, tests_write16 }, - { (char *)"WriteSame10", test_setup, test_teardown, + { "WriteSame10", test_setup, test_teardown, tests_writesame10 }, - { (char *)"WriteSame16", test_setup, test_teardown, + { "WriteSame16", test_setup, test_teardown, tests_writesame16 }, - { (char *)"WriteVerify10", test_setup, test_teardown, + { "WriteVerify10", test_setup, test_teardown, tests_writeverify10 }, - { (char *)"WriteVerify12", test_setup, test_teardown, + { "WriteVerify12", test_setup, test_teardown, tests_writeverify12 }, - { (char *)"WriteVerify16", test_setup, test_teardown, + { "WriteVerify16", test_setup, test_teardown, tests_writeverify16 }, - CU_SUITE_INFO_NULL + { NULL, NULL, NULL, NULL }, }; struct test_family { const char *name; - CU_SuiteInfo *suites; + libiscsi_suite_info *suites; }; static struct test_family families[] = { @@ -747,20 +753,20 @@ print_usage(void) fprintf(stderr, "\n"); } -int +CU_ST_RETTYPE test_setup(void) { iscsic = iscsi_context_login(initiatorname1, tgt_url, &tgt_lun); if (iscsic == NULL) { fprintf(stderr, "error: Failed to login to target for test set-up\n"); - return 1; + CU_ST_RETURN(1); } task = NULL; - return 0; + CU_ST_RETURN(0); } -int +CU_ST_RETTYPE test_setup_pgr(void) { task = NULL; @@ -770,7 +776,7 @@ test_setup_pgr(void) if (iscsic == NULL) { fprintf(stderr, "error: Failed to login to target for test set-up\n"); - return 1; + CU_ST_RETURN(1); } iscsic2 = iscsi_context_login(initiatorname2, tgt_url, &tgt_lun2); @@ -780,12 +786,12 @@ test_setup_pgr(void) iscsi_logout_sync(iscsic); iscsi_destroy_context(iscsic); iscsic = NULL; - return 1; + CU_ST_RETURN(1); } - return 0; + CU_ST_RETURN(0); } -int +CU_ST_RETTYPE test_teardown(void) { if (task) { @@ -797,10 +803,10 @@ test_teardown(void) iscsi_destroy_context(iscsic); iscsic = NULL; } - return 0; + CU_ST_RETURN(0); } -int +CU_ST_RETTYPE test_teardown_pgr(void) { test_teardown(); @@ -808,14 +814,14 @@ test_teardown_pgr(void) free(read_write_buf); read_write_buf = NULL; } - return 0; + CU_ST_RETURN(0); } static void list_all_tests(void) { struct test_family *fp; - CU_SuiteInfo *sp; + libiscsi_suite_info *sp; CU_TestInfo *tp; for (fp = families; fp->name; fp++) { @@ -835,44 +841,44 @@ static CU_ErrorCode add_tests(const char *testname_re) { char *family_re = NULL; - const char *suite_re = NULL; - const char *test_re = NULL; + char *suite_re = NULL; + char *test_re = NULL; char *cp; struct test_family *fp; - CU_SuiteInfo *sp; + libiscsi_suite_info *sp; CU_TestInfo *tp; - /* if not testname(s) register all tests and return */ + /* if not testname(s) register all tests */ if (!testname_re) { - return CU_register_suites(all_suites); - } - - /* - * break testname_re into family/suite/test - * - * syntax is: FAMILY[.SUITE[.TEST]] - */ - family_re = strdup(testname_re); - if ((cp = strchr(family_re, '.')) != NULL) { - *cp++ = 0; - suite_re = cp; - if ((cp = strchr(suite_re, '.')) != NULL) { + family_re = strdup("*"); + suite_re = strdup("*"); + test_re = strdup("*"); + } else { + /* + * break testname_re into family/suite/test + * + * syntax is: FAMILY[.SUITE[.TEST]] + */ + family_re = strdup(testname_re); + if ((cp = strchr(family_re, '.')) != NULL) { *cp++ = 0; - test_re = cp; + suite_re = strdup(cp); + if ((cp = strchr(suite_re, '.')) != NULL) { + *cp++ = 0; + test_re = strdup(cp); + } + } + if (!suite_re) + suite_re = strdup("*"); + if (!test_re) + test_re = strdup("*"); + if (!family_re) { + fprintf(stderr, + "error: can't parse test family name: %s\n", + family_re); + return CUE_NOTEST; } - } - if (suite_re == NULL) { - suite_re = "*"; - } - if (test_re == NULL) { - test_re = "*"; - } - - if (!family_re) { - fprintf(stderr, "error: can't parse test family name: %s\n", - family_re); - return CUE_NOTEST; } /* @@ -905,8 +911,9 @@ add_tests(const char *testname_re) } /* all done -- clean up */ - if (family_re) - free(family_re); + free(family_re); + free(suite_re); + free(test_re); return CUE_SUCCESS; } diff --git a/test-tool/iscsi-test-cu.h b/test-tool/iscsi-test-cu.h index 8d93c76..11bb2cf 100644 --- a/test-tool/iscsi-test-cu.h +++ b/test-tool/iscsi-test-cu.h @@ -35,10 +35,20 @@ extern struct iscsi_context *iscsic2; extern int tgt_lun2; extern unsigned char *read_write_buf; -int test_setup(void); -int test_teardown(void); -int test_setup_pgr(void); -int test_teardown_pgr(void); +#ifdef HAVE_CU_SUITEINFO_PSETUPFUNC +/* libcunit version 2 */ +#define CU_ST_RETTYPE void +#define CU_ST_RETURN(v) return +#else +/* libcunit version 1 */ +#define CU_ST_RETTYPE int +#define CU_ST_RETURN(v) return v +#endif + +CU_ST_RETTYPE test_setup(void); +CU_ST_RETTYPE test_teardown(void); +CU_ST_RETTYPE test_setup_pgr(void); +CU_ST_RETTYPE test_teardown_pgr(void); void test_compareandwrite_simple(void); void test_compareandwrite_miscompare(void); From 4653cd8df4c14f4eea37b6ce66277330774dd3d5 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 29 Sep 2013 21:04:48 +0200 Subject: [PATCH 06/17] iscsi_reconnect: Fix a use-after-free This patch fixes the following Valgrind complaint: Invalid read of size 4 at 0x524A858: iscsi_reconnect (connect.c:378) by 0x5258794: iscsi_service (socket.c:707) by 0x52599C4: event_loop (sync.c:67) by 0x525AFD7: iscsi_reserve6_sync (sync.c:1096) by 0x40A40A: reserve6 (iscsi-support.c:3291) by 0x422C95: test_reserve6_target_warm_reset (test_reserve6_target_warm_reset.c:39) by 0x503B05F: ??? (in /usr/lib/libcunit.so.1.0.1) by 0x503B375: ??? (in /usr/lib/libcunit.so.1.0.1) by 0x503B69F: CU_run_all_tests (in /usr/lib/libcunit.so.1.0.1) by 0x403171: main (iscsi-test-cu.c:1258) Address 0x6443958 is 3,032 bytes inside a block of size 4,120 free'd at 0x4C2B83A: free (vg_replace_malloc.c:468) by 0x524A846: iscsi_reconnect (connect.c:374) by 0x5258794: iscsi_service (socket.c:707) by 0x52599C4: event_loop (sync.c:67) by 0x525AFD7: iscsi_reserve6_sync (sync.c:1096) by 0x40A40A: reserve6 (iscsi-support.c:3291) by 0x422C95: test_reserve6_target_warm_reset (test_reserve6_target_warm_reset.c:39) by 0x503B05F: ??? (in /usr/lib/libcunit.so.1.0.1) by 0x503B375: ??? (in /usr/lib/libcunit.so.1.0.1) by 0x503B69F: CU_run_all_tests (in /usr/lib/libcunit.so.1.0.1) by 0x403171: main (iscsi-test-cu.c:1258) Signed-off-by: Bart Van Assche --- lib/connect.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/connect.c b/lib/connect.c index ee3a0ef..384cf51 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -369,13 +369,13 @@ try_again: iscsi->mallocs+=old_iscsi->mallocs; iscsi->frees+=old_iscsi->frees; + ISCSI_LOG(iscsi, 2, "reconnect was successful"); + memcpy(old_iscsi, iscsi, sizeof(struct iscsi_context)); - memset(iscsi, 0, sizeof(struct iscsi_context)); free(iscsi); old_iscsi->is_reconnecting = 0; old_iscsi->last_reconnect = time(NULL); - ISCSI_LOG(iscsi, 2, "reconnect was successful"); return 0; } From 1337185fe9aa291a977afd8d58fb53ec32295689 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 29 Sep 2013 21:05:37 +0200 Subject: [PATCH 07/17] test_read6_simple: Avoid triggering a use-after-free Do not use the 'task' pointer after the memory it points at has been freed. Detected via valgrind. Signed-off-by: Bart Van Assche --- test-tool/test_read6_simple.c | 1 - 1 file changed, 1 deletion(-) diff --git a/test-tool/test_read6_simple.c b/test-tool/test_read6_simple.c index 617f4ac..b287514 100644 --- a/test-tool/test_read6_simple.c +++ b/test-tool/test_read6_simple.c @@ -70,7 +70,6 @@ test_read6_simple(void) if (task->status != SCSI_STATUS_GOOD) { logging(LOG_NORMAL, "[FAILED] READ6 command: " "failed with sense. %s", iscsi_get_error(iscsic)); - scsi_free_scsi_task(task); } CU_ASSERT_EQUAL(task->status, SCSI_STATUS_GOOD); From e5ba667ceaac3c6da6882218ee22769ff35facee Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 29 Sep 2013 21:06:29 +0200 Subject: [PATCH 08/17] test_writesame16_unmap: Do not leak memory if WRITESAME16 is not supported Detected by Valgrind. Signed-off-by: Bart Van Assche --- test-tool/test_writesame16_unmap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test-tool/test_writesame16_unmap.c b/test-tool/test_writesame16_unmap.c index 6a69423..3e3dfd9 100644 --- a/test-tool/test_writesame16_unmap.c +++ b/test-tool/test_writesame16_unmap.c @@ -54,7 +54,7 @@ test_writesame16_unmap(void) if (ret == -2) { logging(LOG_NORMAL, "[SKIPPED] WRITESAME16 is not implemented."); CU_PASS("[SKIPPED] Target does not support WRITESAME16. Skipping test"); - return; + goto finished; } CU_ASSERT_EQUAL(ret, 0); @@ -142,7 +142,7 @@ test_writesame16_unmap(void) "BlockLimits VPD is missing."); CU_FAIL("[FAILED] WRITESAME16 works but " "BlockLimits VPD is missing."); - return; + goto finished; } i = 256; @@ -247,5 +247,7 @@ test_writesame16_unmap(void) 0, 1, 0, 0, buf); CU_ASSERT_EQUAL(ret, 0); } + +finished: free(buf); } From 8c87941b00a412fbe7209d5f2b4bb045d10747ba Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 13:36:02 -0700 Subject: [PATCH 09/17] Don't use variadic macros of the form args... This is a GCC extension and is not portable. --- include/iscsi-private.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/iscsi-private.h b/include/iscsi-private.h index be85dbc..a905056 100644 --- a/include/iscsi-private.h +++ b/include/iscsi-private.h @@ -321,10 +321,10 @@ void iscsi_set_noautoreconnect(struct iscsi_context *iscsi, int state); void iscsi_decrement_iface_rr(void); -#define ISCSI_LOG(iscsi, level, format, args...) \ +#define ISCSI_LOG(iscsi, level, format, ...) \ do { \ if (level <= iscsi->log_level && iscsi->log_fn) { \ - iscsi_log_message(iscsi, level, format, ## args); \ + iscsi_log_message(iscsi, level, format, ## __VA_ARGS__); \ } \ } while (0) From 720e426c920852577a84ec04d536e37696d3f10d Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 13:43:10 -0700 Subject: [PATCH 10/17] Always declare variables at the start of a new scope. --- lib/connect.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/connect.c b/lib/connect.c index 384cf51..ec95cbb 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -231,6 +231,7 @@ void iscsi_defer_reconnect(struct iscsi_context *iscsi) int iscsi_reconnect(struct iscsi_context *old_iscsi) { struct iscsi_context *iscsi = old_iscsi; + int retry = 0; /* if there is already a deferred reconnect do not try again */ if (iscsi->reconnect_deferred) { @@ -249,8 +250,6 @@ int iscsi_reconnect(struct iscsi_context *old_iscsi) return 0; } - int retry = 0; - if (old_iscsi->last_reconnect) { if (time(NULL) - old_iscsi->last_reconnect < 5) sleep(5); } @@ -288,20 +287,21 @@ try_again: iscsi->reconnect_max_retries = old_iscsi->reconnect_max_retries; if (iscsi_full_connect_sync(iscsi, iscsi->portal, iscsi->lun) != 0) { + int backoff = retry; + if (iscsi->reconnect_max_retries != -1 && retry >= iscsi->reconnect_max_retries) { iscsi_defer_reconnect(old_iscsi); iscsi_destroy_context(iscsi); return -1; } - int backoff=retry; if (backoff > 10) { - backoff+=rand()%10; - backoff-=5; + backoff += rand() % 10; + backoff -= 5; } if (backoff > 30) { - backoff=30; + backoff = 30; } - ISCSI_LOG(iscsi, 1, "reconnect try %d failed, waiting %d seconds",retry,backoff); + ISCSI_LOG(iscsi, 1, "reconnect try %d failed, waiting %d seconds", retry, backoff); iscsi_destroy_context(iscsi); sleep(backoff); retry++; From 1828481ba284c08eaf02dae62a046bcc622a2f42 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 13:48:00 -0700 Subject: [PATCH 11/17] Portability fixes. Declare variables at the start of a scope only. --- lib/init.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/init.c b/lib/init.c index 048f59a..1e53751 100644 --- a/lib/init.c +++ b/lib/init.c @@ -82,17 +82,19 @@ void* iscsi_szmalloc(struct iscsi_context *iscsi, size_t size) { } void iscsi_sfree(struct iscsi_context *iscsi, void* ptr) { - if (ptr == NULL) return; + if (ptr == NULL) { + return; + } if (iscsi->smalloc_free == SMALL_ALLOC_MAX_FREE) { - /* SMALL_ALLOC_MAX_FREE should be adjusted that this happens rarely */ - ISCSI_LOG(iscsi,6,"smalloc free == SMALLOC_MAX_FREE"); int i; + /* SMALL_ALLOC_MAX_FREE should be adjusted that this happens rarely */ + ISCSI_LOG(iscsi, 6, "smalloc free == SMALLOC_MAX_FREE"); /* remove oldest half of free pointers and copy * upper half to lower half */ - iscsi->smalloc_free>>=1; - for (i=0; ismalloc_free; i++) { + iscsi->smalloc_free >>= 1; + for (i = 0; i < iscsi->smalloc_free; i++) { iscsi_free(iscsi, iscsi->smalloc_ptrs[i]); - iscsi->smalloc_ptrs[i] = iscsi->smalloc_ptrs[i+iscsi->smalloc_free]; + iscsi->smalloc_ptrs[i] = iscsi->smalloc_ptrs[i + iscsi->smalloc_free]; } } iscsi->smalloc_ptrs[iscsi->smalloc_free++] = ptr; @@ -102,6 +104,7 @@ struct iscsi_context * iscsi_create_context(const char *initiator_name) { struct iscsi_context *iscsi; + size_t required = ISCSI_RAW_HEADER_SIZE + ISCSI_DIGEST_SIZE; if (!initiator_name[0]) { return NULL; @@ -177,10 +180,15 @@ iscsi_create_context(const char *initiator_name) max(ISCSI_HEADER_SIZE, sizeof(struct iscsi_pdu), sizeof(struct iscsi_in_pdu)) rounded up to the next power of 2. */ iscsi->smalloc_size = 1; - size_t required = ISCSI_RAW_HEADER_SIZE + ISCSI_DIGEST_SIZE; - if (sizeof(struct iscsi_pdu) > required) required = sizeof(struct iscsi_pdu); - if (sizeof(struct iscsi_in_pdu) > required) required = sizeof(struct iscsi_in_pdu); - while (iscsi->smalloc_size < required) iscsi->smalloc_size <<= 1; + if (sizeof(struct iscsi_pdu) > required) { + required = sizeof(struct iscsi_pdu); + } + if (sizeof(struct iscsi_in_pdu) > required) { + required = sizeof(struct iscsi_in_pdu); + } + while (iscsi->smalloc_size < required) { + iscsi->smalloc_size <<= 1; + } ISCSI_LOG(iscsi,5,"small allocation size is %d byte", iscsi->smalloc_size); return iscsi; From 27b82512d540f5251309171ef35c5461a163cc4f Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 13:50:55 -0700 Subject: [PATCH 12/17] Remove 'inline'. Some compilers do not support it. --- lib/login.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/login.c b/lib/login.c index 0448ce2..bbfc09d 100644 --- a/lib/login.c +++ b/lib/login.c @@ -638,7 +638,7 @@ typedef struct MD5Context *gcry_md_hd_t; #define gcry_md_write MD5Update #define GCRY_MD_MD5 1 -static inline void gcry_md_open(gcry_md_hd_t *hd, int algo, unsigned int flags) +static void gcry_md_open(gcry_md_hd_t *hd, int algo, unsigned int flags) { assert(algo == GCRY_MD_MD5 && flags == 0); *hd = malloc(sizeof(struct MD5Context)); @@ -647,12 +647,12 @@ static inline void gcry_md_open(gcry_md_hd_t *hd, int algo, unsigned int flags) } } -static inline void gcry_md_putc(gcry_md_hd_t h, unsigned char c) +static void gcry_md_putc(gcry_md_hd_t h, unsigned char c) { MD5Update(h, &c, 1); } -static inline char *gcry_md_read(gcry_md_hd_t h, int algo) +static char *gcry_md_read(gcry_md_hd_t h, int algo) { unsigned char digest[16]; assert(algo == 0 || algo == GCRY_MD_MD5); @@ -661,7 +661,7 @@ static inline char *gcry_md_read(gcry_md_hd_t h, int algo) return memcpy(h->buf, digest, sizeof(digest)); } -static inline void gcry_md_close(gcry_md_hd_t h) +static void gcry_md_close(gcry_md_hd_t h) { memset(h, 0, sizeof(*h)); free(h); From d7b7c7727af209c99c95bd0d0b33e3f5428d8d3a Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 15:34:56 -0700 Subject: [PATCH 13/17] Fix win32 so it works again --- lib/logging.c | 4 ++++ lib/pdu.c | 6 +++++- lib/scsi-lowlevel.c | 1 + lib/socket.c | 21 ++++++++++++++------- win32/vsbuild.bat | 5 +++-- win32/win32_compat.c | 10 ++++++++++ win32/win32_compat.h | 13 +++++++++++++ 7 files changed, 50 insertions(+), 10 deletions(-) diff --git a/lib/logging.c b/lib/logging.c index 6a7c6d6..89e1e14 100644 --- a/lib/logging.c +++ b/lib/logging.c @@ -69,7 +69,11 @@ iscsi_log_message(struct iscsi_context *iscsi, int level, const char *format, .. if (iscsi->target_name[0]) { static char message2[1024]; +#if defined(WIN32) + _snprintf_s(message2, 1024, 1024, "%s [%s]", message, iscsi->target_name); +#else snprintf(message2, 1024, "%s [%s]", message, iscsi->target_name); +#endif iscsi->log_fn(level, message2); } else diff --git a/lib/pdu.c b/lib/pdu.c index 08ec994..f0b900a 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -73,7 +73,11 @@ void iscsi_dump_pdu_header(struct iscsi_context *iscsi, unsigned char *data) { char dump[ISCSI_RAW_HEADER_SIZE*3+1]={0}; int i; for (i=0;i +#include "win32/win32_compat.h" #else #include #endif diff --git a/lib/socket.c b/lib/socket.c index 468af32..041c25e 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -41,6 +41,7 @@ #if defined(WIN32) #include #include +#include "win32/win32_compat.h" #define ioctl ioctlsocket #define close closesocket #else @@ -56,6 +57,7 @@ #include #endif +#include #include #include #include @@ -140,7 +142,7 @@ int set_tcp_sockopt(int sockfd, int optname, int value) level = SOL_TCP; #endif - return setsockopt(sockfd, level, optname, &value, sizeof(value)); + return setsockopt(sockfd, level, optname, (char *)&value, sizeof(value)); } #ifndef TCP_USER_TIMEOUT @@ -327,7 +329,11 @@ iscsi_connect_async(struct iscsi_context *iscsi, const char *portal, } if (connect(iscsi->fd, &sa.sa, socksize) != 0 +#if defined(WIN32) + && WSAGetLastError() != WSAEWOULDBLOCK) { +#else && errno != EINPROGRESS) { +#endif iscsi_set_error(iscsi, "Connect failed with errno : " "%s(%d)", strerror(errno), errno); close(iscsi->fd); @@ -700,7 +706,7 @@ iscsi_write_to_socket(struct iscsi_context *iscsi) return 0; } -static inline int +static int iscsi_service_reconnect_if_loggedin(struct iscsi_context *iscsi) { if (iscsi->is_loggedin) { @@ -719,7 +725,7 @@ iscsi_service(struct iscsi_context *iscsi, int revents) socklen_t err_size = sizeof(err); if (getsockopt(iscsi->fd, SOL_SOCKET, SO_ERROR, - &err, &err_size) != 0 || err != 0) { + (char *)&err, &err_size) != 0 || err != 0) { if (err == 0) { err = errno; } @@ -751,8 +757,11 @@ iscsi_service(struct iscsi_context *iscsi, int revents) if (iscsi->is_connected == 0 && iscsi->fd != -1 && revents&POLLOUT) { int err = 0; socklen_t err_size = sizeof(err); + struct sockaddr_in local; + socklen_t local_l = sizeof(local); + if (getsockopt(iscsi->fd, SOL_SOCKET, SO_ERROR, - &err, &err_size) != 0 || err != 0) { + (char *)&err, &err_size) != 0 || err != 0) { if (err == 0) { err = errno; } @@ -768,8 +777,6 @@ iscsi_service(struct iscsi_context *iscsi, int revents) return iscsi_service_reconnect_if_loggedin(iscsi); } - struct sockaddr_in local; - socklen_t local_l = sizeof(local); if (getsockname(iscsi->fd, (struct sockaddr *) &local, &local_l) == 0) { ISCSI_LOG(iscsi, 2, "connection established (%s:%u -> %s)", inet_ntoa(local.sin_addr), (unsigned)ntohs(local.sin_port),iscsi->connected_portal); @@ -881,7 +888,7 @@ int iscsi_set_tcp_keepalive(struct iscsi_context *iscsi, int idle _U_, int count { #ifdef SO_KEEPALIVE int value = 1; - if (setsockopt(iscsi->fd, SOL_SOCKET, SO_KEEPALIVE, &value, sizeof(value)) != 0) { + if (setsockopt(iscsi->fd, SOL_SOCKET, SO_KEEPALIVE, (char *)&value, sizeof(value)) != 0) { iscsi_set_error(iscsi, "TCP: Failed to set socket option SO_KEEPALIVE. Error %s(%d)", strerror(errno), errno); return -1; } diff --git a/win32/vsbuild.bat b/win32/vsbuild.bat index 0e48de0..6dfeb75 100644 --- a/win32/vsbuild.bat +++ b/win32/vsbuild.bat @@ -11,6 +11,7 @@ cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\crc3 cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\discovery.c -Folib\discovery.obj cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\init.c -Folib\init.obj cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\login.c -Folib\login.obj +cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\logging.c -Folib\logging.obj cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\md5.c -Folib\md5.obj cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\nop.c -Folib\nop.obj cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd lib\pdu.c -Folib\pdu.obj @@ -27,9 +28,9 @@ cl /I. /Iinclude -Zi -Od -c -D_U_="" -DWIN32 -D_WIN32_WINNT=0x0600 -MDd win32\wi rem rem create a linklibrary/dll rem -lib /out:lib\libiscsi.lib /def:lib\libiscsi.def lib\connect.obj lib\crc32c.obj lib\discovery.obj lib\init.obj lib\login.obj lib\md5.obj lib\nop.obj lib\pdu.obj lib\iscsi-command.obj lib\scsi-lowlevel.obj lib\socket.obj lib\sync.obj lib\task_mgmt.obj lib\win32_compat.obj +lib /out:lib\libiscsi.lib /def:lib\libiscsi.def lib\connect.obj lib\crc32c.obj lib\discovery.obj lib\init.obj lib\login.obj lib\logging.obj lib\md5.obj lib\nop.obj lib\pdu.obj lib\iscsi-command.obj lib\scsi-lowlevel.obj lib\socket.obj lib\sync.obj lib\task_mgmt.obj lib\win32_compat.obj -link /DLL /out:lib\libiscsi.dll /DEBUG /DEBUGTYPE:cv lib\libiscsi.exp lib\connect.obj lib\crc32c.obj lib\discovery.obj lib\init.obj lib\login.obj lib\md5.obj lib\nop.obj lib\pdu.obj lib\iscsi-command.obj lib\scsi-lowlevel.obj lib\socket.obj lib\sync.obj lib\task_mgmt.obj lib\win32_compat.obj ws2_32.lib kernel32.lib +link /DLL /out:lib\libiscsi.dll /DEBUG /DEBUGTYPE:cv lib\libiscsi.exp lib\connect.obj lib\crc32c.obj lib\discovery.obj lib\init.obj lib\login.obj lib\logging.obj lib\md5.obj lib\nop.obj lib\pdu.obj lib\iscsi-command.obj lib\scsi-lowlevel.obj lib\socket.obj lib\sync.obj lib\task_mgmt.obj lib\win32_compat.obj ws2_32.lib kernel32.lib diff --git a/win32/win32_compat.c b/win32/win32_compat.c index fe2ca05..c900544 100644 --- a/win32/win32_compat.c +++ b/win32/win32_compat.c @@ -198,4 +198,14 @@ int win32_gettimeofday(struct timeval *tv, struct timezone *tz) return 0; } +ssize_t win32_readv(int fd, const struct iovec *iov, int iovcnt) +{ + return read(fd, iov[0].iov_base, iov[0].iov_len); +} + +ssize_t win32_writev(int fd, const struct iovec *iov, int iovcnt) +{ + return write(fd, iov[0].iov_base, iov[0].iov_len); +} + #endif diff --git a/win32/win32_compat.h b/win32/win32_compat.h index 0119433..1df3630 100644 --- a/win32/win32_compat.h +++ b/win32/win32_compat.h @@ -32,13 +32,19 @@ THE SOFTWARE. #include #include #include +#include #include +#define SOL_TCP IPPROTO_TCP + +typedef int ssize_t; typedef int uid_t; typedef int gid_t; typedef int socklen_t; /* Wrapper macros to call misc. functions win32 is missing */ +#define writev win32_writev +#define readv win32_readv #define poll(x, y, z) win32_poll(x, y, z) #define inet_pton(x,y,z) win32_inet_pton(x,y,z) #define sleep(x) Sleep(x * 1000) @@ -46,5 +52,12 @@ int win32_inet_pton(int af, const char * src, void * dst); int win32_poll(struct pollfd *fds, unsigned int nfsd, int timeout); int win32_gettimeofday(struct timeval *tv, struct timezone *tz); +struct iovec { + void *iov_base; + size_t iov_len; +}; + +#define inline + #endif//win32_COMPAT_H_ #endif//WIN32 From bca9e3d02224a6cce91cd9c93fc0bfd20e6af67b Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 15:39:09 -0700 Subject: [PATCH 14/17] We don't have __attribute__ on visual studio --- include/iscsi-private.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/iscsi-private.h b/include/iscsi-private.h index a905056..9eca824 100644 --- a/include/iscsi-private.h +++ b/include/iscsi-private.h @@ -299,8 +299,13 @@ int iscsi_process_reject(struct iscsi_context *iscsi, struct iscsi_in_pdu *in); int iscsi_send_target_nop_out(struct iscsi_context *iscsi, uint32_t ttt); +#if defined(WIN32) +void iscsi_set_error(struct iscsi_context *iscsi, const char *error_string, + ...); +#else void iscsi_set_error(struct iscsi_context *iscsi, const char *error_string, ...) __attribute__((format(printf, 2, 3))); +#endif struct scsi_iovector *iscsi_get_scsi_task_iovector_in(struct iscsi_context *iscsi, struct iscsi_in_pdu *in); struct scsi_iovector *iscsi_get_scsi_task_iovector_out(struct iscsi_context *iscsi, struct iscsi_pdu *pdu); From eec84c2805d792115e971bdb378b16c8e0b7026c Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 17:06:21 -0700 Subject: [PATCH 15/17] Add PowerOnOccured as a sense code we allow and ignore during connect --- lib/connect.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/connect.c b/lib/connect.c index ec95cbb..271ce2b 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -50,6 +50,7 @@ iscsi_testunitready_cb(struct iscsi_context *iscsi, int status, if (status != 0) { if (task->sense.key == SCSI_SENSE_UNIT_ATTENTION && (task->sense.ascq == SCSI_SENSE_ASCQ_BUS_RESET || + task->sense.ascq == SCSI_SENSE_ASCQ_POWER_ON_OCCURED || task->sense.ascq == SCSI_SENSE_ASCQ_NEXUS_LOSS)) { /* This is just the normal unitattention/busreset * you always get just after a fresh login. Try From 305d6e1644d5970af74e7e88366182f126397df8 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 17:23:46 -0700 Subject: [PATCH 16/17] Get rid of a lot of WIN32 ifdefs by providing a compatibility function instead --- lib/logging.c | 9 +++++---- lib/login.c | 33 +-------------------------------- win32/win32_compat.h | 1 + 3 files changed, 7 insertions(+), 36 deletions(-) diff --git a/lib/logging.c b/lib/logging.c index 89e1e14..f71d26b 100644 --- a/lib/logging.c +++ b/lib/logging.c @@ -30,6 +30,10 @@ #include #endif +#if defined(WIN32) +#include "win32/win32_compat.h" +#endif + #include #include #include "iscsi.h" @@ -69,11 +73,8 @@ iscsi_log_message(struct iscsi_context *iscsi, int level, const char *format, .. if (iscsi->target_name[0]) { static char message2[1024]; -#if defined(WIN32) - _snprintf_s(message2, 1024, 1024, "%s [%s]", message, iscsi->target_name); -#else + snprintf(message2, 1024, "%s [%s]", message, iscsi->target_name); -#endif iscsi->log_fn(level, message2); } else diff --git a/lib/login.c b/lib/login.c index bbfc09d..d3bce65 100644 --- a/lib/login.c +++ b/lib/login.c @@ -33,6 +33,7 @@ #if defined(WIN32) #include +#include "win32/win32_compat.h" #endif #include @@ -58,11 +59,7 @@ iscsi_login_add_initiatorname(struct iscsi_context *iscsi, struct iscsi_pdu *pdu return 0; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "InitiatorName=%s", iscsi->initiator_name) == -1) { -#else if (snprintf(str, MAX_STRING_SIZE, "InitiatorName=%s", iscsi->initiator_name) == -1) { -#endif iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; } @@ -85,11 +82,7 @@ iscsi_login_add_alias(struct iscsi_context *iscsi, struct iscsi_pdu *pdu) return 0; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "InitiatorAlias=%s", iscsi->alias) == -1) { -#else if (snprintf(str, MAX_STRING_SIZE, "InitiatorAlias=%s", iscsi->alias) == -1) { -#endif iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; } @@ -119,11 +112,7 @@ iscsi_login_add_targetname(struct iscsi_context *iscsi, struct iscsi_pdu *pdu) return -1; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "TargetName=%s", iscsi->target_name) == -1) { -#else if (snprintf(str, MAX_STRING_SIZE, "TargetName=%s", iscsi->target_name) == -1) { -#endif iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; } @@ -235,11 +224,7 @@ iscsi_login_add_initialr2t(struct iscsi_context *iscsi, struct iscsi_pdu *pdu) return 0; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "InitialR2T=%s", iscsi->want_initial_r2t == ISCSI_INITIAL_R2T_NO ? -#else if (snprintf(str, MAX_STRING_SIZE, "InitialR2T=%s", iscsi->want_initial_r2t == ISCSI_INITIAL_R2T_NO ? -#endif "No" : "Yes") == -1) { iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; @@ -264,11 +249,7 @@ iscsi_login_add_immediatedata(struct iscsi_context *iscsi, struct iscsi_pdu *pdu return 0; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "ImmediateData=%s", iscsi->want_immediate_data == ISCSI_IMMEDIATE_DATA_NO ? -#else if (snprintf(str, MAX_STRING_SIZE, "ImmediateData=%s", iscsi->want_immediate_data == ISCSI_IMMEDIATE_DATA_NO ? -#endif "No" : "Yes") == -1) { iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; @@ -293,11 +274,7 @@ iscsi_login_add_maxburstlength(struct iscsi_context *iscsi, struct iscsi_pdu *pd return 0; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "MaxBurstLength=%d", iscsi->max_burst_length) == -1) { -#else if (snprintf(str, MAX_STRING_SIZE, "MaxBurstLength=%d", iscsi->max_burst_length) == -1) { -#endif iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; } @@ -320,11 +297,7 @@ iscsi_login_add_firstburstlength(struct iscsi_context *iscsi, struct iscsi_pdu * return 0; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "FirstBurstLength=%d", iscsi->first_burst_length) == -1) { -#else if (snprintf(str, MAX_STRING_SIZE, "FirstBurstLength=%d", iscsi->first_burst_length) == -1) { -#endif iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; } @@ -347,11 +320,7 @@ iscsi_login_add_maxrecvdatasegmentlength(struct iscsi_context *iscsi, struct isc return 0; } -#if defined(WIN32) - if (_snprintf_s(str, MAX_STRING_SIZE, MAX_STRING_SIZE, "MaxRecvDataSegmentLength=%d", iscsi->initiator_max_recv_data_segment_length) == -1) { -#else if (snprintf(str, MAX_STRING_SIZE, "MaxRecvDataSegmentLength=%d", iscsi->initiator_max_recv_data_segment_length) == -1) { -#endif iscsi_set_error(iscsi, "Out-of-memory: aprintf failed."); return -1; } diff --git a/win32/win32_compat.h b/win32/win32_compat.h index 1df3630..463630d 100644 --- a/win32/win32_compat.h +++ b/win32/win32_compat.h @@ -48,6 +48,7 @@ typedef int socklen_t; #define poll(x, y, z) win32_poll(x, y, z) #define inet_pton(x,y,z) win32_inet_pton(x,y,z) #define sleep(x) Sleep(x * 1000) +#define snprintf(a, b, c, ...) _snprintf_s(a, b, b, c, ## __VA_ARGS) int win32_inet_pton(int af, const char * src, void * dst); int win32_poll(struct pollfd *fds, unsigned int nfsd, int timeout); int win32_gettimeofday(struct timeval *tv, struct timezone *tz); From dfdf2091d46dd0e86ff5c4c26fcb85ee59268067 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sat, 5 Oct 2013 17:25:06 -0700 Subject: [PATCH 17/17] typo --- win32/win32_compat.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/win32/win32_compat.h b/win32/win32_compat.h index 463630d..d6adfd7 100644 --- a/win32/win32_compat.h +++ b/win32/win32_compat.h @@ -48,7 +48,7 @@ typedef int socklen_t; #define poll(x, y, z) win32_poll(x, y, z) #define inet_pton(x,y,z) win32_inet_pton(x,y,z) #define sleep(x) Sleep(x * 1000) -#define snprintf(a, b, c, ...) _snprintf_s(a, b, b, c, ## __VA_ARGS) +#define snprintf(a, b, c, ...) _snprintf_s(a, b, b, c, ## __VA_ARGS__) int win32_inet_pton(int af, const char * src, void * dst); int win32_poll(struct pollfd *fds, unsigned int nfsd, int timeout); int win32_gettimeofday(struct timeval *tv, struct timezone *tz);