On Sat, Apr 12 2008 at 8:52 +0300, FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote:
> On Sun, 06 Apr 2008 12:35:04 +0300
> Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
>
>> On Fri, Apr 04 2008 at 14:46 +0300, Jens Axboe <jens.axboe@xxxxxxxxxx> wrote:
>>> On Thu, Apr 03 2008, Boaz Harrosh wrote:
>>>> static void req_bio_endio(struct request *rq, struct bio *bio,
>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>>> index 6f79d40..2f87c9d 100644
>>>> --- a/include/linux/blkdev.h
>>>> +++ b/include/linux/blkdev.h
>>>> @@ -213,8 +213,15 @@ struct request {
>>>> /*
>>>> * when request is used as a packet command carrier
>>>> */
>>>> - unsigned int cmd_len;
>>>> - unsigned char cmd[BLK_MAX_CDB];
>>>> + unsigned short cmd_len;
>>>> + unsigned short ext_cdb_len; /* length of ext_cdb buffer */
>>>> + union {
>>>> + unsigned char cmd[BLK_MAX_CDB];
>>>> + unsigned char *ext_cdb;/* an optional extended cdb.
>>>> + * points to a user buffer that must
>>>> + * be valid until end of request
>>>> + */
>>>> + };
>>> Why not just something ala
>>>
>>> unsigned short cmd_len;
>>> unsigned char __cmd[BLK_MAX_CDB];
>>> unsigned char *cmd;
>>>
>>> and then have rq_init() do
>>>
>>> rq->cmd = rq->__cmd;
>>>
>>> and just have a function for setting up a larger ->cmd and adjusting
>>> ->cmd_len in the process?
>>>
>>> Then rq_set_cdb() would be
>>>
>>> static inline void rq_set_cdb(struct request *rq, u8 *cdb, short cdb_len)
>>> {
>>> rq->cmd = cdb;
>>> rq->cmd_len = cdb_len;
>>> }
>>>
>>> and rq_get_cdb() plus rq_get_cdb_len() could just go away.
>>>
>> Because this way it is dangerous if large commands are issued to legacy
>> drivers. In scsi-land we have .cmd_len at host template that will govern if
>> we are allow to issue larger commands to the driver. In block devices we do
>> not have such a facility, and the danger is if such commands are issued through
>> bsg or other means, even by malicious code. What you say is the ideal and it
>> is what I've done for scsi, but for block devices we can not do that yet.
>> With the way I did it here, Legacy drivers will see zero length command and
>> will do the right thing, from what I've seen.
>
> What are exactly block devices? ub and ide?
>
> bsg are created only for scsi devices (and scsi objects like sas host)
> now. Are there other means to send commands except for ioctl?
I'm not 100% sure either way, so I would like to be safe. Any way, there
is the size issue, this way we add *nothing* at all, so it looks preferable.
The final outcome will be the same both ways.
I would like if you reconsider the ugliness issue. I admit that at first I
personally disliked it, but now that I look at it, I think it is cleaner,
coding style, this way. Because the union points out the exclusiveness of
the two systems, the striate way give the notion of two separate systems.
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
