Hi Mike:

I have started working more with iscsiuio.

I have discovered what I consider a bug in an error message printed during 
normal operation of iscsiadm that makes it seem like something bad happened.

As you know, when performing discovery via the bnx2i transport and the 
iscsiuio daemon, the iscsiadm command tries to set up host parameters when 
a target is discovered. The iscsiadm command does this by sending a message 
to the iscsiuio daemon, who actually does the work in this case and then 
normally returns that information.

But it looks like the design of iscsiuio is that it does not even try to 
set up the NIC it is using very first time its called. Instead, it 
initializes the NIC only when the first attempt to use it is received, 
while it returns EAGAIN to the caller. The protocol evidently is that the 
caller knows to retry. In this case, the iscsiadm code retries several 
times no matter what error is returned. In fact, the iscsiadm code being 
used translates all errors received from this attempt to talk to iscsiuio 
into an "INTERNAL_ERROR", and the communication is retried anyway.

This translation is the only problem, IMHO, and resulted in these messages 
from iscsiadm, which are misleading (and poorly formatted):

 iscsiadm: Could not set host net params (err 29)

 iscsiadm: Connection to discovery portal 10.125.128.120 failed: internal 
error
 ...

which becomes, with my patches:

 iscsiadm: Could not set host net params (err 29)
 iscsiadm: Connection to discovery portal 192.168.30.1 failed: operation 
failed but retry may succeed
 ...

The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it 
through. (And fix the error message printing so we don't core dump, of 
course.)

I thought about a more aggressive fix, i.e. letting all return values from 
the communications attempt through, but I worried about fixing something 
that is not really broken and perhaps breaking something else. So, in the 
spirit of fixing just what is broken, I submit two patches:

0001-Supply-strings-for-newly-added-error-numbers.patch: This patch is 
needed by the second patch, though it stands alone. This patch fixes the 
problem that a couple of new error codes have been added to the open-iscsi 
package but the array of error strings used to convert those errors into 
printed messages has not kept pace. So two missing error messages are added.

0002-Allow-setting-host-params-to-return-EAGAIN-errors.patch - This is the 
simple patch that allows EAGAIN errors through when trying to talk to 
iscsiuio. If you want the more aggressive version please let me know.

-- 
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 [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
>From 639308da324773203fc6ae019fd820de9765ac1f Mon Sep 17 00:00:00 2001
From: Lee Duncan <[email protected]>
Date: Wed, 10 Dec 2014 15:57:31 -0800
Subject: [PATCH] Supply strings for newly-added error numbers

A couple of new errors have been added to the
iscsi_err enum list, but iscsi_err_msgs[] was
not updated to match, so this adds messages for
the two newly-added errors to the list.

Signed-off-by: Lee Duncan <[email protected]>
---
 usr/iscsi_err.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/usr/iscsi_err.c b/usr/iscsi_err.c
index 4fe1c53adce0..1973e6fcd6a1 100644
--- a/usr/iscsi_err.c
+++ b/usr/iscsi_err.c
@@ -51,6 +51,8 @@ static char *iscsi_err_msgs[] = {
 	/* 26 */ "iSNS registration failed",
 	/* 27 */ "operation not supported",
 	/* 28 */ "device or resource in use",
+	/* 29 */ "operation failed but retry may succeed",
+	/* 30 */ "unknown discovery type",
 };
 
 char *iscsi_err_to_str(int err)
-- 
2.1.2

>From fc0e7b10b0ecaebbda85e48a9b188485c2b0ddaf Mon Sep 17 00:00:00 2001
From: Lee Duncan <[email protected]>
Date: Thu, 11 Dec 2014 11:17:25 -0800
Subject: [PATCH] Allow setting host params to return EAGAIN errors.

When iscsiadm is talking to iscsiuio, the first
attempt to set host params always fails with
EAGAIN and succeeds on the next try. The error
message was of:

 iscsiadm: Could not set host net params (err 29)

 iscsiadm: Connection to discovery portal 10.125.128.120 failed: internal error

becomes:

 iscsiadm: Could not set host net params (err 29)
 iscsiadm: Connection to discovery portal 192.168.30.1 failed: operataion failed but retry may succeed

Note that in both cases the target is discovered
correctly on the second try.

Signed-off-by: Lee Duncan <[email protected]>
---
 usr/discovery.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/usr/discovery.c b/usr/discovery.c
index 635ec8d9403d..565a919ba7e3 100644
--- a/usr/discovery.c
+++ b/usr/discovery.c
@@ -1109,9 +1109,10 @@ static int iscsi_create_leading_conn(struct iscsi_session *session)
 
 	rc = iscsi_host_set_net_params(iface, session);
 	if (rc) {
-		log_error("Could not set host net params (err %d)\n",
+		log_error("Could not set host net params (err %d)",
 			  rc);
-		rc = ISCSI_ERR_INTERNAL;
+		if (rc != ISCSI_ERR_AGAIN)
+			rc = ISCSI_ERR_INTERNAL;
 		goto close_ipc;
 	}
 
-- 
2.1.2

Reply via email to