On Thu, Sep 8, 2011 at 2:26 PM, Aastha Mehta <[email protected]> wrote:
> Sorry, please ignore the previous patch. There was some minor error in that.
> Re-attaching the patch here.
+#include<stdio.h>
+#include<stdlib.h>
+#include<string.h>
+#include "iscsi_if.h"
+#include"kern_error_table.h"
+#define NUM_ERRORS 22
You can probably clean up the includes, and the #define.
+ log_warning("error %d: %s ", error, err_desc);
Maybe do:
log_warning("error %d: %s ", error, err_code_to_string(error);
+ switch(code % ISCSI_ERR_BASE){
+ case 0: return "ISCSI_OK: operation successful";
+ case 1: return "ISCSI_ERR_DATASN: Received invalid data
sequence
number from target";
Since you are returning the error strings here directly, you don't need
"code % ISCSI_ERR_BASE" and can use ISCSI_ERR_* enum values
in the case statements directly. That would be clearer than using just
numbers.
+ default: return "Invalid error code. Is there a new error code
added which is not handled?";
"Invalid" or "Unknown" ? This is probably nitpicking :/
-Ankit
> Thanks,
> Aastha.
>
> On 8 September 2011 01:44, Aastha Mehta <[email protected]> wrote:
>>
>> Hi,
>> Thanks for the feedback. I have included changes mentioned by different
>> people. Attaching patch again. Please review it once more.
>>
>> Thanks,
>> Aastha.
>>
>>
>> On 2 September 2011 13:27, Ankit Jain <[email protected]> wrote:
>>>
>>> On Wed, Aug 31, 2011 at 10:27 PM, Aastha Mehta <[email protected]>
>>> 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 [email protected].
>>> To unsubscribe from this group, send email to
>>> [email protected].
>>> For more options, visit this group at
>>> http://groups.google.com/group/open-iscsi?hl=en.
>>>
>>
>>
>>
>> --
>> Aastha Mehta
>> B.E. (Hons.) Computer Science
>> BITS Pilani
>> E-mail: [email protected]
>>
>>
>
>
>
> --
> Aastha Mehta
> B.E. (Hons.) Computer Science
> BITS Pilani
> E-mail: [email protected]
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "open-iscsi" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to
> [email protected].
> For more options, visit this group at
> http://groups.google.com/group/open-iscsi?hl=en.
>
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/open-iscsi?hl=en.
