Re: PATCH: make open-iscsi userspace tools functionality available as a library
Bart Van Assche wrote: On Mon, Jan 19, 2009 at 4:35 PM, Hans de Goede hdego...@redhat.com wrote: - libiscsi_discover_sendtargets - maybe (very maybe) the int port could be dropped and const char *address could be of the form address_or_host[:port]. Regarding defaults and all that. Nah, that is ok for a cmdline interface, but cumbersome for an API, we could do something where port == 0 would choose the default port though. Have you considered to replace both const char *address and int port by const struct sockaddr *address, socklen_t address_len ? This would avoid having to figure out inside libiscsi_discover_sendtargets() whether the passed address is in IPv4 or in IPv6 format (if numeric). libiscsi builds on top of the existing open-iscsi userspace code, in this code the const char *address and int port notation is used everywhere. Regards, Hans --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Q: CHAP and Linux
Hello, this is a quote from HP StorageWorks EVA iSCSI Connectivity Option 7.1 release notes: mpx100 and CHAP security with Linux not supported The CHAP setup with the Linux iSCSI initiator 3.6.3 is not supported with the mpx100. The Linux iSCSI driver omits CHAP security negotiations at login. Workaround The CHAP setup will be resolved in a future release of the Linux initiator. The release notes are dated Twelfth edition: October 2008. Can anybody say what the problem is (or was)? I feel that there is a different issue with the MPX100 product (rather than the one quoted). Regards, Ulrich --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
[PATCH] iscsi tcp: bidi capable
From: Pete Wyckoff p...@padd.com
Mark iscsi_tcp as being capable of bidirectional transfers. The
bsg interface checks this bit before attempting any bidirectional
commands.
Signed-off-by: Pete Wyckoff p...@padd.com
Signed-off-by: Boaz Harrosh bharr...@panasas.com
---
drivers/scsi/iscsi_tcp.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index af092a8..9e2d4fb 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -806,6 +806,12 @@ static void iscsi_sw_tcp_session_destroy(struct
iscsi_cls_session *cls_session)
iscsi_host_free(shost);
}
+static int iscsi_tcp_slave_alloc(struct scsi_device *sdev)
+{
+ set_bit(QUEUE_FLAG_BIDI, sdev-request_queue-queue_flags);
+ return 0;
+}
+
static int iscsi_sw_tcp_slave_configure(struct scsi_device *sdev)
{
blk_queue_bounce_limit(sdev-request_queue, BLK_BOUNCE_ANY);
@@ -826,6 +832,7 @@ static struct scsi_host_template iscsi_sw_tcp_sht = {
.eh_device_reset_handler= iscsi_eh_device_reset,
.eh_target_reset_handler= iscsi_eh_target_reset,
.use_clustering = DISABLE_CLUSTERING,
+ .slave_alloc= iscsi_tcp_slave_alloc,
.slave_configure= iscsi_sw_tcp_slave_configure,
.proc_name = iscsi_tcp,
.this_id= -1,
--
1.6.0.1
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---
Re: [PATCH] iscsi tcp: bidi capable
Boaz Harrosh wrote: From: Pete Wyckoff p...@padd.com Mark iscsi_tcp as being capable of bidirectional transfers. The bsg interface checks this bit before attempting any bidirectional commands. Signed-off-by: Pete Wyckoff p...@padd.com Signed-off-by: Boaz Harrosh bharr...@panasas.com --- snip Mike hi, whatever happened with this patch? It is over scsi-misc but I think it should be good for iscsi tree. (Sorry) Since open-osd is apparently only going into next Kernel, sigh. Then this is fine for the next Kernel as well. (I was advertising BIDI kernels since 2.6.26, well I guess not from bsg+iscsi) Thanks Boaz --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: PATCH: make open-iscsi userspace tools functionality available as a library
NACK.
I presume you have run this program (and the test-code) through
valgrind with no memory leaks?
Please see my comments below.
diff -urN open-iscsi-2.0-870.1.orig/libiscsi/libiscsi.c
open-iscsi-2.0-870.1/libiscsi/libiscsi.c
--- open-iscsi-2.0-870.1.orig/libiscsi/libiscsi.c 1970-01-01
01:00:00.0 +0100
+++ open-iscsi-2.0-870.1/libiscsi/libiscsi.c 2009-01-19 12:28:08.0
+0100
@@ -0,0 +1,354 @@
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include errno.h
+#include unistd.h
+#include libiscsi.h
+#include idbm.h
+#include iscsiadm.h
+#include log.h
+#include sysfs.h
+#include iscsi_sysfs.h
+#include util.h
+#include sysdeps.h
+#include iface.h
Do these guys need to be in quotes? In the Makefile you did specify the header
directory so I would think that would work.
Also it might make sense to order these guys in alphabetical order.
+
+#define CHECK(a) { rc = a; if (rc) goto leave; }
+
+struct libiscsi_context {
+ char libiscsi_error_string[256];
Use a #define for the size.
+};
+
+struct libiscsi_context *libiscsi_init(void)
+{
+ struct libiscsi_context *context;
+
+ context = calloc(sizeof *context, 1);
Swap the arguments around. First argument is the number of elements, while the
second
is the size of the element.
+ if (!context)
+ return NULL;
+
+ /* enable stdout logging */
+ log_daemon = 0;
+ log_init(libiscsi, 1024);
+ sysfs_init();
+ increase_max_files();
+ if (idbm_init(NULL)) {
+ free(context);
No sysfs_cleanup() call? Or log_close()?
+ return NULL;
+ }
+
+ iface_setup_host_bindings();
+
+ return context;
+}
+
... snip..
+int libiscsi_discover_sendtargets(struct libiscsi_context *context,
+const char *address, int port, const struct libiscsi_auth_info
*auth_info,
+struct libiscsi_node **found_nodes)
.. snip ..
+ if (!auth_info-chap.username[0])
Would it make sense to add logging here as well? Like
'log_warning(Non-existent CHAP username!)' or such?
+ return -EINVAL;
+ if (!auth_info-chap.password[0])
+ return -EINVAL;
Is against the CHAP to have an empty password? Ah n/m. The
http://www.ietf.org/rfc/rfc1994.txt says: The CHAP algorithm requires
that the length of the secret MUST be at least 1 octet. Maybe
put a check for that?
BTW, I am not sure if the PPP CHAP RFC == iSCSI CHAP, so I
might be spewing non-sense here.
+ if (auth_info-chap.reverse_username[0]
+ !auth_info-chap.reverse_password[0])
+ return -EINVAL;
+
+ drec.u.sendtargets.auth.authmethod = AUTH_METHOD_CHAP;
+ strlcpy(drec.u.sendtargets.auth.username,
+ auth_info-chap.username, AUTH_STR_MAX_LEN);
+ strlcpy((char *)drec.u.sendtargets.auth.password,
+ auth_info-chap.password, AUTH_STR_MAX_LEN);
+ drec.u.sendtargets.auth.password_length =
+ strlen((char *)drec.u.sendtargets.auth.password);
This doesn't guard against passwords that are of AUTH_STR_MAX_LEN length. Which
means that there might not be an \0 at then end and your password_length
computation ends up looking at the next structure fields. Or worst (shudder)
Unlikely, but it could happen - and the check is easy enough:
drec.u.sendtargets.auth.password_length =
drec.u.sendtargets.auth.password_length AUTH_STR_MAX_LEN ? AUTH_STR_MAX_LEN
: drec.u.sendtargets.auth.password_length;
Maybe even do that before any 'strlcpy' is done and use this newly
computed length instead of the defines?
+ strlcpy(drec.u.sendtargets.auth.username_in,
+ auth_info-chap.reverse_username, AUTH_STR_MAX_LEN);
+ strlcpy((char *)drec.u.sendtargets.auth.password_in,
+ auth_info-chap.reverse_password, AUTH_STR_MAX_LEN);
+ drec.u.sendtargets.auth.password_in_length =
+ strlen((char *)drec.u.sendtargets.auth.password_in);
And the same check here..
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ CHECK(discovery_sendtargets(drec, new_rec_list))
+ CHECK(idbm_add_discovery(drec, 1))
Make the 1 a #define? Without me looking at the code I had
no idea that 1 stands for 'over-write'. Thought at first it
was the count of records - which looked weird.
+
+ /* bind ifaces to node recs so we know what we have */
+ list_for_each_entry(rec, new_rec_list, list)
+ CHECK(idbm_bind_ifaces_to_node(rec, NULL, bound_rec_list))
+
+ /* now add/update records */
+ list_for_each_entry(rec, bound_rec_list, list) {
+ CHECK(idbm_add_node(rec, drec, 1))
Ditto.
+ found++;
+ }
+
+ if (found_nodes found) {
+ *found_nodes = calloc(found, sizeof
Re: PATCH: make open-iscsi userspace tools functionality available as a library
I would recommend that you provide as the first variable in all of the structs an unsigned int called 'version'. This way if the structs are extended they would be backwards compatible and there is an easy way to identify which version of structs they are. Erm, given the amount of programs which will probably end up using this (not all that much) a distro should be able to easily rebuild all those in case of an ABI change and thus a soname bump. I understand what you are trying to say here, but IMHO the added complexity and ugliness is not worth it. I disagree. The ABI in a distro is a holy thing. When a Linux distro releases their libraries (look for example at libvirt), they can't modify it for the next 7 years (sure they can do it underneath the covers, but look at the Red Hat kernel with those #ifdef __GENKSYSM__ to work-around this). I am curious to understand what is complex and ugly about it? Could you be more specific please. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: PATCH: make open-iscsi userspace tools functionality available as a library
On Tue, Jan 20, 2009 at 7:20 PM, Konrad Rzeszutek kon...@virtualiron.com wrote: I would recommend that you provide as the first variable in all of the structs an unsigned int called 'version'. This way if the structs are extended they would be backwards compatible and there is an easy way to identify which version of structs they are. Erm, given the amount of programs which will probably end up using this (not all that much) a distro should be able to easily rebuild all those in case of an ABI change and thus a soname bump. I understand what you are trying to say here, but IMHO the added complexity and ugliness is not worth it. I disagree. The ABI in a distro is a holy thing. When a Linux distro releases their libraries (look for example at libvirt), they can't modify it for the next 7 years (sure they can do it underneath the covers, but look at the Red Hat kernel with those #ifdef __GENKSYSM__ to work-around this). I am curious to understand what is complex and ugly about it? Could you be more specific please. I agree that having a stable binary interface is very important for a shared library. But if glibc doesn't have versioned structs, why would these be needed in an iscsi library ? Please keep in mind that it is possible to define a new version of a shared library in case the ABI changes. Bart. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: PATCH: make open-iscsi userspace tools functionality available as a library
Hi,
Konrad Rzeszutek wrote:
Thanks for the review!
I presume you have run this program (and the test-code) through
valgrind with no memory leaks?
Erm, no, has iscsiadm been run through valgrind? If not I'm not going to be
running libiscsi through it either (sorry) libiscsi builds on top of many
open-iscsi userspace code internals, and those are not always pretty (many
global variables for example).
Please see my comments below.
diff -urN open-iscsi-2.0-870.1.orig/libiscsi/libiscsi.c
open-iscsi-2.0-870.1/libiscsi/libiscsi.c
--- open-iscsi-2.0-870.1.orig/libiscsi/libiscsi.c1970-01-01
01:00:00.0 +0100
+++ open-iscsi-2.0-870.1/libiscsi/libiscsi.c2009-01-19
12:28:08.0 +0100
@@ -0,0 +1,354 @@
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include errno.h
+#include unistd.h
+#include libiscsi.h
+#include idbm.h
+#include iscsiadm.h
+#include log.h
+#include sysfs.h
+#include iscsi_sysfs.h
+#include util.h
+#include sysdeps.h
+#include iface.h
Do these guys need to be in quotes? In the Makefile you did specify the
header
directory so I would think that would work.
Using might work too, but this way it is clear this are open-iscsi headers,
and not system headers.
+
+#define CHECK(a) { rc = a; if (rc) goto leave; }
+
+struct libiscsi_context {
+char libiscsi_error_string[256];
Use a #define for the size.
Why? Once we will actually start using this, it will be filled with snprintf,
with a sizeof as second parameter, so I see no reason to use a define.
+};
+
+struct libiscsi_context *libiscsi_init(void)
+{
+struct libiscsi_context *context;
+
+context = calloc(sizeof *context, 1);
Swap the arguments around. First argument is the number of elements,
while the second
is the size of the element.
You are right, will fix.
+if (!context)
+return NULL;
+
+/* enable stdout logging */
+log_daemon = 0;
+log_init(libiscsi, 1024);
+sysfs_init();
+increase_max_files();
+if (idbm_init(NULL)) {
+free(context);
No sysfs_cleanup() call? Or log_close()?
log_close() does not exist (weird open-iscsi internals), but yes it needs a
sysfs_cleanup() call.
+return NULL;
+}
+
+iface_setup_host_bindings();
+
+return context;
+}
+
... snip..
+int libiscsi_discover_sendtargets(struct libiscsi_context *context,
+const char *address, int port, const struct libiscsi_auth_info
*auth_info,
+struct libiscsi_node **found_nodes)
.. snip ..
+if (!auth_info-chap.username[0])
Would it make sense to add logging here as well? Like
'log_warning(Non-existent CHAP username!)' or such?
Yes, that is the plan when I actually get error logging implemented, see the
comments about this in my initial mail (I'm waiting on feedback on some
proposed changes to the open-iscsi internal logging API).
+return -EINVAL;
+if (!auth_info-chap.password[0])
+return -EINVAL;
Is against the CHAP to have an empty password? Ah n/m. The
http://www.ietf.org/rfc/rfc1994.txt says: The CHAP algorithm requires
that the length of the secret MUST be at least 1 octet. Maybe
put a check for that?
BTW, I am not sure if the PPP CHAP RFC == iSCSI CHAP, so I
might be spewing non-sense here.
+if (auth_info-chap.reverse_username[0]
+!auth_info-chap.reverse_password[0])
+return -EINVAL;
+
+drec.u.sendtargets.auth.authmethod = AUTH_METHOD_CHAP;
+strlcpy(drec.u.sendtargets.auth.username,
+auth_info-chap.username, AUTH_STR_MAX_LEN);
+strlcpy((char *)drec.u.sendtargets.auth.password,
+auth_info-chap.password, AUTH_STR_MAX_LEN);
+drec.u.sendtargets.auth.password_length =
+strlen((char *)drec.u.sendtargets.auth.password);
This doesn't guard against passwords that are of AUTH_STR_MAX_LEN
length.
It does, first the passed in string gets strlcpy-ed to
drec.u.sendtargets.auth.password, strlcpy will copy at max AUTH_STR_MAX_LEN - 1
bytes and will always 0 terminate the dest.
Then the lenght of drec.u.sendtargets.auth.password gets used, not the length
of what was passed in but the length of the (limited length) copy.
snip
+break;
+default:
+return -EINVAL;
+}
+
+CHECK(discovery_sendtargets(drec, new_rec_list))
+CHECK(idbm_add_discovery(drec, 1))
Make the 1 a #define? Without me looking at the code I had
no idea that 1 stands for 'over-write'. Thought at first it
was the count of records - which looked weird.
Internal open-iscsi API, when it becomes a define there I'll happily use it.
+
+
Re: PATCH: make open-iscsi userspace tools functionality available as a library
On Tue, Jan 20, 2009 at 07:40:08PM +0100, Bart Van Assche wrote: On Tue, Jan 20, 2009 at 7:20 PM, Konrad Rzeszutek kon...@virtualiron.com wrote: I would recommend that you provide as the first variable in all of the structs an unsigned int called 'version'. This way if the structs are extended they would be backwards compatible and there is an easy way to identify which version of structs they are. Erm, given the amount of programs which will probably end up using this (not all that much) a distro should be able to easily rebuild all those in case of an ABI change and thus a soname bump. I understand what you are trying to say here, but IMHO the added complexity and ugliness is not worth it. I disagree. The ABI in a distro is a holy thing. When a Linux distro releases their libraries (look for example at libvirt), they can't modify it for the next 7 years (sure they can do it underneath the covers, but look at the Red Hat kernel with those #ifdef __GENKSYSM__ to work-around this). I am curious to understand what is complex and ugly about it? Could you be more specific please. I agree that having a stable binary interface is very important for a shared library. But if glibc doesn't have versioned structs, why would these be needed in an iscsi library ? Please keep in mind that it is possible to define a new version of a shared library in case the ABI changes. Yeah. I just thought about that when I sent the e-mail about libvirt. I think that is a good compromise - however we still need the ground-work in place so that this library is built as libiscsi.so.1.0.0 and that the linker saves the version information. I guess this implies that the .spec file needs to be changed to include this file as well. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: Q: CHAP and Linux
Ulrich Windl wrote: Hello, this is a quote from HP StorageWorks EVA iSCSI Connectivity Option 7.1 release notes: mpx100 and CHAP security with Linux not supported The CHAP setup with the Linux iSCSI initiator 3.6.3 is not supported with the mpx100. The Linux iSCSI driver omits CHAP security negotiations at login. I think this is referring to a old initiator (linux-iscsi based), because that 3.6.3 is a old linux-iscsi version. I talked to some HP engineer when this issue came up the other month and they said it works fine with open-iscsi. Could you describe again what is going wrong? Does discovery work with CHAP? Does CHAP work with normal sessions? What error are you getting? Are you setting the node.session.auth.* and the discovery.sendtargets.auth.* --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: Q: on ISIDs and TSIDs
Ulrich Windl wrote: Hi, there are Initiator Session IDs (ISID) and Target Session IDs (TSID) in the iSCSI specification. iscsiadm -m session -i (from SLES10 SP1) displays: Session (sid 6): is that the ISID? No. It is just the kernel's identifier for the session. Host Number: 9 : is that the TSID? No. It is the kernel's identifier for the host that the session is on. If both is true, I'd suggest to adjust the wording to be closer to the specification. If I'm wrong, can I display TSIDs and ISIDs? No. It is not currently displayed. I can add it. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: PATCH: make open-iscsi userspace tools functionality available as a library
On Tue, Jan 20, 2009 at 07:58:07PM +0100, Hans de Goede wrote:
Hi,
Konrad Rzeszutek wrote:
Thanks for the review!
I presume you have run this program (and the test-code) through
valgrind with no memory leaks?
Erm, no, has iscsiadm been run through valgrind? If not I'm not going to be
running libiscsi through it either (sorry) libiscsi builds on top of many
I meant the test-cases.
open-iscsi userspace code internals, and those are not always pretty (many
global variables for example).
I believe valgrind marks those as 'possible memory-leaks'
.. snip ..
+#include iface.h
Do these guys need to be in quotes? In the Makefile you did specify the
header
directory so I would think that would work.
Using might work too, but this way it is clear this are open-iscsi
headers,
and not system headers.
Sounds fair.
.. snip ..
+CHECK(discovery_sendtargets(drec, new_rec_list))
+CHECK(idbm_add_discovery(drec, 1))
Make the 1 a #define? Without me looking at the code I had
no idea that 1 stands for 'over-write'. Thought at first it
was the count of records - which looked weird.
Internal open-iscsi API, when it becomes a define there I'll happily use it.
How about a comment instead then.
+
+/* bind ifaces to node recs so we know what we have */
+list_for_each_entry(rec, new_rec_list, list)
+CHECK(idbm_bind_ifaces_to_node(rec, NULL, bound_rec_list))
+
+/* now add/update records */
+list_for_each_entry(rec, bound_rec_list, list) {
+CHECK(idbm_add_node(rec, drec, 1))
Ditto.
Ditto.
Ditto :-)
+found++;
+}
+
+if (found_nodes found) {
+*found_nodes = calloc(found, sizeof **found_nodes);
Please swap the arguments.
Erm, no, this time I got it right the first time.
Right.
.. snip ..
+int login_helper(void *data, node_rec_t *rec)
+{
+int rc = iscsid_req_by_rec(MGMT_IPC_SESSION_LOGIN, rec);
+if (rc) {
+iscsid_handle_error(rc);
+rc = ENOTCONN;
Should that have a - in front of it? Hmm.. I thought Mike wanted
all of the return codes to be a positive number. Which means all
the other functions you have should _NOT_ have the '-'?
Looking at the 'idbm_*' all of its return codes are positive. Perhaps
a global search and replace for the -E to E?
The only functions returning negative codes are the discovery functions, as
I think having the same mechanism and behavior to return error codes will
save folks from trouble down the line. Having all functions return a positive
error codes.
they return the number of nodes found on success. If people dislike this I
can
return the number of found nodes through a pointer instead.
+}
+return rc;
+}
+
+int libiscsi_node_login(struct libiscsi_context *context,
+const struct libiscsi_node *node)
+{
+int nr_found = 0, rc;
+
+CHECK(idbm_for_each_iface(nr_found, NULL, login_helper,
+(char *)node-name, node-tpgt,
+(char *)node-address, node-port))
+if (nr_found == 0)
+rc = ENODEV;
+leave:
+return rc;
+}
+
+static int logout_helper(void *data, struct session_info *info)
+{
+int rc;
+struct node_rec *rec = data;
+
+if (!iscsi_match_session(rec, info))
+return -1; /* Not a match */
Oh boy. Can you put a comment of why you are mixing standard error
codes with negative numbers? At first I thought this was an error
until I looked in 'iscsi_sysfs_for_each_session' and found:
if less than zero it means it was not a match
Can you just say that 'iscsi_sysfs_for_each_session' requires this
type of return code for not a match please?
You mean make the comment more verbose, sure if that makes you happy :) This
Please do.
are those infamous open-iscsi internals again, which I'm trying really hard
to
hide from the outside (iow when only looking at libiscsi.h) once you start
looking at libiscsi.c, well ...
.. snip ..
+static int get_parameter_helper(void *data, node_rec_t *rec)
+{
+struct db_set_param *param = data;
+recinfo_t *info;
+int i;
+
+info = idbm_recinfo_alloc(MAX_KEYS);
+if (!info)
+return ENOMEM;
+
+idbm_recinfo_node(rec, info);
+
+for (i = 0; i MAX_KEYS; i++) {
+if (!info[i].visible)
+continue;
+
+if (strcmp(param-name, info[i].name))
How about 'strncmp' ?
Why?
Knee-jerk reaction. In this case I can see you don't do anything
else beside 'continue' so it doesn't matter.
+continue;
+
+strlcpy(param-value, info[i].value,
[RFC] manage qla4xxx iscsi sessions with iscsiadm
Hey Dave,
In the qla4xxx branch of the iscsi git tree I added code so that
iscsiadm can control sessions that are accessed through qla4xxx.
The patches are here:
http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6-iscsi.git;a=commit;h=f95246f8e84a12267e3c9e811a94004f598ef285
http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6-iscsi.git;a=commit;h=ea4d8306487fca1b601a2ef69a85951b19843382
http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6-iscsi.git;a=commit;h=4320dc0e234368620b40c155d571e8c46b814216
http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6-iscsi.git;a=commit;h=76ed2193986e9910fd695c881ae1cb08e35feb1c
I was not sure what I was doing exactly :) since I was going by the
qlogic.com driver ioctl code, so if you could take a look at
http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6-iscsi.git;a=commit;h=ea4d8306487fca1b601a2ef69a85951b19843382
and check if I am interacting with the hardware correctly it would be
nice. Specifically I am worried about qla4xxx_session_logout_ddb and
qla4xxx_session_login_ddb.
If you want to run this then you also need the userspace patch that is
attached. It should apply to the open-iscsi git head.
To start remove the persistent targets in your FLASH since we will use
the iscsd db instead (note that for boot and root you can have targets
in flash and the drivers/firmware auto login will work but you just
cannot use iscsiadm to login/logout).
To run login/logout then just build the code, modprobe qla4xxx and start
iscsid. Then you need to do discovery
./iscsiadm -m iface -P 3
will display the different ifaces like this:
Iface: iface0
Iface Name: iface0
Iface Transport: qla4xxx
Iface Initiatorname: unknown
Iface IPaddress: unknown
Iface HWaddress: 00:c0:dd:08:63:e8
Iface Netdev: unknown
Host Number: 20 State: running
Iface: iface1
Iface Name: iface1
Iface Transport: qla4xxx
Iface Initiatorname: unknown
Iface IPaddress: unknown
Iface HWaddress: 00:c0:dd:08:63:ea
Iface Netdev: unknown
Host Number: 21 State: running
For this I am going to use the one with mac 00:c0:dd:08:63:e8 (iface0)
./iscsiadm -m discovery -t st -p max -I iface0 -P 1
Target: iqn.2001-04.com.max.1
Portal: 20.15.0.4:3260,1
Iface Name: iface0
Target: iqn.2001-04.com.max.2
Portal: 20.15.0.4:3260,1
Iface Name: iface0
Target: iqn.2001-04.com.max.3
Portal: 20.15.0.4:3260,1
Iface Name: iface0
Then I will log in:
./iscsiadm -m node -T iqn.2001-04.com.max.3 -l
(you can also pass in the portal if you have multiple portals on a
target and only want to log into one).
You should then see a error or a login ok message.
You can then do
./iscsiadm -m session -P 3
to see some values (I also added code to be able to set the segment
limtis and max r2ts (does qla4xxx firmware support more than 1 btw.
And then to logout do
./iscsiadm -m node T iqn.2001-04.com.max.3 -u
and you should get a error or a ok message.
Note that you cannot control sessions that are the persistent in the
firmware/flash. So if you have targets in FLASH then the iscsiadm logout
and login commands are not supported. qla4xxx will still log into them
and find disks so that boot and / work, but you cannot login/logout with
iscsiadm (you actually can if you have targets setup in FLASH, then load
the module, then start iscsid, but that is by accident).
Some questions on qlogic hardware:
- How do you set CHAP settings for each target?
- How do you set Data/Header digests settings?
- Currently for iscsiadm -m disocvery you need a seperate NIC. It
does not go through the card. Is there a way to ask the card to do
sendtargets and then just have it tell us what targets were found (I do
not want the card to login).
Can I just send a discovery session login pdu and sendtargets command in
a passthrough interface, and then get back raw pdus?
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---
diff --git a/include/iscsi_if.h b/include/iscsi_if.h
index afa86e8..effd004 100644
--- a/include/iscsi_if.h
+++ b/include/iscsi_if.h
@@ -335,6 +335,10 @@ enum iscsi_host_param {
#define CAP_FW_DB 0x200
#define CAP_SENDTARGETS_OFFLOAD0x400
#define CAP_DATA_PATH_OFFLOAD 0x800
+#define CAP_DIGEST_OFFLOAD 0x1000 /* offload hdr and data digests */
+#define CAP_PADDING_OFFLOAD0x2000 /* offload padding insertion, removal,
+ and verification */
+#define CAP_LOGIN_OFFLOAD
RE: [RFC] manage qla4xxx iscsi sessions with iscsiadm
Mike Christie wrote: Thanks Mike. I will take review and get back. -david S. Hey Dave, In the qla4xxx branch of the iscsi git tree I added code so that iscsiadm can control sessions that are accessed through qla4xxx. The patches are here: http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6- iscsi.git;a=commit;h=f95246f8e84a12267e3c9e811a94004f598ef285 http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6- iscsi.git;a=commit;h=ea4d8306487fca1b601a2ef69a85951b19843382 http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6- iscsi.git;a=commit;h=4320dc0e234368620b40c155d571e8c46b814216 http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6- iscsi.git;a=commit;h=76ed2193986e9910fd695c881ae1cb08e35feb1c I was not sure what I was doing exactly :) since I was going by the qlogic.com driver ioctl code, so if you could take a look at http://git.kernel.org/?p=linux/kernel/git/mnc/linux-2.6- iscsi.git;a=commit;h=ea4d8306487fca1b601a2ef69a85951b19843382 and check if I am interacting with the hardware correctly it would be nice. Specifically I am worried about qla4xxx_session_logout_ddb and qla4xxx_session_login_ddb. If you want to run this then you also need the userspace patch that is attached. It should apply to the open-iscsi git head. To start remove the persistent targets in your FLASH since we will use the iscsd db instead (note that for boot and root you can have targets in flash and the drivers/firmware auto login will work but you just cannot use iscsiadm to login/logout). To run login/logout then just build the code, modprobe qla4xxx and start iscsid. Then you need to do discovery ./iscsiadm -m iface -P 3 will display the different ifaces like this: Iface: iface0 Iface Name: iface0 Iface Transport: qla4xxx Iface Initiatorname: unknown Iface IPaddress: unknown Iface HWaddress: 00:c0:dd:08:63:e8 Iface Netdev: unknown Host Number: 20 State: running Iface: iface1 Iface Name: iface1 Iface Transport: qla4xxx Iface Initiatorname: unknown Iface IPaddress: unknown Iface HWaddress: 00:c0:dd:08:63:ea Iface Netdev: unknown Host Number: 21 State: running For this I am going to use the one with mac 00:c0:dd:08:63:e8 (iface0) ./iscsiadm -m discovery -t st -p max -I iface0 -P 1 Target: iqn.2001-04.com.max.1 Portal: 20.15.0.4:3260,1 Iface Name: iface0 Target: iqn.2001-04.com.max.2 Portal: 20.15.0.4:3260,1 Iface Name: iface0 Target: iqn.2001-04.com.max.3 Portal: 20.15.0.4:3260,1 Iface Name: iface0 Then I will log in: ./iscsiadm -m node -T iqn.2001-04.com.max.3 -l (you can also pass in the portal if you have multiple portals on a target and only want to log into one). You should then see a error or a login ok message. You can then do ./iscsiadm -m session -P 3 to see some values (I also added code to be able to set the segment limtis and max r2ts (does qla4xxx firmware support more than 1 btw. And then to logout do ./iscsiadm -m node T iqn.2001-04.com.max.3 -u and you should get a error or a ok message. Note that you cannot control sessions that are the persistent in the firmware/flash. So if you have targets in FLASH then the iscsiadm logout and login commands are not supported. qla4xxx will still log into them and find disks so that boot and / work, but you cannot login/logout with iscsiadm (you actually can if you have targets setup in FLASH, then load the module, then start iscsid, but that is by accident). Some questions on qlogic hardware: - How do you set CHAP settings for each target? - How do you set Data/Header digests settings? - Currently for iscsiadm -m disocvery you need a seperate NIC. It does not go through the card. Is there a way to ask the card to do sendtargets and then just have it tell us what targets were found (I do not want the card to login). Can I just send a discovery session login pdu and sendtargets command in a passthrough interface, and then get back raw pdus? --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
