Re: [PATCH 2/3] block layer varlen-cdb
On Tue, Feb 12 2008 at 19:54 +0200, Boaz Harrosh [EMAIL PROTECTED] wrote: On Tue, Feb 12 2008 at 19:48 +0200, Christoph Hellwig [EMAIL PROTECTED] 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 - PS: the struct request does not grow. Because before cmd_len was unsigned but now both cmd_len and varlen_cdb_len are short so it is the same as before. Boaz - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] block layer varlen-cdb
On Tue, Feb 12 2008 at 19:48 +0200, Christoph Hellwig [EMAIL PROTECTED] 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 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] block layer varlen-cdb
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. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
