Re: [PATCH ver2 24/24] ide-scsi.c: convert to data accessors and !use_sg cleanup

[Boaz Harrosh <bharrosh@panasas.com>]

On Tue, Sep 18 2007 at 11:14 +0200, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Sep 18, 2007 at 11:03:21AM +0200, Boaz Harrosh wrote:
>>  - Convert ide-scsi to the new data accessors and cleanup
>>    the !use_sg code paths.
>>
>>   In old code the MODE_SENSE or MODE_SELECT code paths still
>>   assumed scsi_cmnd->request_buffer is a linear char pointer.
>>   This means that this driver was broken since 2.6.17. Though
>>   I admit this assumption is hidden behind a flag:
>>   test_bit(PC_TRANSFORM, &pc->flags).
>>
>>   I have hacked these code paths to properly handle an sg_count==1,
>>   which is true in todays implementation of MODE_SENSE or
>>   MODE_SELECT.
>>
>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>> ---
>>  drivers/scsi/ide-scsi.c |   85 +++++++++++++++++++++++++++--------------------
>>  1 files changed, 49 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
>> index 1cc01ac..5c9a444 100644
>> --- a/drivers/scsi/ide-scsi.c
>> +++ b/drivers/scsi/ide-scsi.c
>> @@ -63,7 +63,7 @@
>>  
>>  typedef struct idescsi_pc_s {
>>  	u8 c[12];				/* Actual packet bytes */
>> -	int request_transfer;			/* Bytes to transfer */
>> +	unsigned int request_transfer;		/* Bytes to transfer */
>>  	int actually_transferred;		/* Bytes actually transferred */
>>  	int buffer_size;			/* Size of our data buffer */
>>  	struct request *rq;			/* The corresponding request */
>> @@ -175,7 +175,8 @@ static void idescsi_input_buffers (ide_drive_t *drive, idescsi_pc_t *pc, unsigne
>>  	char *buf;
>>  
>>  	while (bcount) {
>> -		if (pc->sg - (struct scatterlist *) pc->scsi_cmd->request_buffer > pc->scsi_cmd->use_sg) {
>> +		if (pc->sg - scsi_sglist(pc->scsi_cmd) >
>> +		                                 scsi_sg_count(pc->scsi_cmd)) {
>>  			printk (KERN_ERR "ide-scsi: scatter gather table too small, discarding data\n");
>>  			idescsi_discard_data (drive, bcount);
>>  			return;
>>  static inline void idescsi_transform_pc1 (ide_drive_t *drive, idescsi_pc_t *pc)
>>  {
>> -	u8 *c = pc->c, *scsi_buf = pc->buffer, *sc = pc->scsi_cmd->cmnd;
>> -	char *atapi_buf;
>> -
>>  	if (!test_bit(PC_TRANSFORM, &pc->flags))
>>  		return;
>>  	if (drive->media == ide_cdrom || drive->media == ide_optical) {
>> +		u8 *c = pc->c;
>> +
>>  		if (c[0] == READ_6 || c[0] == WRITE_6) {
>>  			c[8] = c[4];		c[5] = c[3];		c[4] = c[2];
>>  			c[3] = c[1] & 0x1f;	c[2] = 0;		c[1] &= 0xe0;
>>  			c[0] += (READ_10 - READ_6);
>>  		}
>>  		if (c[0] == MODE_SENSE || c[0] == MODE_SELECT) {
>> +			u8 *sc = pc->scsi_cmd->cmnd;
>> +			char *atapi_buf;
>>  			unsigned short new_len;
>> -			if (!scsi_buf)
>> -				return;
> 
> Please just kill the translations.  Once we set use_10_for_rw and
> use_10_for_ms in the host template the upper layers won't send it anymore,
> and we should just reject these when coming from SG_IO.
> 

Thank you Christoph This is what I needed. I have set these 2 bits
and killed the translation. I will let the Device deal with SG_IO.
So I will not do any filtering of these commands. If SG_IO sends
the wrong commands to the wrong Devices it can deal with the
errors, right?

I will post a new ver3 patch.

Thanks
Boaz