[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 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