On Wed, Aug 31, 2011 at 10:27 PM, Aastha Mehta <aasth...@gmail.com> wrote:
> Hello,
>
> Attached is the patch for the first kernel TODO item in the TODO list
> circulated earlier. I could not send the patch through git send-email, so
> have attached it here.

I'm new to the code base. Some comments inline:

>diff --git a/usr/initiator.c b/usr/initiator.c
>index 0350ff8..a2f412a 100644
[...]
>
>-      log_warning("Kernel reported iSCSI connection %d:%d error (%d) "
>-                  "state (%d)", session->id, conn->id, error,
>+      char *err_desc = err_code_to_string(error);
>+
>+      log_warning("Kernel reported iSCSI connection %d:%d error (%d) %s"
>+                  "state (%d)", session->id, conn->id, error, err_desc,
>                   conn->state);
>       iscsi_ev_context_put(ev_context);

You probably need a space after the "%s" for err_desc, and also it might be
nice to quote the error desc, to differentiate it from rest of the message. eg:

   log_warning("Kernel reported iSCSI connection %d:%d error (%d) '%s' "

>diff --git a/usr/kern_error_table.c b/usr/kern_error_table.c
>+
>+char *err_table[NUM_ERRORS] = {
>+      "ISCSI_OK: iscsi ok",
>+      "ISCSI_ERR_DATASN: Got invalid data sequence from iSCSI target",

AFAIU, this error is when we get an unexpected data sequence number.

>+      "ISCSI_ERR_DATALEN: Invalid R2T, unexpected datalen value",

This is not R2T specific, AFAICS.

>+      "ISCSI_ERR_BAD_ITT: Invalid invitation to transmit",

ITT is 'Initiator Task Tag'.

>+
>+char *err_code_to_string(int code){
>+      char *err_string;
>+      switch(code%1000){
>+              case 0: err_string = err_table[0];
>+                      break;

Wouldn't this be equivalent ? :
  err_string = err_table [code % ISCSI_ERR_BASE];

Also, probably check for error code for which the table has no entry
(eg. a new error code added but the table hasn't been updated) and
handle it accordingly. Also, I would add a comment in iscsi_if.h, suggesting
that this error table needs to be updated when adding new error codes.

Regards,
Ankit

-- 
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?hl=en.

Reply via email to