Re: [PATCH 2/3] block layer varlen-cdb

[Boaz Harrosh <bharrosh@panasas.com>]

On Tue, Feb 12 2008 at 19:48 +0200, Christoph Hellwig <hch@infradead.org> wrote:
> On Sun, Feb 10, 2008 at 09:09:41PM +0200, Boaz Harrosh wrote:
>> - add varlen_cdb and varlen_cdb_len to hold a large user cdb
>>   if needed. They start as empty. Allocation of buffer must
>>   be done by user and held until request execution is done.
>> - Since there can be either a fix_length command up to 16 bytes
>>   or a variable_length, larger then 16 bytes, commands but never
>>   both, we hold the two types in a union to save space. The
>>   presence of varlen_cdb_len and cmd_len==0 signals a varlen_cdb
>>   mode.
> 
> this one I'm a bit confused by, why can't we just set the length
> of the variable length command in cmd_len aswell, and if cmd_len >
> the length of the cmd array it's a variable length command?
> 
> Note that this is both to keep the logic simpler and not to grow
> struct request further.  Especially for the rather rare case
> of a bidi command.

Because this will be dangerous for the Legacy block devices.
Unlike scsi drivers block drivers do not have a .max_cmnd_len
and upper layer will not check to make sure that the device supports
the longer command. If such a command goes through, lets say bsg
the drivers do blindly memcpy(,,rq->cmd_len) and will crash.
Better safe then sorry, at no cost.

Boaz