On Mon, Feb 11 2008 at 17:43 +0200, Pete Wyckoff <[EMAIL PROTECTED]> wrote:
> [EMAIL PROTECTED] wrote on Thu, 31 Jan 2008 22:29 +0200:
>>   iscsi bidi support at the generic libiscsi level
>>   - prepare the additional bidi_read rlength header.
>>   - access the right scsi_in() and/or scsi_out() side of things.
>>     also for resid.
>>   - Handle BIDI underflow overflow from target
>>
>> Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
> 
> I see you do this a bit differently than in your previous patch set.
> In particular, the residual handling in libiscsi.c.  (I'm editing in
> a bit more context to the patch below.)
> 
>> +    if (scsi_bidi_cmnd(sc) &&
>> +            (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
>> +                                    ISCSI_FLAG_CMD_BIDI_OVERFLOW))) {
>> +            int res_count = be32_to_cpu(rhdr->bi_residual_count);
>> +
>> +            if (res_count > 0 &&
>> +                    (rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW ||
>> +                    res_count <= scsi_in(sc)->length))
>> +                    scsi_in(sc)->resid = res_count;
>> +            else
>> +                    sc->result =
>> +                            (DID_BAD_TARGET << 16) | rhdr->cmd_status;
>> +    }
>> +
>>      if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW |
>>                         ISCSI_FLAG_CMD_OVERFLOW)) {
>>              int res_count = be32_to_cpu(rhdr->residual_count);
>>
>>              if (res_count > 0 &&
>>                  (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
>>                   res_count <= scsi_bufflen(sc)))
>> +                    /* write side for bidi or uni-io set_resid */
>>                      scsi_set_resid(sc, res_count);
>>              else
>>                      sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
>>       } else if (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
>>                                 ISCSI_FLAG_CMD_BIDI_OVERFLOW))
>>               sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
> 
> I haven't tested this, but, consider a bidi command that results in
> an overflow on the read part, and no overflow on the write part.
> E.g., the user supplied a data-in buffer that wasn't big enough to
> hold the returned data from the target, but the data-out buffer was
> just right.
> 
> Then this code will set scsi_in(sc)->resid properly, informing the
> caller that there were extra bytes that were not transferred.  But
> the "else" clause at the bottom will also set sc->result to be bad.
> I don't think we want this.
> 
> Your earlier patch got rid of the second bidi_overflow handler and
> just did all the logic for both bidi and non-bidi cases in a single
> if clause.  Seemed better.
> 
>               -- Pete
You are most probably right I will investigate what happened. It looks
like I went back to some old version right? or a merge fallout
Thanks for reviewing.

Please also test latest head-of-line code if possible + iscsi patches
+ last varlen I sent.

Is there any new patches I need for 2.6.24 or head-of-line for my 
osd-dev tree?

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 [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---

Reply via email to