On 03/09/2015 08:54 PM, Lee Duncan wrote:
> Hi Mike:
>
> I am *finally* getting to this bug, previously buried under a mountain of
> more pressing matters.
>
>> On Jan 12, 2015, at 12:50 PM, Mike Christie <micha...@cs.wisc.edu> wrote:
>>
>> Thanks. I merged and pushed these patches.
>>
>> When you get some time, reply to that mail I sent offlist about how long
>> it is taking for the set host net params to succeed, so I can fix my
>> login/init patch as needed.
>
> I have looked at this problem again, and I will reply in a new email
> tomorrow. Fair warning. :)
:)
Hey,
Attached is the patch that was modifying iscsid's code path which from
what I understand handled the same problem you were handling in the
iscsiadm's discovery code path.
The patch is removing the iscsiuio specific calls in iscsid and using
the generic initial login/connect retry code. It is currently retrying
for up to login_timeout seconds instead of the hard coded 5 seconds.
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
>From 05abcf6947f32186e540a102b20cd7d768a29d08 Mon Sep 17 00:00:00 2001
From: Mike Christie <micha...@cs.wisc.edu>
Date: Thu, 5 Feb 2015 02:11:16 -0600
Subject: [PATCH 1/1] iscsid: make sure actor is delated before rescheduling
iscsi_conn_connect() be called from a login_timer that is not deleted.
This causes it to be scheduled multiple times. This patch adds a new
function actor_timer_mod to handle both deletion and rescheduling.
This patch also then removed the uio poll code which was using the
login_timer to schedule itself. This uio retry, is now just done in
the generic initial login retry.
---
usr/actor.c | 8 ++++
usr/actor.h | 3 +-
usr/initiator.c | 122 +++++--------------------------------------------
usr/initiator.h | 1 -
usr/initiator_common.c | 8 ++--
5 files changed, 26 insertions(+), 116 deletions(-)
diff --git a/usr/actor.c b/usr/actor.c
index 37b5024..21cd819 100644
--- a/usr/actor.c
+++ b/usr/actor.c
@@ -182,6 +182,14 @@ actor_timer(actor_t *thread, uint32_t timeout_secs, void (*callback)(void *),
actor_schedule_private(thread, timeout_secs, 0);
}
+void
+actor_timer_mod(actor_t *thread, uint32_t new_timeout_secs, void *data)
+{
+ actor_delete(thread);
+ thread->data = data;
+ actor_schedule_private(thread, new_timeout_secs, 0);
+}
+
/*
* Execute all items that have expired.
*
diff --git a/usr/actor.h b/usr/actor.h
index 7283dce..f572f2e 100644
--- a/usr/actor.h
+++ b/usr/actor.h
@@ -43,7 +43,8 @@ extern void actor_schedule_head(actor_t *thread);
extern void actor_schedule(actor_t *thread);
extern void actor_timer(actor_t *thread, uint32_t delay_secs,
void (*callback)(void *), void *data);
-extern int actor_timer_mod(actor_t *thread, uint32_t new_delay_secs, void *data);
+extern void actor_timer_mod(actor_t *thread, uint32_t new_delay_secs,
+ void *data);
extern void actor_poll(void);
#endif /* ACTOR_H */
diff --git a/usr/initiator.c b/usr/initiator.c
index 1aadc9b..f70db4d 100644
--- a/usr/initiator.c
+++ b/usr/initiator.c
@@ -262,6 +262,7 @@ __session_conn_create(iscsi_session_t *session, int cid)
conn->state = ISCSI_CONN_STATE_FREE;
conn->session = session;
+ actor_init(&conn->login_timer, iscsi_login_timedout, NULL);
/*
* TODO: we must export the socket_fd/transport_eph from sysfs
* so if iscsid is resyncing up we can pick that up and cleanup up
@@ -528,9 +529,7 @@ queue_delayed_reopen(queue_task_t *qtask, int delay)
* iscsi_login_eh can handle the login resched as
* if it were login time out
*/
- actor_delete(&conn->login_timer);
- actor_timer(&conn->login_timer, delay,
- iscsi_login_timedout, qtask);
+ actor_timer_mod(&conn->login_timer, delay, qtask);
}
static int iscsi_conn_connect(struct iscsi_conn *conn, queue_task_t *qtask)
@@ -565,53 +564,10 @@ static int iscsi_conn_connect(struct iscsi_conn *conn, queue_task_t *qtask)
iscsi_sched_ev_context(ev_context, conn, 0, EV_CONN_POLL);
log_debug(3, "Setting login timer %p timeout %d", &conn->login_timer,
conn->login_timeout);
- actor_timer(&conn->login_timer, conn->login_timeout,
- iscsi_login_timedout, qtask);
+ actor_timer_mod(&conn->login_timer, conn->login_timeout, qtask);
return 0;
}
-static void iscsi_uio_poll_login_timedout(void *data)
-{
- struct queue_task *qtask = data;
- struct iscsi_conn *conn = qtask->conn;
- iscsi_session_t *session = conn->session;
-
- log_debug(3, "timeout waiting for UIO ...\n");
- mgmt_ipc_write_rsp(qtask, ISCSI_ERR_TRANS_TIMEOUT);
- conn_delete_timers(conn);
- __session_destroy(session);
-}
-
-static int iscsi_sched_uio_poll(queue_task_t *qtask)
-{
- struct iscsi_conn *conn = qtask->conn;
- struct iscsi_session *session = conn->session;
- struct iscsi_transport *t = session->t;
- struct iscsi_ev_context *ev_context;
-
- if (!t->template->set_net_config)
- return 0;
-
- ev_context = iscsi_ev_context_get(conn, 0);
- if (!ev_context) {
- /* while reopening the recv pool should be full */
- log_error("BUG: __session_conn_reopen could "
- "not get conn context for recv.");
- return -ENOMEM;
- }
-
- ev_context->data = qtask;
- conn->state = ISCSI_CONN_STATE_XPT_WAIT;
-
- iscsi_sched_ev_context(ev_context, conn, 0, EV_UIO_POLL);
-
- log_debug(3, "Setting login UIO poll timer %p timeout %d",
- &conn->login_timer, conn->login_timeout);
- actor_timer(&conn->login_timer, conn->login_timeout,
- iscsi_uio_poll_login_timedout, qtask);
- return -EAGAIN;
-}
-
static void
__session_conn_reopen(iscsi_conn_t *conn, queue_task_t *qtask, int do_stop,
int redirected)
@@ -1740,53 +1696,6 @@ failed_login:
}
-static void session_conn_uio_poll(void *data)
-{
- struct iscsi_ev_context *ev_context = data;
- iscsi_conn_t *conn = ev_context->conn;
- struct iscsi_session *session = conn->session;
- queue_task_t *qtask = ev_context->data;
- int rc;
-
- log_debug(4, "retrying uio poll");
- rc = iscsi_set_net_config(session->t, session,
- &conn->session->nrec.iface);
- if (rc != 0) {
- if (rc == ISCSI_ERR_AGAIN) {
- ev_context->data = qtask;
- iscsi_sched_ev_context(ev_context, conn, 2,
- EV_UIO_POLL);
- return;
- } else {
- log_error("session_conn_uio_poll() "
- "connection failure [0x%x]", rc);
- actor_delete(&conn->login_timer);
- iscsi_login_eh(conn, qtask, ISCSI_ERR_INTERNAL);
- iscsi_ev_context_put(ev_context);
- return;
- }
- }
-
- iscsi_ev_context_put(ev_context);
- actor_delete(&conn->login_timer);
- log_debug(4, "UIO ready trying connect");
-
- /* uIP is ready try to connect */
- if (gettimeofday(&conn->initial_connect_time, NULL))
- log_error("Could not get initial connect time. If "
- "login errors iscsid may give up the initial "
- "login early. You should manually login.");
-
- conn->state = ISCSI_CONN_STATE_XPT_WAIT;
- if (iscsi_conn_connect(conn, qtask)) {
- int delay = ISCSI_CONN_ERR_REOPEN_DELAY;
-
- log_debug(4, "Waiting %u seconds before trying to reconnect.\n",
- delay);
- queue_delayed_reopen(qtask, delay);
- }
-}
-
static int iscsi_sched_ev_context(struct iscsi_ev_context *ev_context,
struct iscsi_conn *conn, unsigned long tmo,
int event)
@@ -1825,12 +1734,7 @@ static int iscsi_sched_ev_context(struct iscsi_ev_context *ev_context,
break;
case EV_CONN_POLL:
actor_init(&ev_context->actor, session_conn_poll,
- ev_context);
- actor_schedule(&ev_context->actor);
- break;
- case EV_UIO_POLL:
- actor_init(&ev_context->actor, session_conn_uio_poll,
- ev_context);
+ ev_context);
actor_schedule(&ev_context->actor);
break;
case EV_CONN_LOGOUT_TIMER:
@@ -1970,14 +1874,12 @@ static int __session_login_task(node_rec_t *rec, queue_task_t *qtask)
rc = iscsi_host_set_net_params(&rec->iface, session);
if (rc == ISCSI_ERR_AGAIN) {
- iscsi_sched_uio_poll(qtask);
/*
- * Cannot block iscsid, so caller is going to internally
- * retry the operation.
+ * host/iscsiuio not ready. Cannot block iscsid, so caller is
+ * going to internally retry the operation.
*/
- qtask->rsp.command = MGMT_IPC_SESSION_LOGIN;
- qtask->rsp.err = ISCSI_SUCCESS;
- return ISCSI_SUCCESS;
+ __session_destroy(session);
+ return ISCSI_ERR_HOST_NOT_FOUND;
} else if (rc) {
__session_destroy(session);
return ISCSI_ERR_LOGIN;
@@ -2024,17 +1926,17 @@ session_login_task(node_rec_t *rec, queue_task_t *qtask)
static void session_login_task_retry(void *data)
{
struct login_task_retry_info *info = data;
+ struct node_rec *rec = info->rec;
int rc;
- rc = __session_login_task(info->rec, info->qtask);
+ rc = __session_login_task(rec, info->qtask);
if (rc == ISCSI_ERR_HOST_NOT_FOUND) {
- if (info->retry_count == 5) {
+ if (info->retry_count == rec->conn[0].timeo.login_timeout) {
/* give up */
goto write_rsp;
}
- rc = queue_session_login_task_retry(info, info->rec,
- info->qtask);
+ rc = queue_session_login_task_retry(info, rec, info->qtask);
if (rc)
goto write_rsp;
/* we are going to internally retry */
diff --git a/usr/initiator.h b/usr/initiator.h
index c34625b..c11d77f 100644
--- a/usr/initiator.h
+++ b/usr/initiator.h
@@ -83,7 +83,6 @@ typedef enum iscsi_event_e {
EV_CONN_LOGOUT_TIMER,
EV_CONN_STOP,
EV_CONN_LOGIN,
- EV_UIO_POLL,
} iscsi_event_e;
struct queue_task;
diff --git a/usr/initiator_common.c b/usr/initiator_common.c
index eb03b23..98ca636 100644
--- a/usr/initiator_common.c
+++ b/usr/initiator_common.c
@@ -249,7 +249,7 @@ int iscsi_setup_portal(struct iscsi_conn *conn, char *address, int port)
return 0;
}
-int host_set_param(struct iscsi_transport *t,
+static int host_set_param(struct iscsi_transport *t,
uint32_t host_no, int param, char *value,
int type)
{
@@ -261,7 +261,7 @@ int host_set_param(struct iscsi_transport *t,
log_error("can't set operational parameter %d for "
"host %d, retcode %d (%d)", param, host_no,
rc, errno);
- return rc;
+ return ISCSI_ERR_INVAL;
}
return 0;
}
@@ -677,13 +677,13 @@ int iscsi_host_set_net_params(struct iface_rec *iface,
log_warning("Please set the iface.ipaddress for iface "
"%s, then retry the login command.\n",
iface->name);
- return EINVAL;
+ return ISCSI_ERR_INVAL;
} else if (t->template->set_host_ip == SET_HOST_IP_OPT) {
log_info("Optional iface.ipaddress for iface %s "
"not set.\n", iface->name);
return 0;
} else {
- return EINVAL;
+ return ISCSI_ERR_INVAL;
}
}
--
1.8.3.1
