From 28d0db9c964068103040539c5057a16de6540e22 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi Date: Sat, 25 Nov 2017 16:44:30 +0000 Subject: [PATCH 1/4] lib/pdu.c: clean up empty lines Remove some extra empty lines between functions. Signed-off-by: Felipe Franciosi --- lib/pdu.c | 4 ---- lib/sync.c | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/pdu.c b/lib/pdu.c index 0bcbe11..74acc99 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -168,7 +168,6 @@ iscsi_tcp_free_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu) iscsi_sfree(iscsi, pdu); } - int iscsi_add_data(struct iscsi_context *iscsi, struct iscsi_data *data, unsigned char *dptr, int dsize, int pdualignment) @@ -252,7 +251,6 @@ iscsi_get_pdu_data_size(const unsigned char *hdr) return size; } - int iscsi_get_pdu_padding_size(const unsigned char *hdr) { @@ -332,7 +330,6 @@ int iscsi_process_target_nop_in(struct iscsi_context *iscsi, return 0; } - static void iscsi_reconnect_after_logout(struct iscsi_context *iscsi, int status, void *command_data _U_, void *opaque _U_) { @@ -392,7 +389,6 @@ int iscsi_process_reject(struct iscsi_context *iscsi, return 0; } - static void iscsi_process_pdu_serials(struct iscsi_context *iscsi, struct iscsi_in_pdu *in) { uint32_t itt = scsi_get_uint32(&in->hdr[16]); diff --git a/lib/sync.c b/lib/sync.c index ce59c29..5b209d7 100644 --- a/lib/sync.c +++ b/lib/sync.c @@ -135,6 +135,11 @@ iscsi_full_connect_sync(struct iscsi_context *iscsi, event_loop(iscsi, &state); + /* in case of error, clear loggedin flag to prevent pending pdu callbacks */ + if (state.status != 0) { + iscsi->is_loggedin = 0; + } + return (state.status == SCSI_STATUS_GOOD) ? 0 : -1; } From 5aafc29991a36feaf6d5bdc922b1df3d977accc2 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi Date: Sat, 25 Nov 2017 16:46:57 +0000 Subject: [PATCH 2/4] lib/pdu.c: Fix whitespace formatting iscsi_queue_pdu used whitespaces for identation while the rest of the file uses hard tabs. --- lib/pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pdu.c b/lib/pdu.c index 74acc99..789d412 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -766,5 +766,5 @@ iscsi_timeout_scan(struct iscsi_context *iscsi) int iscsi_queue_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu) { - return iscsi->drv->queue_pdu(iscsi, pdu); + return iscsi->drv->queue_pdu(iscsi, pdu); } From 3c4925e8da4f51559f19166b16200af3ee297aa9 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi Date: Sat, 25 Nov 2017 16:59:00 +0000 Subject: [PATCH 3/4] pdu: Introduce iscsi_cancel_pdus() Introduce a helper exported from lib/pdu.c which cancels all pdus for a given context. This patch eliminates repeated code from various other files which have the same purpose. The only functional difference is that the cancellation done from iscsi-command.c was (incorrectly) not checking for iscsi->is_loggedin before issuing callbacks. Signed-off-by: Felipe Franciosi --- include/iscsi-private.h | 1 + lib/connect.c | 25 +------------------------ lib/init.c | 24 +----------------------- lib/iscsi-command.c | 19 +------------------ lib/pdu.c | 29 +++++++++++++++++++++++++++++ 5 files changed, 33 insertions(+), 65 deletions(-) diff --git a/include/iscsi-private.h b/include/iscsi-private.h index aedbbe0..218cf74 100644 --- a/include/iscsi-private.h +++ b/include/iscsi-private.h @@ -284,6 +284,7 @@ void iscsi_pdu_set_itt(struct iscsi_pdu *pdu, uint32_t itt); void iscsi_pdu_set_ritt(struct iscsi_pdu *pdu, uint32_t ritt); void iscsi_pdu_set_datasn(struct iscsi_pdu *pdu, uint32_t datasn); void iscsi_pdu_set_bufferoffset(struct iscsi_pdu *pdu, uint32_t bufferoffset); +void iscsi_cancel_pdus(struct iscsi_context *iscsi); int iscsi_pdu_add_data(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, unsigned char *dptr, int dsize); int iscsi_queue_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu); diff --git a/lib/connect.c b/lib/connect.c index 4043f7c..d13cc62 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -270,34 +270,11 @@ void iscsi_set_reconnect_max_retries(struct iscsi_context *iscsi, int count) void iscsi_defer_reconnect(struct iscsi_context *iscsi) { - struct iscsi_pdu *pdu; - iscsi->reconnect_deferred = 1; ISCSI_LOG(iscsi, 2, "reconnect deferred, cancelling all tasks"); - while ((pdu = iscsi->outqueue)) { - ISCSI_LIST_REMOVE(&iscsi->outqueue, pdu); - if (iscsi->is_loggedin && pdu->callback) { - /* If an error happened during connect/login, - we don't want to call any of the callbacks. - */ - pdu->callback(iscsi, SCSI_STATUS_CANCELLED, - NULL, pdu->private_data); - } - iscsi->drv->free_pdu(iscsi, pdu); - } - while ((pdu = iscsi->waitpdu)) { - ISCSI_LIST_REMOVE(&iscsi->waitpdu, pdu); - if (iscsi->is_loggedin && pdu->callback) { - /* If an error happened during connect/login, - we don't want to call any of the callbacks. - */ - pdu->callback(iscsi, SCSI_STATUS_CANCELLED, - NULL, pdu->private_data); - } - iscsi->drv->free_pdu(iscsi, pdu); - } + iscsi_cancel_pdus(iscsi); } void iscsi_reconnect_cb(struct iscsi_context *iscsi _U_, int status, diff --git a/lib/init.c b/lib/init.c index 34a95a5..0bcf1b7 100644 --- a/lib/init.c +++ b/lib/init.c @@ -338,7 +338,6 @@ iscsi_set_targetname(struct iscsi_context *iscsi, const char *target_name) int iscsi_destroy_context(struct iscsi_context *iscsi) { - struct iscsi_pdu *pdu; int i; if (iscsi == NULL) { @@ -349,28 +348,7 @@ iscsi_destroy_context(struct iscsi_context *iscsi) iscsi_disconnect(iscsi); } - while ((pdu = iscsi->outqueue)) { - ISCSI_LIST_REMOVE(&iscsi->outqueue, pdu); - if (iscsi->is_loggedin && pdu->callback) { - /* If an error happened during connect/login, we don't want to - call any of the callbacks. - */ - pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL, - pdu->private_data); - } - iscsi->drv->free_pdu(iscsi, pdu); - } - while ((pdu = iscsi->waitpdu)) { - ISCSI_LIST_REMOVE(&iscsi->waitpdu, pdu); - if (iscsi->is_loggedin && pdu->callback) { - /* If an error happened during connect/login, we don't want to - call any of the callbacks. - */ - pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL, - pdu->private_data); - } - iscsi->drv->free_pdu(iscsi, pdu); - } + iscsi_cancel_pdus(iscsi); if (iscsi->outqueue_current != NULL && iscsi->outqueue_current->flags & ISCSI_PDU_DELETE_WHEN_SENT) { iscsi->drv->free_pdu(iscsi, iscsi->outqueue_current); diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index 5be7ee2..8b6859a 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -2697,22 +2697,5 @@ iscsi_scsi_cancel_task(struct iscsi_context *iscsi, void iscsi_scsi_cancel_all_tasks(struct iscsi_context *iscsi) { - struct iscsi_pdu *pdu; - - while ((pdu = iscsi->waitpdu)) { - ISCSI_LIST_REMOVE(&iscsi->waitpdu, pdu); - if (pdu->callback) { - pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL, - pdu->private_data); - } - iscsi->drv->free_pdu(iscsi, pdu); - } - while ((pdu = iscsi->outqueue)) { - ISCSI_LIST_REMOVE(&iscsi->outqueue, pdu); - if (pdu->callback) { - pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL, - pdu->private_data); - } - iscsi->drv->free_pdu(iscsi, pdu); - } + iscsi_cancel_pdus(iscsi); } diff --git a/lib/pdu.c b/lib/pdu.c index 789d412..441efa7 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -768,3 +768,32 @@ iscsi_queue_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu) { return iscsi->drv->queue_pdu(iscsi, pdu); } + +void +iscsi_cancel_pdus(struct iscsi_context *iscsi) +{ + struct iscsi_pdu *pdu; + + while ((pdu = iscsi->outqueue)) { + ISCSI_LIST_REMOVE(&iscsi->outqueue, pdu); + if (iscsi->is_loggedin && pdu->callback) { + /* If an error happened during connect/login, + we don't want to call any of the callbacks. + */ + pdu->callback(iscsi, SCSI_STATUS_CANCELLED, + NULL, pdu->private_data); + } + iscsi->drv->free_pdu(iscsi, pdu); + } + while ((pdu = iscsi->waitpdu)) { + ISCSI_LIST_REMOVE(&iscsi->waitpdu, pdu); + if (iscsi->is_loggedin && pdu->callback) { + /* If an error happened during connect/login, + we don't want to call any of the callbacks. + */ + pdu->callback(iscsi, SCSI_STATUS_CANCELLED, + NULL, pdu->private_data); + } + iscsi->drv->free_pdu(iscsi, pdu); + } +} From 25eb87c7ee8be65cbba9c63a5720e11eddfa27c7 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi Date: Sat, 25 Nov 2017 17:12:38 +0000 Subject: [PATCH 4/4] sync: cancel pending pdus on error The set of sync connect calls use a stack variable to track the connection status. This is ok because such calls block on event_poll() until the connection is established. However, event_poll may return early in case of errors (or timeout) while PDUs are still queued on the context (and pointing to a local stack). This cancels any pending PDUs before returning from sync connect calls. Signed-off-by: Felipe Franciosi --- lib/sync.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/sync.c b/lib/sync.c index 5b209d7..f8a139c 100644 --- a/lib/sync.c +++ b/lib/sync.c @@ -114,6 +114,11 @@ iscsi_connect_sync(struct iscsi_context *iscsi, const char *portal) /* clear connect_data so it doesnt point to our stack */ iscsi->connect_data = NULL; + /* in case of error, cancel any pending pdus */ + if (state.status != SCSI_STATUS_GOOD) { + iscsi_cancel_pdus(iscsi); + } + return (state.status == SCSI_STATUS_GOOD) ? 0 : -1; } @@ -135,9 +140,9 @@ iscsi_full_connect_sync(struct iscsi_context *iscsi, event_loop(iscsi, &state); - /* in case of error, clear loggedin flag to prevent pending pdu callbacks */ - if (state.status != 0) { - iscsi->is_loggedin = 0; + /* in case of error, cancel any pending pdus */ + if (state.status != SCSI_STATUS_GOOD) { + iscsi_cancel_pdus(iscsi); } return (state.status == SCSI_STATUS_GOOD) ? 0 : -1;