[Stgt-devel] [PATCH] Move device type to LUN

[Hannes Reinecke]

FUJITA Tomonori wrote:
> From: Hannes Reinecke <hare at suse.de>
> Subject: Re: [Stgt-devel] [PATCH] Move device type to LUN
> Date: Mon, 18 Jun 2007 08:29:13 +0200
> 
>> FUJITA Tomonori wrote:
>>> From: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
>>> Subject: Re: [Stgt-devel] [PATCH] Move device type to LUN
>>> Date: Fri, 15 Jun 2007 13:36:12 +0900
>>>
>>>> From: Hannes Reinecke <hare at suse.de>
>>>> Subject: Re: [Stgt-devel] [PATCH] Move device type to LUN
>>>> Date: Tue, 12 Jun 2007 14:44:03 +0200
>>>>
>>>>> FUJITA Tomonori wrote:
>>>>>> From: Hannes Reinecke <hare at suse.de>
>>>>>> Subject: Re: [Stgt-devel] [PATCH] Move device type to LUN
>>>>>> Date: Mon, 11 Jun 2007 17:15:19 +0200
>>>>>>
>>>>>>> Pete Wyckoff wrote:
>>>>>>>> hare at suse.de wrote on Tue, 05 Jun 2007 15:38 +0200:
>>>>>>>>> This patch moves the device type down to the LUN structure.
>>>>>>>>> And in doing so we now also have the proper peripheral device
>>>>>>>>> type and peripheral device qualifier attributes for the INQUIRY
>>>>>>>>> data.
>>>>>>>> Makes sense to me too.
>>>>>>>>
>>>>>>>>> One thing puzzles me, though: do we support commands with no
>>>>>>>>> LUN attached to it? IE is it valid to have 'cmd->dev == NULL'?
>>>>>>>>> If so: where is the point here? If that's our handling of a
>>>>>>>>> non-existing LUN 0 we should rather add a proper LUN 0 and
>>>>>>>>> treat cmd->dev == NULL as an error case ...
>>>>>>>> In the SCSI model, every device must have at least one LUN for
>>>>>>>> handling REPORT LUNS and a couple other commands.  It is addressed
>>>>>>>> as LUN 0 or using the "well-known" LUN for the command.  In the stgt
>>>>>>>> abstraction, though, there is no magic LUN like this.  Instead
>>>>>>>> things like spc_inqury use cmd->dev == NULL to handle this case.
>>>>>>>>
>>>>>>> Ah. Hence.
>>>>>>>
>>>>>>>> Perhaps it is reasonable to create a special device for these
>>>>>>>> commands.  Up in target.c, you could assign cmd->dev to target->dev,
>>>>>>>> where that is the special device, paralleling the way that
>>>>>>>> target->cmd_queue is used.  As a side effect, lots of "if (cmd->dev)
>>>>>>>> ... else ILLEGAL_REQUEST" clauses can be removed from device code
>>>>>>>> too.
>>>>>>>>
>>>>>>> Well, I actually thought of creating a proper LUN 0 with type 0xc.
>>>>>> We still need to handle cmd->dev == NULL case though probably we can
>>>>>> remove cmd->dev == NULL case in device type code.
>>>>>>
>>>>>> I don't have storage systems that work in your way. Is it common?
>>>>>>
>>>>>>
>>>>> Yes, quite common. HP and EMC (to name but a few) do it this way.
>>>>> Most storage arrays actually refuse to attach any devices to LUN0, as
>>>>> this is a pure management LUN.
>>>>>
>>>>> I will draft up a patch which creates a proper controller LUN0.
>>>> Ok, here's a patch. This depends on the patch that I've just
>>>> submitted.
>>>>
>>>> This adds scc support and fixes ibmvio. iSCSI works. ibmvio works for
>>>> me, but it detects scsi devices in a unique way, so I'm not sure it
>>>> works for everyone.
>>>>
>>>> As described, a scsi controler device is automatically created as lun0
>>>> with a new target.
>>>>
>>>> lily:/home/fujita# ./tgt/usr/tgtadm --lld iscsi --op new --mode target --tid 1 -T iqn.2001-04.com.example:storage.disk2.tulip.sys1.xyz
>>>> lily:/home/fujita# ./tgt/usr/tgtadm --op show --mode target
>>>> Target 1: iqn.2001-04.com.example:storage.disk2.tulip.sys1.xyz
>>>>     System information:
>>>>         Driver: iscsi
>>>>         Status: running
>>>>     I_T nexus information:
>>>>     LUN information:
>>>>         LUN: 0
>>>>             Type: raid
>>>>             SCSI ID: deadbeaf1:0
>>>>             SCSI SN: beaf10
>>>>             Size: 0
>>>>             Backing store: No backing store
>>>>     Account information:
>>>>     ACL information:
>>>>
>>>>
>>>> The patch is still very hacky but shows how it works. Let me know this
>>>> works or not.
>>> I did some cleanups.
>>>
>>> http://git.kernel.org/?p=linux/kernel/git/tomo/tgt.git;a=shortlog;h=scc
>>>
>> Regarding the disk_type_names cleanups:
>> - What is the rationale for this? Just truncating the full name to
>> something unintelligible doesn't seem like the smartest move to me,
>> especially as we're not facing any string length restrictions here ...
> 
> They are the names that lsscsi uses. But I don't have strong
> preference.
> 
Well, lsscsi has string length limitations as the output has to be
neatly arranged in colums; we're not facing these limitations here.
So I'd prefer to have legible names.

> 
>> Regarding device type registration cleanups:
>> - We're losing the ability to select sg_bst for passthrough during
>>   registration. Is that intentional?
> 
> Yeah. I plan to fix it though I'm not hurry since it needs bsg, which
> is supposed to be merged into 2.6.23.
Ah. Cool. I'll wait for it, then.

The other patches look ok to me. I see if I manage to give them a spin.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare at suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Markus Rex, HRB 16746 (AG N?rnberg)