[stgt] [PATCH] Fix ModeSense. ModeSense never returns residuals. Additionally fix the two arguments that were swapped in set_byte_safe.
ronniesahlberg at gmail.com
Sat Jun 1 17:07:55 CEST 2013
You are right, I misread/misremembered rfc5048/3.1
I have re-sent a patch that only fixes the swapped parameters.
I added a test for residuals and they work correctly as is :
$ ./bin/iscsi-test-cu iscsi://127.0.0.1/iqn.ronnie.test/1 --test
CUnit - A Unit testing framework for C - Version 2.1-0
Test: Residuals ...
Test of MODESENSE6 Residuals
MODESENSE6 command should not result in any residuals
Try a MODESENSE6 command with 4 bytes of transfer length and
verify that we dont get residuals.
[SUCCESS] All Pages fetched.
Verify that we got at most 4 bytes of DATA-IN
[SUCCESS] <= 4 bytes of DATA-IN received.
Verify residual overflow flag not set
Try a MODESENSE6 command with 255 bytes of transfer length and
verify that we get residuals if the target returns less than the
requested amount of data.
[SUCCESS] All Pages fetched.
We got less than the requested 255 bytes from the target. Verify
that underflow is set.
[SUCCESS] Residual underflow is set
--Run Summary: Type Total Ran Passed Failed
suites 1 1 n/a 0
tests 1 1 1 0
asserts 3 3 3 0
Tests completed with return value: 0
On Fri, May 31, 2013 at 4:50 AM, Alexander Nezhinsky
<nezhinsky at gmail.com> wrote:
> On Fri, May 31, 2013 at 8:30 AM, Ronnie Sahlberg
> <ronniesahlberg at gmail.com> wrote:
>> diff --git a/usr/spc.c b/usr/spc.c
>> index 074fdad..197e611 100644
>> --- a/usr/spc.c
>> +++ b/usr/spc.c
>> @@ -622,7 +622,7 @@ static int build_mode_page(uint8_t *data, struct mode_pg *pg,
>> * Set a byte at the given index within dst buffer to val,
>> * not exceeding dst_len bytes available at dst.
>> -void set_byte_safe(uint8_t *dst, uint32_t dst_len, uint32_t index, int val)
>> +void set_byte_safe(uint8_t *dst, uint32_t index, uint32_t dst_len, int val)
>> if (index < dst_len)
>> dst[index] = (uint8_t)val;
> Oops, you are right! My fault, i swapped the arguments when calling
> the function.
>> @@ -713,10 +713,8 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
>> set_byte_safe(data, 7, alloc_len, blk_desc_len & 0xff);
>> - scsi_set_in_resid_by_actual(cmd, actual_len);
>> return SAM_STAT_GOOD;
>> - scsi_set_in_resid_by_actual(cmd, 0);
>> sense_data_build(cmd, key, asc);
>> return SAM_STAT_CHECK_CONDITION;
> I guess we shouldn't remove these calls as the following reasoning:
> > 2, These commands never return residual under/overflow so dont set
> the residuals for them
> does not exactly apply here.
> Truncation of Data-IN buffer to ALLOC_LENGTH is not reflected in residual count,
> but if ALLOC_LENGTH is less than EDTL, it should produce a non-zero
> residual count.
> That is one of the purposes of scsi_set_in_resid_by_actual().
> Also, please refer to the commit log of d7af3dc1 which explains this logic.
> Is spc_mode_sense() really a special case which requires a different treatment?
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the stgt